Cool, happy to see it coming. I would also add tests for:
a) presence of sanitize_address function attribute
b) actual KASan instrumentation with correct shadow offset/scale.
c) missing module ctors/callbacks in KASan mode.
================
Comment at: lib/Transforms/Instrumentation/AddressSanitizer.cpp:1359
@@ -1349,3 +1358,3 @@
bool Changed = false;
----------------
Wait a second.... Do you do *anything* in AddressSanitizerModule pass in
`CompileKernel` mode? You don't add module constructor, and you don't
instrument globals. Maybe, we should just avoid creating this pass in the first
place?
================
Comment at: lib/Transforms/Instrumentation/AddressSanitizer.cpp:1405
@@ -1393,1 +1404,3 @@
+ const std::string MemIntrinCallbackPrefix =
+ CompileKernel ? std::string("") : ClMemoryAccessCallbackPrefix;
AsanMemmove = checkSanitizerInterfaceFunction(M.getOrInsertFunction(
----------------
Hm... So you don't have __asan_memset() in the kernel. Maybe, we shouldn't
instrument mem intrinsics in this case at all, not replace intrinisics with
call to plain memset()?
================
Comment at: lib/Transforms/Instrumentation/AddressSanitizer.cpp:1441
@@ +1440,3 @@
+ if (!CompileKernel) {
+ std::tie(AsanCtorFunction, AsanInitFunction) =
+ createSanitizerCtorAndInitFunctions(M, kAsanModuleCtorName,
----------------
You will access uninitialized memory in `AddressSanitizer::runOnFunction`
(potentially in `AddressSanitizer::maybeInsertAsanInitAtFunctionEntry` as well,
but the latter is relevant only for Mac).
================
Comment at: lib/Transforms/Instrumentation/AddressSanitizer.cpp:1530
@@ -1515,3 +1529,3 @@
- bool UseCalls = false;
+ bool UseCalls = CompileKernel;
if (ClInstrumentationWithCallsThreshold >= 0 &&
----------------
bool UseCalls = CompileKernel || (ClInstrumentationWithCallsThreshold >= 0 &&
...);
================
Comment at: tools/clang/include/clang/Basic/Sanitizers.def:44
@@ -43,1 +43,3 @@
+// Kernel AddressSanitizer (KASAN)
+SANITIZER("kernel-address", KernelAddress)
----------------
What is the "correct" way to capitalize it? KASAN? KASan?
================
Comment at: tools/clang/lib/AST/Decl.cpp:3684
@@ -3683,2 +3683,3 @@
ASTContext &Context = getASTContext();
- if (!Context.getLangOpts().Sanitize.has(SanitizerKind::Address) ||
+ if (!(Context.getLangOpts().Sanitize.has(SanitizerKind::Address) ||
+ Context.getLangOpts().Sanitize.has(SanitizerKind::KernelAddress)) ||
----------------
Wow, that's hard to parse. I'd vote for adding a method to `SanitizerSet`
if (!Context.getLangOpts().Sanitize.hasOneOf(SanitizerKind::Address,
SanitizerKind::KernelAddress)
...
That would also be useful in several places below.
================
Comment at: tools/clang/lib/CodeGen/BackendUtil.cpp:211
@@ +210,3 @@
+ PM.add(createAddressSanitizerFunctionPass(/*CompileKernel*/true));
+ PM.add(createAddressSanitizerModulePass(/*CompileKernel*/true));
+}
----------------
See note above - I'm not sure we need a module pass for that.
http://reviews.llvm.org/D10411
EMAIL PREFERENCES
http://reviews.llvm.org/settings/panel/emailpreferences/
_______________________________________________
cfe-commits mailing list
cfe-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
