Currently, __attribute__((no_sanitize('hwaddress'))) is not possible. Add this piece of plumbing, and now that we properly support copying attributes between an old and a new global variable, add a regression test for the GlobalOpt bug that previously lost the attribute.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
Thank you for this, can you also add a release note for the fix?
clang/lib/Sema/SemaDeclAttr.cpp | ||
---|---|---|
8879 | Spurious whitespace change? | |
clang/test/CodeGen/hwasan-globals.cpp | ||
2–3 | Are these files automatically deleted when the test is done because we're using %t, or do we need to clean those up manually? | |
10 | Should we add a memtag test as well given that also changed in this patch? | |
compiler-rt/test/hwasan/TestCases/global-with-reduction.c | ||
25 | I'm not a compiler-rt expert, but is this valid? I assume this is using the system stdlib.h which is not something we usually want in lit tests. I think that's why atoi was previously being forward declared; then we don't need to include the whole header file. | |
50 | This is unnecessary -- falling off main already returns 0. | |
compiler-rt/test/hwasan/TestCases/global.c | ||
47 | This is unnecessary -- falling off main already returns 0. |
Update.
clang/lib/Sema/SemaDeclAttr.cpp | ||
---|---|---|
8879 | unfortunately there's no way in my editor to trim trailing whitespace only on changed lines :(, so i end up fixing things like this drive-by. let me know if you feel very strongly about this diff and I can kill it, but I personally think the drive-by-fix isn't a huge problem and the alternative of whitespace-fix-only commit seems a bit overkill | |
clang/test/CodeGen/hwasan-globals.cpp | ||
2–3 | AFAIK nothing is ever automatically deleted (e.g. the outputs of the compiler). Is automated cleanup here necessary? | |
10 | sure, done | |
compiler-rt/test/hwasan/TestCases/global-with-reduction.c | ||
25 | I don't see there being any problem with including stdlib.h here, it's done in lots of other compiler-rt tests. I patched this up because the compiler actually complains about forward-declaring c library functions (it's just silenced by llvm-lit by default). | |
50 | sure, done | |
compiler-rt/test/hwasan/TestCases/global.c | ||
47 | sure, done |
clang/lib/Sema/SemaDeclAttr.cpp | ||
---|---|---|
8879 | Personally, I don't feel very strongly because the chances of the whitespace being someone's entrypoint to git-blame is pretty minimal (especially given there's only one change here). However, we typically still ask for formatting changes to be separated out (https://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access #2) rather than lumped in with functional changes so reviewers will ask for these sort of changes to be backed out, so this may crop up repeatedly if your editor doesn't give you the options you need. | |
clang/test/CodeGen/hwasan-globals.cpp | ||
2–3 | Necessary? Probably not (I'd expect these to go into the temp directory). A kindness so disks don't fill up? Probably. Because the content here is static anyway, these could just be files in the Inputs directory so they don't need to be created every time the test is run. | |
compiler-rt/test/hwasan/TestCases/global-with-reduction.c | ||
25 | Ah, good point about other tests doing this (it's a restriction we have in Clang tests, but not here from what I can tell). |
Btw, precommit CI caught a failure:
FAIL: Clang :: CodeGen/hwasan-globals.cpp (4495 of 15547) ******************** TEST 'Clang :: CodeGen/hwasan-globals.cpp' FAILED ******************** Script: -- : 'RUN: at line 1'; echo "int extra_global;" > C:\ws\w3\llvm-project\premerge-checks\build\tools\clang\test\CodeGen\Output\hwasan-globals.cpp.tmp.extra-source.cpp : 'RUN: at line 2'; echo "global:*ignorelisted_global*" > C:\ws\w3\llvm-project\premerge-checks\build\tools\clang\test\CodeGen\Output\hwasan-globals.cpp.tmp.ignorelist : 'RUN: at line 3'; c:\ws\w3\llvm-project\premerge-checks\build\bin\clang.exe -cc1 -internal-isystem c:\ws\w3\llvm-project\premerge-checks\build\lib\clang\15.0.0\include -nostdsysteminc -include C:\ws\w3\llvm-project\premerge-checks\build\tools\clang\test\CodeGen\Output\hwasan-globals.cpp.tmp.extra-source.cpp -fsanitize=hwaddress -fsanitize-ignorelist=C:\ws\w3\llvm-project\premerge-checks\build\tools\clang\test\CodeGen\Output\hwasan-globals.cpp.tmp.ignorelist -emit-llvm -o - C:\ws\w3\llvm-project\premerge-checks\clang\test\CodeGen\hwasan-globals.cpp | c:\ws\w3\llvm-project\premerge-checks\build\bin\filecheck.exe C:\ws\w3\llvm-project\premerge-checks\clang\test\CodeGen\hwasan-globals.cpp --check-prefixes=CHECK : 'RUN: at line 5'; echo "src:C:\ws\w3\llvm-project\premerge-checks\clang\test\CodeGen\hwasan-globals.cpp" | sed -e 's/\\/\\\\/g' > C:\ws\w3\llvm-project\premerge-checks\build\tools\clang\test\CodeGen\Output\hwasan-globals.cpp.tmp.ignorelist-src : 'RUN: at line 6'; c:\ws\w3\llvm-project\premerge-checks\build\bin\clang.exe -cc1 -internal-isystem c:\ws\w3\llvm-project\premerge-checks\build\lib\clang\15.0.0\include -nostdsysteminc -include C:\ws\w3\llvm-project\premerge-checks\build\tools\clang\test\CodeGen\Output\hwasan-globals.cpp.tmp.extra-source.cpp -fsanitize=hwaddress -fsanitize-ignorelist=C:\ws\w3\llvm-project\premerge-checks\build\tools\clang\test\CodeGen\Output\hwasan-globals.cpp.tmp.ignorelist-src -emit-llvm -o - C:\ws\w3\llvm-project\premerge-checks\clang\test\CodeGen\hwasan-globals.cpp | c:\ws\w3\llvm-project\premerge-checks\build\bin\filecheck.exe C:\ws\w3\llvm-project\premerge-checks\clang\test\CodeGen\hwasan-globals.cpp --check-prefix=IGNORELIST -- Exit Code: 1 Command Output (stdout): -- $ ":" "RUN: at line 1" $ "echo" "int extra_global;" $ ":" "RUN: at line 2" $ "echo" "global:*ignorelisted_global*" $ ":" "RUN: at line 3" $ "c:\ws\w3\llvm-project\premerge-checks\build\bin\clang.exe" "-cc1" "-internal-isystem" "c:\ws\w3\llvm-project\premerge-checks\build\lib\clang\15.0.0\include" "-nostdsysteminc" "-include" "C:\ws\w3\llvm-project\premerge-checks\build\tools\clang\test\CodeGen\Output\hwasan-globals.cpp.tmp.extra-source.cpp" "-fsanitize=hwaddress" "-fsanitize-ignorelist=C:\ws\w3\llvm-project\premerge-checks\build\tools\clang\test\CodeGen\Output\hwasan-globals.cpp.tmp.ignorelist" "-emit-llvm" "-o" "-" "C:\ws\w3\llvm-project\premerge-checks\clang\test\CodeGen\hwasan-globals.cpp" $ "c:\ws\w3\llvm-project\premerge-checks\build\bin\filecheck.exe" "C:\ws\w3\llvm-project\premerge-checks\clang\test\CodeGen\hwasan-globals.cpp" "--check-prefixes=CHECK" # command stderr: C:\ws\w3\llvm-project\premerge-checks\clang\test\CodeGen\hwasan-globals.cpp:21:11: error: CHECK: expected string not found in input // CHECK: @{{.*}}extra_global{{.*}}.hwasan = ^ <stdin>:10:77: note: scanning from here @"?ignorelisted_global@@3HA" = dso_local global i32 0, no_sanitize_hwaddress, align 4 ^ <stdin>:17:1: note: possible intended match here @"?extra_global@@3HA.hwasan" = private global { i32, [12 x i8] } { i32 0, [12 x i8] c"\00\00\00\00\00\00\00\00\00\00\00\01" }, align 16 ^ Input file: <stdin> Check file: C:\ws\w3\llvm-project\premerge-checks\clang\test\CodeGen\hwasan-globals.cpp -dump-input=help explains the following input dump. Input was: <<<<<< . . . 5: 6: $hwasan.module_ctor = comdat any 7: 8: @"?attributed_global@@3HA" = dso_local global i32 0, no_sanitize_hwaddress, align 4 9: @"?disable_instrumentation_global@@3HA" = dso_local global i32 0, no_sanitize_hwaddress, align 4 10: @"?ignorelisted_global@@3HA" = dso_local global i32 0, no_sanitize_hwaddress, align 4 check:21'0 X~~~~~~~~~ error: no match found 11: @llvm.used = appending global [1 x ptr] [ptr @hwasan.module_ctor], section "llvm.metadata" check:21'0 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 12: @llvm.global_ctors = appending global [1 x { i32, ptr, ptr }] [{ i32, ptr, ptr } { i32 0, ptr @hwasan.module_ctor, ptr @hwasan.module_ctor }] check:21'0 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 13: @__start_hwasan_globals = external hidden constant [0 x i8] check:21'0 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 14: @__stop_hwasan_globals = external hidden constant [0 x i8] check:21'0 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 15: @hwasan.note = private constant { i32, i32, i32, [8 x i8], i32, i32 } { i32 8, i32 8, i32 3, [8 x i8] c"LLVM\00\00\00\00", i32 trunc (i64 sub (i64 ptrtoint (ptr @__start_hwasan_globals to i64), i64 ptrtoint (ptr @hwasan.note to i64)) to i32), i32 trunc (i64 sub (i64 ptrtoint (ptr @__stop_hwasan_globals to i64), i64 ptrtoint (ptr @hwasan.note to i64)) to i32) }, section ".note.hwasan.globals", comdat($hwasan.module_ctor), align 4 check:21'0 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 16: @hwasan.dummy.global = private constant [0 x i8] zeroinitializer, section "hwasan_globals", comdat($hwasan.module_ctor), !associated !0 check:21'0 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 17: @"?extra_global@@3HA.hwasan" = private global { i32, [12 x i8] } { i32 0, [12 x i8] c"\00\00\00\00\00\00\00\00\00\00\00\01" }, align 16 check:21'0 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ check:21'1 ? possible intended match 18: @"?extra_global@@3HA.hwasan.descriptor" = private constant { i32, i32 } { i32 trunc (i64 sub (i64 ptrtoint (ptr @"?extra_global@@3HA.hwasan" to i64), i64 ptrtoint (ptr @"?extra_global@@3HA.hwasan.descriptor" to i64)) to i32), i32 16777220 }, section "hwasan_globals", !associated !1 check:21'0 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 19: @"?global@@3HA.hwasan" = private global { i32, [12 x i8] } { i32 0, [12 x i8] c"\00\00\00\00\00\00\00\00\00\00\00\02" }, align 16 check:21'0 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 20: @"?global@@3HA.hwasan.descriptor" = private constant { i32, i32 } { i32 trunc (i64 sub (i64 ptrtoint (ptr @"?global@@3HA.hwasan" to i64), i64 ptrtoint (ptr @"?global@@3HA.hwasan.descriptor" to i64)) to i32), i32 33554436 }, section "hwasan_globals", !associated !2 check:21'0 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 21: @"?static_var@?1??func@@YAXXZ@4HA.hwasan" = private global { i32, [12 x i8] } { i32 0, [12 x i8] c"\00\00\00\00\00\00\00\00\00\00\00\03" }, align 16 check:21'0 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 22: @"?static_var@?1??func@@YAXXZ@4HA.hwasan.descriptor" = private constant { i32, i32 } { i32 trunc (i64 sub (i64 ptrtoint (ptr @"?static_var@?1??func@@YAXXZ@4HA.hwasan" to i64), i64 ptrtoint (ptr @"?static_var@?1??func@@YAXXZ@4HA.hwasan.descriptor" to i64)) to i32), i32 50331652 }, section "hwasan_globals", !associated !3 check:21'0 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ . . . >>>>>> error: command failed with exit status: 1 -- ********************
clang/lib/Sema/SemaDeclAttr.cpp | ||
---|---|---|
8879 | Fixed in ee28837a1fbd574dbec14b9f09cb4effab6a492a. | |
clang/test/CodeGen/hwasan-globals.cpp | ||
2–3 | Done (and also will update asan-globals.cpp when this lands to use the new files as well). |
LGTM! The debian failures look like precommit CI is in a broken state again, but have nothing to do with your patch.
clang/lib/Sema/SemaDeclAttr.cpp | ||
---|---|---|
8879 | Thanks! |
From the content of this patch, it probably is llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp:
switch (TargetTriple.getArch()) { case Triple::x86_64: // The signal handler will find the data address in rdi. Asm = InlineAsm::get( FunctionType::get(IRB.getVoidTy(), {PtrLong->getType()}, false), "int3\nnopl " + itostr(0x40 + (AccessInfo & HWASanAccessInfo::RuntimeMask)) + "(%rax)", "{rdi}", /*hasSideEffects=*/true); break; case Triple::aarch64: case Triple::aarch64_be: // The signal handler will find the data address in x0. Asm = InlineAsm::get( FunctionType::get(IRB.getVoidTy(), {PtrLong->getType()}, false), "brk #" + itostr(0x900 + (AccessInfo & HWASanAccessInfo::RuntimeMask)), "{x0}", /*hasSideEffects=*/true); break; default: report_fatal_error("unsupported architecture"); }
I thought *-registered-target is true as long as the target is available, not only when the target happens to be the default.
Also, if there would be more such tests in the future, maybe a LIT feature that the default target supports HWAsan makes sense?
Too right. Given the purpose of this test is to just check that the globals have the right IR attributes, any target is fine - as the IR attributes aren't target-specific.
Took another fix-forward whack-a-mole attempt, committing in a sec.
I see that the clang-ppc64le-linux bot is green with the second attempt (https://lab.llvm.org/buildbot/#/builders/105/builds/27200). Please let me know if you have further issues.
Spurious whitespace change?