[RFC] [KASAN] Instrument globals

[glider]

Summary

Enable globals instrumentation in KASAN. To avoid breaking the kernel logic, we don't instrument non-default data sections, most of which usually contain arrays of structs or pointers.

[dvyukov]

Alex, is this still relevant? If not, please close.

lib/CodeGen/AsmPrinter/AsmPrinter.cpp

void AsmPrinter::EmitGlobalVariable(const GlobalVariable *GV) {
 
   if (MAI->hasDotTypeDotSizeDirective())
     OutStreamer->EmitSymbolAttribute(EmittedSym, MCSA_ELF_TypeObject);
 
   SectionKind GVKind = TargetLoweringObjectFile::getKindForGlobal(GV, TM);
 
   const DataLayout &DL = GV->getParent()->getDataLayout();
   uint64_t Size = DL.getTypeAllocSize(GV->getType()->getElementType());
+  uint64_t SymbolSize = 0;
+  if (GV->hasAttribute("global-symbol-size")) {
+    auto Attr = GV->getAttribute("global-symbol-size");
+    if (Attr.isStringAttribute())
+      SymbolSize = std::stoi(Attr.getValueAsString());
+  }
 
   // If the alignment is specified, we *must* obey it.  Overaligning a global
   // with a specified alignment is a prompt way to break globals emitted to
   // sections and expected to be contiguous (e.g. ObjC metadata).
   unsigned AlignLog = getGVAlignmentLog2(GV, DL);
 
   for (const HandlerInfo &HI : Handlers) {
     NamedRegionTimer T(HI.TimerName, HI.TimerDescription,
                        HI.TimerGroupName, HI.TimerGroupDescription,
                        TimePassesIsEnabled);
-    HI.Handler->setSymbolSize(GVSym, Size);
+    HI.Handler->setSymbolSize(GVSym, SymbolSize);
   }
 
   // Handle common symbols
   if (GVKind.isCommon()) {
     if (Size == 0) Size = 1;   // .comm Foo, 0 is undefined, avoid it.
+    if (SymbolSize == 0) SymbolSize = Size;
     unsigned Align = 1 << AlignLog;
     if (!getObjFileLowering().getCommDirectiveSupportsAlignment())
       Align = 0;
 
     // .comm _foo, 42, 4
-    OutStreamer->EmitCommonSymbol(GVSym, Size, Align);
+    OutStreamer->EmitCommonSymbol(GVSym, SymbolSize, Align);
     return;
   }
 
   // Determine to which section this global should be emitted.
   MCSection *TheSection = getObjFileLowering().SectionForGlobal(GV, GVKind, TM);
 
   // If we have a bss global going to a section that supports the
   // zerofill directive, do so here.
   if (GVKind.isBSS() && MAI->hasMachoZeroFillDirective() &&
       TheSection->isVirtualSection()) {
     if (Size == 0)
       Size = 1; // zerofill of 0 bytes is undefined.
+    if (SymbolSize == 0) SymbolSize = Size;
     unsigned Align = 1 << AlignLog;
     EmitLinkage(GV, GVSym);
     // .zerofill __DATA, __bss, _foo, 400, 5
-    OutStreamer->EmitZerofill(TheSection, GVSym, Size, Align);
+    OutStreamer->EmitZerofill(TheSection, GVSym, SymbolSize, Align);
     return;
   }
 
   // If this is a BSS local symbol and we are emitting in the BSS
   // section use .lcomm/.comm directive.
   if (GVKind.isBSSLocal() &&
       getObjFileLowering().getBSSSection() == TheSection) {
     if (Size == 0)
       Size = 1; // .comm Foo, 0 is undefined, avoid it.
+    if (SymbolSize == 0) SymbolSize = Size;
     unsigned Align = 1 << AlignLog;
 
     // Use .lcomm only if it supports user-specified alignment.
     // Otherwise, while it would still be correct to use .lcomm in some
     // cases (e.g. when Align == 1), the external assembler might enfore
     // some -unknown- default alignment behavior, which could cause
     // spurious differences between external and integrated assembler.
     // Prefer to simply fall back to .local / .comm in this case.
     if (MAI->getLCOMMDirectiveAlignmentType() != LCOMM::NoAlignment) {
       // .lcomm _foo, 42
-      OutStreamer->EmitLocalCommonSymbol(GVSym, Size, Align);
+      OutStreamer->EmitLocalCommonSymbol(GVSym, SymbolSize, Align);
       return;
     }
 
     if (!getObjFileLowering().getCommDirectiveSupportsAlignment())
       Align = 0;
 
     // .local _foo
     OutStreamer->EmitSymbolAttribute(GVSym, MCSA_Local);
     // .comm _foo, 42, 4
-    OutStreamer->EmitCommonSymbol(GVSym, Size, Align);
+    OutStreamer->EmitCommonSymbol(GVSym, SymbolSize, Align);
     return;
   }
 
   // Handle thread local data for mach-o which requires us to output an
   // additional structure of data and mangle the original symbol so that we
   // can reference it later.
   //
   // TODO: This should become an "emit thread local global" method on TLOF.
   // All of this macho specific stuff should be sunk down into TLOFMachO and
   // stuff like "TLSExtraDataSection" should no longer be part of the parent
   // TLOF class.  This will also make it more obvious that stuff like
   // MCStreamer::EmitTBSSSymbol is macho specific and only called from macho
   // specific code.
   if (GVKind.isThreadLocal() && MAI->hasMachoTBSSDirective()) {
     // Emit the .tbss symbol
     MCSymbol *MangSym =
         OutContext.getOrCreateSymbol(GVSym->getName() + Twine("$tlv$init"));
 
     if (GVKind.isThreadBSS()) {
       TheSection = getObjFileLowering().getTLSBSSSection();
-      OutStreamer->EmitTBSSSymbol(TheSection, MangSym, Size, 1 << AlignLog);
+      OutStreamer->EmitTBSSSymbol(TheSection, MangSym, SymbolSize, 1 << AlignLog);
     } else if (GVKind.isThreadData()) {
       OutStreamer->SwitchSection(TheSection);
 
       EmitAlignment(AlignLog, GV);
       OutStreamer->EmitLabel(MangSym);
 
       EmitGlobalConstant(GV->getParent()->getDataLayout(),
                          GV->getInitializer());
void AsmPrinter::EmitGlobalVariable(const GlobalVariable *GV) {
 
   OutStreamer->EmitLabel(EmittedInitSym);
 
   EmitGlobalConstant(GV->getParent()->getDataLayout(), GV->getInitializer());
 
   if (MAI->hasDotTypeDotSizeDirective())
     // .size foo, 42
     OutStreamer->emitELFSize(EmittedInitSym,
-                             MCConstantExpr::create(Size, OutContext));
+                             MCConstantExpr::create(SymbolSize, OutContext));
 
   OutStreamer->AddBlankLine();
 }
 
 /// Emit the directive and value for debug thread local expression
 ///
 /// \p Value - The value to emit.
 /// \p Size - The size of the integer (in bytes) to emit.

lib/Transforms/Instrumentation/AddressSanitizer.cpp

if (Comdat *C = G->getComdat()) {
     case Comdat::SameSize:
       return false;
     }
   }
 
   if (G->hasSection()) {
     StringRef Section = G->getSection();
 
+    // There are many non-default sections in the kernel, many of which contain
+    // arrays of structures and pointers, which are merged together and
+    // traversed at runtime. Instrumenting them would break the kernel.
+    if (CompileKernel)
+      return false;
+
     // Globals from llvm.metadata aren't emitted, do not instrument them.
     if (Section == "llvm.metadata") return false;
     // Do not instrument globals from special LLVM sections.
     if (Section.find("__llvm") != StringRef::npos || Section.find("__LLVM") != StringRef::npos) return false;
 
     // Do not instrument function pointers to initialization and termination
     // routines: dynamic linker will not properly handle redzones.
     if (Section.startswith(".preinit_array") ||
for (size_t i = 0; i < n; i++) {
     Indices2[0] = IRB.getInt32(0);
     Indices2[1] = IRB.getInt32(0);
 
     G->replaceAllUsesWith(
         ConstantExpr::getGetElementPtr(NewTy, NewGlobal, Indices2, true));
     NewGlobal->takeName(G);
     G->eraseFromParent();
     NewGlobals[i] = NewGlobal;
+    NewGlobal->addAttribute("global-symbol-size", std::to_string(SizeInBytes));
 
     Constant *SourceLoc;
     if (!MD.SourceLoc.empty()) {
       auto SourceLocGlobal = createPrivateGlobalForSourceLoc(M, MD.SourceLoc);
       SourceLoc = ConstantExpr::getPointerCast(SourceLocGlobal, IntptrTy);
     } else {
       SourceLoc = ConstantInt::get(IntptrTy, 0);
     }
for (size_t i = 0; i < n; i++) {
     GlobalsToAddToUsedList.push_back(G);
   }
   appendToCompilerUsed(M, ArrayRef<GlobalValue *>(GlobalsToAddToUsedList));
 
   std::string ELFUniqueModuleId =
       (UseGlobalsGC && TargetTriple.isOSBinFormatELF()) ? getUniqueModuleId(&M)
                                                         : "";
 
-  if (!ELFUniqueModuleId.empty()) {
+  // Clang emits invalid flags for section directives when trying to instrument
+  // kernel globals with InstrumentGlobalsELF().
+  if (!ELFUniqueModuleId.empty() && !CompileKernel) {
     InstrumentGlobalsELF(IRB, M, NewGlobals, Initializers, ELFUniqueModuleId);
     *CtorComdat = true;
   } else if (UseGlobalsGC && TargetTriple.isOSBinFormatCOFF()) {
     InstrumentGlobalsCOFF(IRB, M, NewGlobals, Initializers);
   } else if (UseGlobalsGC && ShouldUseMachOGlobalsSection()) {
     InstrumentGlobalsMachO(IRB, M, NewGlobals, Initializers);
   } else {
     InstrumentGlobalsWithMetadataArray(IRB, M, NewGlobals, Initializers);

 bool AddressSanitizerModule::runOnModule(Module &M) {
   C = &(M.getContext());
   int LongSize = M.getDataLayout().getPointerSizeInBits();
   IntptrTy = Type::getIntNTy(*C, LongSize);
   TargetTriple = Triple(M.getTargetTriple());
   Mapping = getShadowMapping(TargetTriple, LongSize, CompileKernel);
   initializeCallbacks(M);
 
-  if (CompileKernel)
-    return false;
+  // No need to emit __asan_init and __asan_version_mismatch_check_vXX for the
+  // kernel.
+  std::string VersionCheckName =
+      CompileKernel
+          ? ""
+          : kAsanVersionCheckNamePrefix + std::to_string(GetAsanVersion(M));
+  std::string InitName = CompileKernel ? "" : kAsanInitName;
 
   // Create a module constructor. A destructor is created lazily because not all
   // platforms, and not all modules need it.
-  std::string VersionCheckName =
-      kAsanVersionCheckNamePrefix + std::to_string(GetAsanVersion(M));
   std::tie(AsanCtorFunction, std::ignore) = createSanitizerCtorAndInitFunctions(
-      M, kAsanModuleCtorName, kAsanInitName, /*InitArgTypes=*/{},
+      M, kAsanModuleCtorName, InitName, /*InitArgTypes=*/{},
       /*InitArgs=*/{}, VersionCheckName);
 
   bool CtorComdat = true;
   bool Changed = false;
-  // TODO(glider): temporarily disabled globals instrumentation for KASan.
   if (ClGlobals) {
     IRBuilder<> IRB(AsanCtorFunction->getEntryBlock().getTerminator());
     Changed |= InstrumentGlobals(IRB, M, &CtorComdat);
   }
 
   // Put the constructor and destructor in comdat if both
   // (1) global instrumentation is not TU-specific
   // (2) target is ELF.
   if (UseCtorComdat && TargetTriple.isOSBinFormatELF() && CtorComdat) {
+    // When building the kernel, don't emit the constructor if there're no
+    // globals.
+    if (!CompileKernel || Changed) {
       AsanCtorFunction->setComdat(M.getOrInsertComdat(kAsanModuleCtorName));
       appendToGlobalCtors(M, AsanCtorFunction, kAsanCtorAndDtorPriority,
                           AsanCtorFunction);
+    }
     if (AsanDtorFunction) {
       AsanDtorFunction->setComdat(M.getOrInsertComdat(kAsanModuleDtorName));
       appendToGlobalDtors(M, AsanDtorFunction, kAsanCtorAndDtorPriority,
                           AsanDtorFunction);
     }
   } else {
     appendToGlobalCtors(M, AsanCtorFunction, kAsanCtorAndDtorPriority);
     if (AsanDtorFunction)

lib/Transforms/Utils/ModuleUtils.cpp

 
 std::pair<Function *, Function *> llvm::createSanitizerCtorAndInitFunctions(
     Module &M, StringRef CtorName, StringRef InitName,
     ArrayRef<Type *> InitArgTypes, ArrayRef<Value *> InitArgs,
     StringRef VersionCheckName) {
   assert(!InitName.empty() && "Expected init function name");
   assert(InitArgs.size() == InitArgTypes.size() &&
          "Sanitizer's init function expects different number of arguments");
-  Function *InitFunction =
-      declareSanitizerInitFunction(M, InitName, InitArgTypes);
+  Function *InitFunction = nullptr;
+
   Function *Ctor = Function::Create(
       FunctionType::get(Type::getVoidTy(M.getContext()), false),
       GlobalValue::InternalLinkage, CtorName, &M);
   BasicBlock *CtorBB = BasicBlock::Create(M.getContext(), "", Ctor);
   IRBuilder<> IRB(ReturnInst::Create(M.getContext(), CtorBB));
+  if (!InitName.empty()) {
+    InitFunction = declareSanitizerInitFunction(M, InitName, InitArgTypes);
     IRB.CreateCall(InitFunction, InitArgs);
+  }
   if (!VersionCheckName.empty()) {
     Function *VersionCheckFunction =
         checkSanitizerInterfaceFunction(M.getOrInsertFunction(
             VersionCheckName, FunctionType::get(IRB.getVoidTy(), {}, false),
             AttributeList()));
     IRB.CreateCall(VersionCheckFunction, {});
   }
   return std::make_pair(Ctor, InitFunction);

test/Instrumentation/AddressSanitizer/adaptive_global_redzones.ll

 ; RUN: opt < %s -asan -asan-module -S | FileCheck %s
 ; RUN: opt < %s -asan -asan-module -asan-mapping-scale=5 -S | FileCheck %s
+; RUN: opt < %s -asan -asan-module -asan-kernel=1 -S | FileCheck %s
 target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32:64"
 target triple = "x86_64-unknown-linux-gnu"
 
 ; Here we check that the global redzone sizes grow with the object size.
 
 @G10 = global [10 x i8] zeroinitializer, align 1
 ; CHECK: @G10 = global { [10 x i8], [54 x i8] }