[compiler-rt] d843d5c - Revert "[hwasan] Add __hwasan_record_frame_record to the hwasan interface"

Leonard Chan via llvm-commits llvm-commits at lists.llvm.org
Wed Jul 13 15:06:38 PDT 2022


Author: Leonard Chan
Date: 2022-07-13T15:06:07-07:00
New Revision: d843d5c8e6c9e16a8ff18cae3ff0aea3c4bace49

URL: https://github.com/llvm/llvm-project/commit/d843d5c8e6c9e16a8ff18cae3ff0aea3c4bace49
DIFF: https://github.com/llvm/llvm-project/commit/d843d5c8e6c9e16a8ff18cae3ff0aea3c4bace49.diff

LOG: Revert "[hwasan] Add __hwasan_record_frame_record to the hwasan interface"

This reverts commit 4956620387ee45a48a394853a47ddd65195c4cc6.

This broke a sanitizer builder: https://lab.llvm.org/buildbot/#/builders/77/builds/19597

Added: 
    

Modified: 
    compiler-rt/lib/hwasan/hwasan.cpp
    compiler-rt/lib/hwasan/hwasan_interface_internal.h
    compiler-rt/test/hwasan/TestCases/deep-recursion.c
    compiler-rt/test/hwasan/TestCases/stack-history-length.c
    compiler-rt/test/hwasan/TestCases/stack-uar.c
    compiler-rt/test/hwasan/TestCases/stack-uas.c
    llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
    llvm/test/Instrumentation/HWAddressSanitizer/prologue.ll

Removed: 
    


################################################################################
diff  --git a/compiler-rt/lib/hwasan/hwasan.cpp b/compiler-rt/lib/hwasan/hwasan.cpp
index b771025cb93d3..f8725a1734326 100644
--- a/compiler-rt/lib/hwasan/hwasan.cpp
+++ b/compiler-rt/lib/hwasan/hwasan.cpp
@@ -576,12 +576,6 @@ u8 __hwasan_generate_tag() {
   return t->GenerateRandomTag();
 }
 
