[PATCH] [ASan] Initial support for Kernel AddressSanitizer

[Alexey Samsonov]

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/