hwasan: Remove the old frame descriptor mechanism.

[pcc]

Summary

Depends on D63469

[eugenis]

LGTM

llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp

154 ↗

(On Diff #205239)

We use this in android (hwasan-create-frame-descriptions=0). Could we keep the flag around for a while?

[pcc]

llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp

154 ↗

(On Diff #205239)

Yeah, I needed to hack the flag back in (similar to https://github.com/pcc/llvm-project/commit/576c50647de5bda105cf79e97d30d0f452d7986f) to get the Android build working again. Since there's no guarantee of stability around these flags, maybe Android should be doing something similar in their compiler temporarily?

[eugenis]

llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp

154 ↗

(On Diff #205239)

That could be a better option.
In hindsight, we should not have enabled this experimental feature by default.

llvm/trunk/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp

static cl::opt<bool> ClWithTls(
     cl::Hidden, cl::init(true));
 
 static cl::opt<bool>
     ClRecordStackHistory("hwasan-record-stack-history",
                          cl::desc("Record stack frames with tagged allocations "
                                   "in a thread-local ring buffer"),
                          cl::Hidden, cl::init(true));
 static cl::opt<bool>
-    ClCreateFrameDescriptions("hwasan-create-frame-descriptions",
-                              cl::desc("create static frame descriptions"),
-                              cl::Hidden, cl::init(true));
-
-static cl::opt<bool>
     ClInstrumentMemIntrinsics("hwasan-instrument-mem-intrinsics",
                               cl::desc("instrument memory intrinsics"),
                               cl::Hidden, cl::init(true));
 
 static cl::opt<bool>
     ClInstrumentLandingPads("hwasan-instrument-landing-pads",
                               cl::desc("instrument landing pads"), cl::Hidden,
                               cl::init(true));

 
 private:
   LLVMContext *C;
   std::string CurModuleUniqueId;
   Triple TargetTriple;
   FunctionCallee HWAsanMemmove, HWAsanMemcpy, HWAsanMemset;
   FunctionCallee HWAsanHandleVfork;
 
-  // Frame description is a way to pass names/sizes of local variables
-  // to the run-time w/o adding extra executable code in every function.
-  // We do this by creating a separate section with {PC,Descr} pairs and passing
-  // the section beg/end to __hwasan_init_frames() at module init time.
-  std::string createFrameString(ArrayRef<AllocaInst*> Allocas);
-  void createFrameGlobal(Function &F, const std::string &FrameString);
-  // Get the section name for frame descriptions. Currently ELF-only.
-  const char *getFrameSection() { return "__hwasan_frames"; }
-  const char *getFrameSectionBeg() { return  "__start___hwasan_frames"; }
-  const char *getFrameSectionEnd() { return  "__stop___hwasan_frames"; }
-  GlobalVariable *createFrameSectionBound(Module &M, Type *Ty,
-                                          const char *Name) {
-    auto GV = new GlobalVariable(M, Ty, false, GlobalVariable::ExternalLinkage,
-                                 nullptr, Name);
-    GV->setVisibility(GlobalValue::HiddenVisibility);
-    return GV;
-  }
-
   /// This struct defines the shadow mapping using the rule:
   ///   shadow = (mem >> Scale) + Offset.
   /// If InGlobal is true, then
   ///   extern char __hwasan_shadow[];
   ///   shadow = (mem >> Scale) + &__hwasan_shadow
   /// If InTls is true, then
   ///   extern char *__hwasan_tls;
   ///   shadow = (mem>>Scale) + align_up(__hwasan_shadow, kShadowBaseAlignment)
std::tie(HwasanCtorFunction, std::ignore) =
             /*InitArgTypes=*/{},
             /*InitArgs=*/{},
             // This callback is invoked when the functions are created the first
             // time. Hook them into the global ctors list in that case:
             [&](Function *Ctor, FunctionCallee) {
               Comdat *CtorComdat = M.getOrInsertComdat(kHwasanModuleCtorName);
               Ctor->setComdat(CtorComdat);
               appendToGlobalCtors(M, Ctor, 0, Ctor);
-
-              IRBuilder<> IRBCtor(Ctor->getEntryBlock().getTerminator());
-              IRBCtor.CreateCall(
-                  declareSanitizerInitFunction(M, "__hwasan_init_frames",
-                                               {Int8PtrTy, Int8PtrTy}),
-                  {createFrameSectionBound(M, Int8Ty, getFrameSectionBeg()),
-                   createFrameSectionBound(M, Int8Ty, getFrameSectionEnd())});
-            });
-
-    // Create a zero-length global in __hwasan_frame so that the linker will
-    // always create start and stop symbols.
-    //
-    // N.B. If we ever start creating associated metadata in this pass this
-    // global will need to be associated with the ctor.
-    Type *Int8Arr0Ty = ArrayType::get(Int8Ty, 0);
-    M.getOrInsertGlobal("__hwasan", Int8Arr0Ty, [&] {
-      auto *GV = new GlobalVariable(
-          M, Int8Arr0Ty, /*isConstantGlobal=*/true, GlobalValue::PrivateLinkage,
-          Constant::getNullValue(Int8Arr0Ty), "__hwasan");
-      GV->setSection(getFrameSection());
-      Comdat *CtorComdat = M.getOrInsertComdat(kHwasanModuleCtorName);
-      GV->setComdat(CtorComdat);
-      appendToCompilerUsed(M, GV);
-      return GV;
             });
   }
 
   if (!TargetTriple.isAndroid()) {
     Constant *C = M.getOrInsertGlobal("__hwasan_tls", IntptrTy, [&] {
       auto *GV = new GlobalVariable(M, IntptrTy, /*isConstantGlobal=*/false,
                                     GlobalValue::ExternalLinkage, nullptr,
                                     "__hwasan_tls", nullptr,
                                     GlobalVariable::InitialExecTLSModel);
Value *HWAddressSanitizer::getHwasanThreadSlotPtr(IRBuilder<> &IRB, Type *Ty) {
   }
   if (ThreadPtrGlobal)
     return ThreadPtrGlobal;
 
 
   return nullptr;
 }
 
-// Creates a string with a description of the stack frame (set of Allocas).
-// The string is intended to be human readable.
-// The current form is: Size1 Name1; Size2 Name2; ...
-std::string
-HWAddressSanitizer::createFrameString(ArrayRef<AllocaInst *> Allocas) {
-  std::ostringstream Descr;
-  for (auto AI : Allocas)
-    Descr << getAllocaSizeInBytes(*AI) << " " <<  AI->getName().str() << "; ";
-  return Descr.str();
-}
-
-// Creates a global in the frame section which consists of two pointers:
-// the function PC and the frame string constant.
-void HWAddressSanitizer::createFrameGlobal(Function &F,
-                                           const std::string &FrameString) {
-  Module &M = *F.getParent();
-  auto DescrGV = createPrivateGlobalForString(M, FrameString, true);
-  auto PtrPairTy = StructType::get(F.getType(), DescrGV->getType());
-  auto GV = new GlobalVariable(
-      M, PtrPairTy, /*isConstantGlobal*/ true, GlobalVariable::PrivateLinkage,
-      ConstantStruct::get(PtrPairTy, (Constant *)&F, (Constant *)DescrGV),
-      "__hwasan");
-  GV->setSection(getFrameSection());
-  appendToCompilerUsed(M, GV);
-  // Put GV into the F's Comadat so that if F is deleted GV can be deleted too.
-  if (auto Comdat =
-          GetOrCreateFunctionComdat(F, TargetTriple, CurModuleUniqueId))
-    GV->setComdat(Comdat);
-}
-
 void HWAddressSanitizer::emitPrologue(IRBuilder<> &IRB, bool WithFrameRecord) {
   if (!Mapping.InTls) {
     LocalDynamicShadow = getDynamicShadowNonTls(IRB);
     return;
   }
 
   if (!WithFrameRecord && TargetTriple.isAndroid()) {
     LocalDynamicShadow = getDynamicShadowIfunc(IRB);
bool HWAddressSanitizer::sanitizeFunction(Function &F) {
   initializeCallbacks(*F.getParent());
 
   if (!LandingPadVec.empty())
     instrumentLandingPads(LandingPadVec);
 
   if (AllocasToInstrument.empty() && ToInstrument.empty())
     return false;
 
-  if (ClCreateFrameDescriptions && !AllocasToInstrument.empty())
-    createFrameGlobal(F, createFrameString(AllocasToInstrument));
-
-
   assert(!LocalDynamicShadow);
 
   Instruction *InsertPt = &*F.getEntryBlock().begin();
   IRBuilder<> EntryIRB(InsertPt);
   emitPrologue(EntryIRB,
                /*WithFrameRecord*/ ClRecordStackHistory &&
                    !AllocasToInstrument.empty());
 

llvm/trunk/test/Instrumentation/HWAddressSanitizer/basic.ll

 ; Test basic address sanitizer instrumentation.
 ;
 ; RUN: opt < %s -hwasan -hwasan-recover=0 -hwasan-with-ifunc=1 -hwasan-with-tls=0 -S | FileCheck %s --check-prefixes=CHECK,ABORT,ABORT-DYNAMIC-SHADOW
 ; RUN: opt < %s -hwasan -hwasan-recover=1 -hwasan-with-ifunc=1 -hwasan-with-tls=0 -S | FileCheck %s --check-prefixes=CHECK,RECOVER,RECOVER-DYNAMIC-SHADOW
 ; RUN: opt < %s -hwasan -hwasan-recover=0 -hwasan-mapping-offset=0 -S | FileCheck %s --check-prefixes=CHECK,ABORT,ABORT-ZERO-BASED-SHADOW
 ; RUN: opt < %s -hwasan -hwasan-recover=1 -hwasan-mapping-offset=0 -S | FileCheck %s --check-prefixes=CHECK,RECOVER,RECOVER-ZERO-BASED-SHADOW
 
 ; Ensure than hwasan runs with the new PM pass
 ; RUN: opt < %s -passes='function(hwasan)' -hwasan-recover=0 -hwasan-with-ifunc=1 -hwasan-with-tls=0 -S | FileCheck %s --check-prefixes=CHECK,ABORT,ABORT-DYNAMIC-SHADOW
 ; RUN: opt < %s -passes='function(hwasan)' -hwasan-recover=1 -hwasan-with-ifunc=1 -hwasan-with-tls=0 -S | FileCheck %s --check-prefixes=CHECK,RECOVER,RECOVER-DYNAMIC-SHADOW
 ; RUN: opt < %s -passes='function(hwasan)' -hwasan-recover=0 -hwasan-mapping-offset=0 -S | FileCheck %s --check-prefixes=CHECK,ABORT,ABORT-ZERO-BASED-SHADOW
 ; RUN: opt < %s -passes='function(hwasan)' -hwasan-recover=1 -hwasan-mapping-offset=0 -S | FileCheck %s --check-prefixes=CHECK,RECOVER,RECOVER-ZERO-BASED-SHADOW
 
 ; CHECK: @llvm.global_ctors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 0, void ()* @hwasan.module_ctor, i8* bitcast (void ()* @hwasan.module_ctor to i8*) }]
-; CHECK: @__hwasan = private constant [0 x i8] zeroinitializer, section "__hwasan_frames", comdat($hwasan.module_ctor)
 
 target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
 target triple = "aarch64--linux-android"
 
 define i8 @test_load8(i8* %a) sanitize_hwaddress {
 ; CHECK-LABEL: @test_load8(
 ; RECOVER: %[[A:[^ ]*]] = ptrtoint i8* %a to i64
 ; RECOVER: %[[B:[^ ]*]] = lshr i64 %[[A]], 56
entry:
   %b = load i8, i8 addrspace(256)* %a, align 4
   ret i8 %b
 }
 
 ; CHECK: declare void @__hwasan_init()
 
 ; CHECK:      define internal void @hwasan.module_ctor() comdat {
 ; CHECK-NEXT:   call void @__hwasan_init()
-; CHECK-NEXT:   call void @__hwasan_init_frames(
 ; CHECK-NEXT:   ret void
 ; CHECK-NEXT: }

llvm/trunk/test/Instrumentation/HWAddressSanitizer/frame-descriptor.ll

-; Test frame descriptors
-;
-; RUN: opt < %s -hwasan -S | FileCheck %s
-
-target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
-target triple = "aarch64--linux-android"
-
-declare void @use32(i32*, i64*)
-
-define void @test_alloca() sanitize_hwaddress {
-entry:
-  %XYZ = alloca i32, align 4
-  %ABC = alloca i64, align 4
-  call void @use32(i32* nonnull %XYZ, i64 *nonnull %ABC)
-  ret void
-}
-
-; CHECK: @[[STR:[0-9]*]] = private unnamed_addr constant [15 x i8] c"4 XYZ; 8 ABC; \00", align 1
-; CHECK: private constant { void ()*, [15 x i8]* } { void ()* @test_alloca, [15 x i8]* @[[STR]] }, section "__hwasan_frames", comdat($test_alloca)
-
-; CHECK-LABEL: @test_alloca(
-; CHECK: ret void
-
-; CHECK-LABEL: @hwasan.module_ctor
-; CHECK: call void @__hwasan_init_frames(i8* @__start___hwasan_frames, i8* @__stop___hwasan_frames)
-; CHECK: ret void
-

llvm/trunk/test/Instrumentation/HWAddressSanitizer/with-calls.ll

entry:
   %b = load i8, i8 addrspace(256)* %a, align 4
   ret i8 %b
 }
 
 ; CHECK: declare void @__hwasan_init()
 
 ; CHECK:      define internal void @hwasan.module_ctor() comdat {
 ; CHECK-NEXT:   call void @__hwasan_init()
-; CHECK-NEXT:   call void @__hwasan_init_frames(
 ; CHECK-NEXT:   ret void
 ; CHECK-NEXT: }