Add -fsanitize=kernel-hwaddress flag, that enables -hwasan-kernel=1 -hwasan-recover=1 -hwasan-match-all-tag=0xff.
Details
Diff Detail
Event Timeline
Note: goes on top of https://reviews.llvm.org/D44827 and https://reviews.llvm.org/D44981
llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp | ||
---|---|---|
733 | I'd prefer a CHECK(!CompileKernel || Recover). | |
llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp | ||
345 | How about matchAllTag = ClMatchAllTag.getNumOccurrences() > 0 ? ClMatchAllTag : (CompileKernel ? 0xFF : -1) ? In fact, the same logic could be applied to CompileKernel and Recover in the constructor. | |
llvm/test/Instrumentation/HWAddressSanitizer/X86/kernel.ll | ||
21 | what happened here? Recover vs trap difference? |
llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp | ||
---|---|---|
345 | getNumOccurrences is what I was looking for! Done for ClMatchAllTag and ClRecover. For ClRecover the behavior now is: when ClRecover is provided, use it's value; otherwise use True if CompileKernel and the value passed from CGOpts if not CompileKernel. | |
llvm/test/Instrumentation/HWAddressSanitizer/X86/kernel.ll | ||
21 | Had to change that since I forced ClRecover when ClKhwasan. Fixed now. |
llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp | ||
---|---|---|
345 | Looking at this: this->Recover = ClRecover.getNumOccurrences() > 0 ? ClRecover : Recover; Recover does not depend on CompileKernel at all. Why not do the same getNumOccurrences() thing for CompileKernel? What happens if the user asks for -fsanitize=kernel-hwaddress -fsanitize-recover=kernel-hwaddress? Looks like recover will be silently ignored. Could this be fixed by declaring it non-recoverable in SanitizerArgs? |
llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp | ||
---|---|---|
345 |
Done
Ah, so there's -fsanitize-recover and that's where those CodeGenOptions come from :) You probably meant -fsanitize=kernel-hwaddress -fnosanitize-recover=kernel-hwaddress (-fsanitize=kernel-[hw]address always enables recover right now)? Yes, in this case it the latter flag will be ignored. Is there a way to mark -fsanitize=kernel-address non-unrecoverable? There's a little chance that we might want to change that in the future and make KASAN/KHWASAN support the recover=0 mode. |
llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp | ||
---|---|---|
345 | It would need to be done in SanitizerArgs.cpp (see ex. Unrecoverable). It appears that we don't have any sanitizers that are not-not-recoverable yet. |
PTAL. Added AlwaysRecoverable to SanitizerArgs.cpp. Right now recovery is disallowed for both KASan and KHWASan, but all the appropriate flags seem to be passed properly to the instrumentation pass, so we can easily allow it in the future in we want.
clang/lib/Driver/SanitizerArgs.cpp | ||
---|---|---|
451 | Looks great, but this new logic needs a test. |
the tests fail with:
clang: /code/llvm-project/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp:1064: llvm::FunctionPass *llvm::createAddressSanitizerFunctionPass(bool, bool, bool): Assertion `!CompileKernel || Recover' failed.
Hm, I don't see this failure. Just in case I rebased onto the latest tip.
How do you run the tests? I tried both make -j48 check-clang check-llvm and make -j48 check-all. I do see some tests failing, but they are not related to Sanitizers.
Simply ninja check-all. Maybe the patch got applied incorrectly. Phabricator is really bad with file copies.
clang/test/CodeGen/address-safety-attr-flavors.cpp | ||
---|---|---|
17–1 | ^^^ this line expands to this: and clang: /code/llvm-project/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp:1064: llvm::FunctionPass *llvm::createAddressSanitizerFunctionPass(bool, bool, bool): Assertion `!CompileKernel || Recover' failed. |
Fixed the assert. PTAL.
The reason tests didn't work for me was Release build mode. Tried Debug mode - apparently 64 GB RAM is not enough to build llvm =/ Ended up using Release build with asserts enabled.
I've also got commit access now, so I'll try to commit this myself.
Could be too many parallel links, try -DLLVM_PARALLEL_LINK_JOBS=3.
Anyway, Debug build is way too slow. I normally use Release with asserts, too.
Looks great, but this new logic needs a test.