-void __hwasan_add_frame_record(u64 frame_record_info) {
-  Thread *t = GetCurrentThread();
-  if (t)
-    t->stack_allocations()->push(frame_record_info);
-}
-
 #if !SANITIZER_SUPPORTS_WEAK_HOOKS
 extern "C" {
 SANITIZER_INTERFACE_ATTRIBUTE SANITIZER_WEAK_ATTRIBUTE

diff  --git a/compiler-rt/lib/hwasan/hwasan_interface_internal.h b/compiler-rt/lib/hwasan/hwasan_interface_internal.h
index d1ecbb592a219..ef771add411c9 100644
--- a/compiler-rt/lib/hwasan/hwasan_interface_internal.h
+++ b/compiler-rt/lib/hwasan/hwasan_interface_internal.h
@@ -168,14 +168,6 @@ void __hwasan_thread_exit();
 SANITIZER_INTERFACE_ATTRIBUTE
 void __hwasan_print_memory_usage();
 
-// The compiler will generate this when
-// `-hwasan-record-stack-history-with-calls` is added as a flag, which will add
-// frame record information to the stack ring buffer. This is an alternative to
-// the compiler emitting instructions in the prologue for doing the same thing
-// by accessing the ring buffer directly.
-SANITIZER_INTERFACE_ATTRIBUTE
-void __hwasan_add_frame_record(u64 frame_record_info);
-
 SANITIZER_INTERFACE_ATTRIBUTE
 void *__hwasan_memcpy(void *dst, const void *src, uptr size);
 SANITIZER_INTERFACE_ATTRIBUTE

diff  --git a/compiler-rt/test/hwasan/TestCases/deep-recursion.c b/compiler-rt/test/hwasan/TestCases/deep-recursion.c
index 373c986c8b290..d29c5fbb951d6 100644
--- a/compiler-rt/test/hwasan/TestCases/deep-recursion.c
+++ b/compiler-rt/test/hwasan/TestCases/deep-recursion.c
@@ -5,15 +5,6 @@
 // RUN: %env_hwasan_opts=stack_history_size=5 not %run %t 2>&1 | FileCheck %s --check-prefix=D5
 // RUN:                                       not %run %t 2>&1 | FileCheck %s --check-prefix=DEFAULT
 
-// Run the same tests as above, but using the __hwasan_add_frame_record libcall.
-// The output should be the exact same.
-// RUN: %clang_hwasan -O1 %s -o %t -mllvm -hwasan-record-stack-history=libcall
-// RUN: %env_hwasan_opts=stack_history_size=1 not %run %t 2>&1 | FileCheck %s --check-prefix=D1
-// RUN: %env_hwasan_opts=stack_history_size=2 not %run %t 2>&1 | FileCheck %s --check-prefix=D2
-// RUN: %env_hwasan_opts=stack_history_size=3 not %run %t 2>&1 | FileCheck %s --check-prefix=D3
-// RUN: %env_hwasan_opts=stack_history_size=5 not %run %t 2>&1 | FileCheck %s --check-prefix=D5
-// RUN:                                       not %run %t 2>&1 | FileCheck %s --check-prefix=DEFAULT
-
 // REQUIRES: stable-runtime
 
 // Stack histories are currently not recorded on x86.

diff  --git a/compiler-rt/test/hwasan/TestCases/stack-history-length.c b/compiler-rt/test/hwasan/TestCases/stack-history-length.c
index 2434299a84a41..510a9afeae5aa 100644
--- a/compiler-rt/test/hwasan/TestCases/stack-history-length.c
+++ b/compiler-rt/test/hwasan/TestCases/stack-history-length.c
@@ -2,12 +2,6 @@
 // RUN: %env_hwasan_opts=stack_history_size=2048 not %run %t 2045 2>&1 | FileCheck %s --check-prefix=YES
 // RUN: %env_hwasan_opts=stack_history_size=2048 not %run %t 2047 2>&1 | FileCheck %s --check-prefix=NO
 
-// Run the same tests as above, but using the __hwasan_add_frame_record libcall.
-// The output should be the exact same.
-// RUN: %clang_hwasan -O1 %s -o %t -mllvm -hwasan-record-stack-history=libcall
-// RUN: %env_hwasan_opts=stack_history_size=2048 not %run %t 2045 2>&1 | FileCheck %s --check-prefix=YES
-// RUN: %env_hwasan_opts=stack_history_size=2048 not %run %t 2047 2>&1 | FileCheck %s --check-prefix=NO
-
 // REQUIRES: stable-runtime
 
 // Stack histories are currently not recorded on x86.

diff  --git a/compiler-rt/test/hwasan/TestCases/stack-uar.c b/compiler-rt/test/hwasan/TestCases/stack-uar.c
index 3663eac5d2685..9a81dfc3d6bce 100644
--- a/compiler-rt/test/hwasan/TestCases/stack-uar.c
+++ b/compiler-rt/test/hwasan/TestCases/stack-uar.c
@@ -2,10 +2,6 @@
 // RUN: %clang_hwasan -g %s -o %t && not %run %t 2>&1 | FileCheck %s
 // RUN: %clang_hwasan -g %s -o %t && not %env_hwasan_opts=symbolize=0 %run %t 2>&1 | FileCheck %s --check-prefix=NOSYM
 
-// Run the same test as above, but using the __hwasan_add_frame_record libcall.
-// The output should be the exact same.
-// RUN: %clang_hwasan -g %s -o %t -mllvm -hwasan-record-stack-history=libcall && not %env_hwasan_opts=symbolize=0 %run %t 2>&1 | FileCheck %s --check-prefix=NOSYM
-
 // REQUIRES: stable-runtime
 
 // Stack histories currently are not recorded on x86.

diff  --git a/compiler-rt/test/hwasan/TestCases/stack-uas.c b/compiler-rt/test/hwasan/TestCases/stack-uas.c
index 7f5a6f26d0675..2936c28e26778 100644
--- a/compiler-rt/test/hwasan/TestCases/stack-uas.c
+++ b/compiler-rt/test/hwasan/TestCases/stack-uas.c
@@ -8,10 +8,6 @@
 
 // RUN: %clang_hwasan -mllvm -hwasan-use-after-scope -g %s -o %t && not %run %t 2>&1 | FileCheck %s
 
-// Run the same test as above, but using the __hwasan_add_frame_record libcall.
-// The output should be the exact same.
-// RUN: %clang_hwasan -mllvm -hwasan-use-after-scope -mllvm -hwasan-record-stack-history=libcall -g %s -o %t && not %env_hwasan_opts=symbolize=0 %run %t 2>&1 | FileCheck %s --check-prefix=NOSYM
-
 // REQUIRES: stable-runtime
 
 // Stack histories currently are not recorded on x86.

diff  --git a/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp b/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
index 8366ac1278c89..c3a1d61d797e9 100644
--- a/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
@@ -180,31 +180,11 @@ static cl::opt<bool> ClWithTls(
              "platforms that support this"),
     cl::Hidden, cl::init(true));
 
-// Mode for selecting how to insert frame record info into the stack ring
-// buffer.
-enum RecordStackHistoryMode {
-  // Do not record frame record info.
-  none,
-
-  // Insert instructions into the prologue for storing into the stack ring
-  // buffer directly.
-  instr,
-
-  // Add a call to __hwasan_add_frame_record in the runtime.
-  libcall,
-};
-
-static cl::opt<RecordStackHistoryMode> ClRecordStackHistory(
-    "hwasan-record-stack-history",
-    cl::desc("Record stack frames with tagged allocations in a thread-local "
-             "ring buffer"),
-    cl::values(clEnumVal(none, "Do not record stack ring history"),
-               clEnumVal(instr, "Insert instructions into the prologue for "
-                                "storing into the stack ring buffer directly"),
-               clEnumVal(libcall, "Add a call to __hwasan_add_frame_record for "
-                                  "storing into the stack ring buffer")),
-    cl::Hidden, cl::init(instr));
-
+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>
     ClInstrumentMemIntrinsics("hwasan-instrument-mem-intrinsics",
                               cl::desc("instrument memory intrinsics"),
@@ -399,7 +379,6 @@ class HWAddressSanitizer {
 
   FunctionCallee HwasanTagMemoryFunc;
   FunctionCallee HwasanGenerateTagFunc;
-  FunctionCallee HwasanRecordFrameRecordFunc;
 
   Constant *ShadowGlobal;
 
@@ -651,9 +630,6 @@ void HWAddressSanitizer::initializeCallbacks(Module &M) {
   HwasanGenerateTagFunc =
       M.getOrInsertFunction("__hwasan_generate_tag", Int8Ty);
 
-  HwasanRecordFrameRecordFunc = M.getOrInsertFunction(
-      "__hwasan_record_frame_record", IRB.getVoidTy(), Int64Ty);
-
   ShadowGlobal = M.getOrInsertGlobal("__hwasan_shadow",
                                      ArrayType::get(IRB.getInt8Ty(), 0));
 
@@ -1181,67 +1157,39 @@ void HWAddressSanitizer::emitPrologue(IRBuilder<> &IRB, bool WithFrameRecord) {
   if (!WithFrameRecord && ShadowBase)
     return;
 
-  Value *SlotPtr = nullptr;
-  Value *ThreadLong = nullptr;
-  Value *ThreadLongMaybeUntagged = nullptr;
-
-  auto getThreadLongMaybeUntagged = [&]() {
-    if (!SlotPtr)
-      SlotPtr = getHwasanThreadSlotPtr(IRB, IntptrTy);
-    if (!ThreadLong)
-      ThreadLong = IRB.CreateLoad(IntptrTy, SlotPtr);
-    // Extract the address field from ThreadLong. Unnecessary on AArch64 with
-    // TBI.
-    return TargetTriple.isAArch64() ? ThreadLong
-                                    : untagPointer(IRB, ThreadLong);
-  };
+  Value *SlotPtr = getHwasanThreadSlotPtr(IRB, IntptrTy);
+  assert(SlotPtr);
+
+  Value *ThreadLong = IRB.CreateLoad(IntptrTy, SlotPtr);
+  // Extract the address field from ThreadLong. Unnecessary on AArch64 with TBI.
+  Value *ThreadLongMaybeUntagged =
+      TargetTriple.isAArch64() ? ThreadLong : untagPointer(IRB, ThreadLong);
 
   if (WithFrameRecord) {
-    switch (ClRecordStackHistory) {
-    case libcall: {
-      // Emit a runtime call into hwasan rather than emitting instructions for
-      // recording stack history.
-      Value *FrameRecordInfo = getFrameRecordInfo(IRB);
-      IRB.CreateCall(HwasanRecordFrameRecordFunc, {FrameRecordInfo});
-      break;
-    }
-    case instr: {
-      ThreadLongMaybeUntagged = getThreadLongMaybeUntagged();
-
-      StackBaseTag = IRB.CreateAShr(ThreadLong, 3);
-
-      // Store data to ring buffer.
-      Value *FrameRecordInfo = getFrameRecordInfo(IRB);
-      Value *RecordPtr = IRB.CreateIntToPtr(ThreadLongMaybeUntagged,
-                                            IntptrTy->getPointerTo(0));
-      IRB.CreateStore(FrameRecordInfo, RecordPtr);
-
-      // Update the ring buffer. Top byte of ThreadLong defines the size of the
-      // buffer in pages, it must be a power of two, and the start of the buffer
-      // must be aligned by twice that much. Therefore wrap around of the ring
-      // buffer is simply Addr &= ~((ThreadLong >> 56) << 12).
-      // The use of AShr instead of LShr is due to
-      //   https://bugs.llvm.org/show_bug.cgi?id=39030
-      // Runtime library makes sure not to use the highest bit.
-      Value *WrapMask = IRB.CreateXor(
-          IRB.CreateShl(IRB.CreateAShr(ThreadLong, 56), 12, "", true, true),
-          ConstantInt::get(IntptrTy, (uint64_t)-1));
-      Value *ThreadLongNew = IRB.CreateAnd(
-          IRB.CreateAdd(ThreadLong, ConstantInt::get(IntptrTy, 8)), WrapMask);
-      IRB.CreateStore(ThreadLongNew, SlotPtr);
-      break;
-    }
-    case none: {
-      llvm_unreachable(
-          "A stack history recording mode should've been selected.");
-    }
-    }
+    StackBaseTag = IRB.CreateAShr(ThreadLong, 3);
+
+    // Store data to ring buffer.
+    Value *FrameRecordInfo = getFrameRecordInfo(IRB);
+    Value *RecordPtr =
+        IRB.CreateIntToPtr(ThreadLongMaybeUntagged, IntptrTy->getPointerTo(0));
+    IRB.CreateStore(FrameRecordInfo, RecordPtr);
+
+    // Update the ring buffer. Top byte of ThreadLong defines the size of the
+    // buffer in pages, it must be a power of two, and the start of the buffer
+    // must be aligned by twice that much. Therefore wrap around of the ring
+    // buffer is simply Addr &= ~((ThreadLong >> 56) << 12).
+    // The use of AShr instead of LShr is due to
+    //   https://bugs.llvm.org/show_bug.cgi?id=39030
+    // Runtime library makes sure not to use the highest bit.
+    Value *WrapMask = IRB.CreateXor(
+        IRB.CreateShl(IRB.CreateAShr(ThreadLong, 56), 12, "", true, true),
+        ConstantInt::get(IntptrTy, (uint64_t)-1));
+    Value *ThreadLongNew = IRB.CreateAnd(
+        IRB.CreateAdd(ThreadLong, ConstantInt::get(IntptrTy, 8)), WrapMask);
+    IRB.CreateStore(ThreadLongNew, SlotPtr);
   }
 
   if (!ShadowBase) {
-    if (!ThreadLongMaybeUntagged)
-      ThreadLongMaybeUntagged = getThreadLongMaybeUntagged();
-
     // Get shadow base address by aligning RecordPtr up.
     // Note: this is not correct if the pointer is already aligned.
     // Runtime library will make sure this never happens.
@@ -1465,7 +1413,7 @@ bool HWAddressSanitizer::sanitizeFunction(Function &F,
   Instruction *InsertPt = &*F.getEntryBlock().begin();
   IRBuilder<> EntryIRB(InsertPt);
   emitPrologue(EntryIRB,
-               /*WithFrameRecord*/ ClRecordStackHistory != none &&
+               /*WithFrameRecord*/ ClRecordStackHistory &&
                    Mapping.WithFrameRecord &&
                    !SInfo.AllocasToInstrument.empty());
 

diff  --git a/llvm/test/Instrumentation/HWAddressSanitizer/prologue.ll b/llvm/test/Instrumentation/HWAddressSanitizer/prologue.ll
index a33bfdfa6865f..703f946708f5d 100644
--- a/llvm/test/Instrumentation/HWAddressSanitizer/prologue.ll
+++ b/llvm/test/Instrumentation/HWAddressSanitizer/prologue.ll
@@ -1,19 +1,17 @@
 ; Test -hwasan-with-ifunc flag.
 ;
 ; RUN: opt -passes=hwasan -S < %s | \
-; RUN:     FileCheck %s --check-prefixes=CHECK,CHECK-NOGLOBAL,CHECK-TLS-SLOT,CHECK-HISTORY,CHECK-HISTORY-TLS-SLOT,CHECK-HISTORY-TLS
-; RUN: opt -passes=hwasan -S -hwasan-with-ifunc=0 -hwasan-with-tls=1 -hwasan-record-stack-history=instr < %s | \
-; RUN:     FileCheck %s --check-prefixes=CHECK,CHECK-NOGLOBAL,CHECK-TLS-SLOT,CHECK-HISTORY,CHECK-HISTORY-TLS-SLOT,CHECK-HISTORY-TLS
-; RUN: opt -passes=hwasan -S -hwasan-with-ifunc=0 -hwasan-with-tls=1 -hwasan-record-stack-history=none < %s | \
+; RUN:     FileCheck %s --check-prefixes=CHECK,CHECK-NOGLOBAL,CHECK-TLS-SLOT,CHECK-HISTORY,CHECK-HISTORY-TLS-SLOT
+; RUN: opt -passes=hwasan -S -hwasan-with-ifunc=0 -hwasan-with-tls=1 -hwasan-record-stack-history=1 < %s | \
+; RUN:     FileCheck %s --check-prefixes=CHECK,CHECK-NOGLOBAL,CHECK-TLS-SLOT,CHECK-HISTORY,CHECK-HISTORY-TLS-SLOT
+; RUN: opt -passes=hwasan -S -hwasan-with-ifunc=0 -hwasan-with-tls=1 -hwasan-record-stack-history=0 < %s | \
 ; RUN:     FileCheck %s --check-prefixes=CHECK,CHECK-NOGLOBAL,CHECK-IFUNC,CHECK-NOHISTORY
 ; RUN: opt -passes=hwasan -S -hwasan-with-ifunc=0 -hwasan-with-tls=0 < %s | \
 ; RUN:     FileCheck %s --check-prefixes=CHECK,CHECK-GLOBAL,CHECK-NOHISTORY
 ; RUN: opt -passes=hwasan -S -hwasan-with-ifunc=1  -hwasan-with-tls=0 < %s | \
 ; RUN:     FileCheck %s --check-prefixes=CHECK,CHECK-IFUNC,CHECK-NOHISTORY
 ; RUN: opt -passes=hwasan -S -mtriple=aarch64-fuchsia < %s | \
-; RUN:     FileCheck %s --check-prefixes=CHECK,CHECK-ZERO-OFFSET,CHECK-SHORT-GRANULES,CHECK-HISTORY,CHECK-HWASAN-TLS,CHECK-HISTORY-HWASAN-TLS,CHECK-HISTORY-TLS
-; RUN: opt -passes=hwasan -S -mtriple=aarch64-fuchsia -hwasan-record-stack-history=libcall < %s | \
-; RUN:     FileCheck %s --check-prefixes=CHECK,CHECK-HISTORY,CHECK-HISTORY-LIBCALL
+; RUN:     FileCheck %s --check-prefixes=CHECK,CHECK-ZERO-OFFSET,CHECK-SHORT-GRANULES,CHECK-HISTORY,CHECK-HWASAN-TLS,CHECK-HISTORY-HWASAN-TLS
 
 target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
 target triple = "aarch64--linux-android22"
@@ -68,28 +66,20 @@ define void @test_alloca() sanitize_hwaddress {
 
 ; CHECK-NOHISTORY-NOT: store i64
 
-; When watching stack history, all code paths attempt to get PC and SP and mix them together.
-; CHECK-HISTORY: %[[PC:[^ ]*]] = call i64 @llvm.read_register.i64(metadata [[MD:![0-9]*]])
-; CHECK-HISTORY: %[[SP0:[^ ]*]] = call i8* @llvm.frameaddress.p0i8(i32 0)
-; CHECK-HISTORY: %[[SP1:[^ ]*]] = ptrtoint i8* %[[SP0]] to i64
-; CHECK-HISTORY: %[[SP2:[^ ]*]] = shl i64 %[[SP1]], 44
-
-; CHECK-HISTORY: %[[MIX:[^ ]*]] = or i64 %[[PC]], %[[SP2]]
-; CHECK-HISTORY-TLS: %[[PTR:[^ ]*]] = inttoptr i64 %[[D]] to i64*
-; CHECK-HISTORY-TLS: store i64 %[[MIX]], i64* %[[PTR]]
-; CHECK-HISTORY-TLS: %[[D1:[^ ]*]] = ashr i64 %[[D]], 56
-; CHECK-HISTORY-TLS: %[[D2:[^ ]*]] = shl nuw nsw i64 %[[D1]], 12
-; CHECK-HISTORY-TLS: %[[D3:[^ ]*]] = xor i64 %[[D2]], -1
-; CHECK-HISTORY-TLS: %[[D4:[^ ]*]] = add i64 %[[D]], 8
-; CHECK-HISTORY-TLS: %[[D5:[^ ]*]] = and i64 %[[D4]], %[[D3]]
+; CHECK-HISTORY: call i64 @llvm.read_register.i64(metadata [[MD:![0-9]*]])
+; CHECK-HISTORY: %[[PTR:[^ ]*]] = inttoptr i64 %[[D]] to i64*
+; CHECK-HISTORY: store i64 %{{.*}}, i64* %[[PTR]]
+; CHECK-HISTORY: %[[D1:[^ ]*]] = ashr i64 %[[D]], 56
+; CHECK-HISTORY: %[[D2:[^ ]*]] = shl nuw nsw i64 %[[D1]], 12
+; CHECK-HISTORY: %[[D3:[^ ]*]] = xor i64 %[[D2]], -1
+; CHECK-HISTORY: %[[D4:[^ ]*]] = add i64 %[[D]], 8
+; CHECK-HISTORY: %[[D5:[^ ]*]] = and i64 %[[D4]], %[[D3]]
 ; CHECK-HISTORY-TLS-SLOT: store i64 %[[D5]], i64* %[[C]]
 ; CHECK-HISTORY-HWASAN-TLS: store i64 %[[D5]], i64* @__hwasan_tls
-; CHECK-HISTORY-LIBCALL: call void @__hwasan_record_frame_record(i64 %[[MIX]])
 
 ; CHECK-TLS:   %[[F:[^ ]*]] = or i64 %[[D]], 4294967295
 ; CHECK-TLS:   = add i64 %[[F]], 1
 
-; CHECK-HISTORY-LIBCALL: %[[E:hwasan.stack.base.tag]] = xor
 ; CHECK-HISTORY: = xor i64 %[[E]], 0
 
 ; CHECK-NOHISTORY-NOT: store i64


        


More information about the llvm-commits mailing list