From 8455ed25f0c39e3b98e8677c4c80cef081621b7a Mon Sep 17 00:00:00 2001 From: Vitaly Buka Date: Wed, 18 Dec 2024 19:22:10 -0800 Subject: [PATCH 1/5] =?UTF-8?q?[=F0=9D=98=80=F0=9D=97=BD=F0=9D=97=BF]=20ch?= =?UTF-8?q?anges=20to=20main=20this=20commit=20is=20based=20on?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Created using spr 1.3.4 [skip ci] --- .../Instrumentation/BoundsChecking.cpp | 102 ++++++++++++++---- .../BoundsChecking/runtimes.ll | 12 +-- 2 files changed, 87 insertions(+), 27 deletions(-) diff --git a/llvm/lib/Transforms/Instrumentation/BoundsChecking.cpp b/llvm/lib/Transforms/Instrumentation/BoundsChecking.cpp index c86d967716a5a..d84f3ae73b062 100644 --- a/llvm/lib/Transforms/Instrumentation/BoundsChecking.cpp +++ b/llvm/lib/Transforms/Instrumentation/BoundsChecking.cpp @@ -8,6 +8,7 @@ #include "llvm/Transforms/Instrumentation/BoundsChecking.h" #include "llvm/ADT/Statistic.h" +#include "llvm/ADT/StringRef.h" #include "llvm/ADT/Twine.h" #include "llvm/Analysis/MemoryBuiltins.h" #include "llvm/Analysis/ScalarEvolution.h" @@ -104,6 +105,29 @@ static Value *getBoundsCheckCond(Value *Ptr, Value *InstVal, return Or; } +static CallInst *InsertTrap(BuilderTy &IRB) { + if (!DebugTrapBB) + return IRB.CreateIntrinsic(Intrinsic::trap, {}, {}); + return IRB.CreateIntrinsic( + Intrinsic::ubsantrap, {}, + ConstantInt::get(IRB.getInt8Ty(), + IRB.GetInsertBlock()->getParent()->size())); +} + +static CallInst *InsertCall(BuilderTy &IRB, bool MayReturn, StringRef Name) { + Function *Fn = IRB.GetInsertBlock()->getParent(); + LLVMContext &Ctx = Fn->getContext(); + llvm::AttrBuilder B(Ctx); + B.addAttribute(llvm::Attribute::NoUnwind); + if (!MayReturn) + B.addAttribute(llvm::Attribute::NoReturn); + FunctionCallee Callee = Fn->getParent()->getOrInsertFunction( + Name, + llvm::AttributeList::get(Ctx, llvm::AttributeList::FunctionIndex, B), + Type::getVoidTy(Ctx)); + return IRB.CreateCall(Callee); +} + /// Adds run-time bounds checks to memory accessing instructions. /// /// \p Or is the condition that should guard the trap. @@ -126,20 +150,53 @@ static void insertBoundsCheck(Value *Or, BuilderTy &IRB, GetTrapBBT GetTrapBB) { BasicBlock *Cont = OldBB->splitBasicBlock(SplitI); OldBB->getTerminator()->eraseFromParent(); + BasicBlock *TrapBB = GetTrapBB(IRB, Cont); + if (C) { // If we have a constant zero, unconditionally branch. // FIXME: We should really handle this differently to bypass the splitting // the block. - BranchInst::Create(GetTrapBB(IRB), OldBB); + BranchInst::Create(TrapBB, OldBB); return; } // Create the conditional branch. - BranchInst::Create(GetTrapBB(IRB), Cont, Or, OldBB); + BranchInst::Create(TrapBB, Cont, Or, OldBB); } +struct ReportingOpts { + bool MayReturn = false; + bool UseTrap = false; + bool MinRuntime = false; + StringRef Name; + + ReportingOpts(BoundsCheckingPass::ReportingMode Mode) { + switch (Mode) { + case BoundsCheckingPass::ReportingMode::Trap: + UseTrap = true; + break; + case BoundsCheckingPass::ReportingMode::MinRuntime: + Name = "__ubsan_handle_local_out_of_bounds_minimal"; + MinRuntime = true; + MayReturn = true; + break; + case BoundsCheckingPass::ReportingMode::MinRuntimeAbort: + Name = "__ubsan_handle_local_out_of_bounds_minimal_abort"; + MinRuntime = true; + break; + case BoundsCheckingPass::ReportingMode::FullRuntime: + Name = "__ubsan_handle_local_out_of_bounds"; + MayReturn = true; + break; + case BoundsCheckingPass::ReportingMode::FullRuntimeAbort: + Name = "__ubsan_handle_local_out_of_bounds_abort"; + break; + } + } +}; + static bool addBoundsChecking(Function &F, TargetLibraryInfo &TLI, - ScalarEvolution &SE) { + ScalarEvolution &SE, const ReportingOpts &Opts) { if (F.hasFnAttribute(Attribute::NoSanitizeBounds)) return false; @@ -180,37 +237,40 @@ static bool addBoundsChecking(Function &F, TargetLibraryInfo &TLI, // Create a trapping basic block on demand using a callback. Depending on // flags, this will either create a single block for the entire function or // will create a fresh block every time it is called. - BasicBlock *TrapBB = nullptr; - auto GetTrapBB = [&TrapBB](BuilderTy &IRB) { + BasicBlock *ReuseTrapBB = nullptr; + auto GetTrapBB = [&ReuseTrapBB, &Opts](BuilderTy &IRB, BasicBlock *Cont) { Function *Fn = IRB.GetInsertBlock()->getParent(); auto DebugLoc = IRB.getCurrentDebugLocation(); IRBuilder<>::InsertPointGuard Guard(IRB); - if (TrapBB && SingleTrapBB && !DebugTrapBB) - return TrapBB; + // Create a trapping basic block on demand using a callback. Depending on + // flags, this will either create a single block for the entire function or + // will create a fresh block every time it is called. + if (ReuseTrapBB) + return ReuseTrapBB; - TrapBB = BasicBlock::Create(Fn->getContext(), "trap", Fn); + BasicBlock *TrapBB = BasicBlock::Create(Fn->getContext(), "trap", Fn); IRB.SetInsertPoint(TrapBB); - Intrinsic::ID IntrID = DebugTrapBB ? Intrinsic::ubsantrap : Intrinsic::trap; + CallInst *TrapCall = Opts.UseTrap + ? InsertTrap(IRB) + : InsertCall(IRB, Opts.MayReturn, Opts.Name); - CallInst *TrapCall; - if (DebugTrapBB) { - TrapCall = IRB.CreateIntrinsic( - IntrID, {}, ConstantInt::get(IRB.getInt8Ty(), Fn->size())); + TrapCall->setDoesNotThrow(); + TrapCall->setDebugLoc(DebugLoc); + if (Opts.MayReturn) { + IRB.CreateBr(Cont); } else { - TrapCall = IRB.CreateIntrinsic(IntrID, {}, {}); + TrapCall->setDoesNotReturn(); + IRB.CreateUnreachable(); } - TrapCall->setDoesNotReturn(); - TrapCall->setDoesNotThrow(); - TrapCall->setDebugLoc(DebugLoc); - IRB.CreateUnreachable(); + if (!Opts.MayReturn && SingleTrapBB && !DebugTrapBB) + ReuseTrapBB = TrapBB; return TrapBB; }; - // Add the checks. for (const auto &Entry : TrapInfo) { Instruction *Inst = Entry.first; BuilderTy IRB(Inst->getParent(), BasicBlock::iterator(Inst), TargetFolder(DL)); @@ -224,7 +284,7 @@ PreservedAnalyses BoundsCheckingPass::run(Function &F, FunctionAnalysisManager & auto &TLI = AM.getResult(F); auto &SE = AM.getResult(F); - if (!addBoundsChecking(F, TLI, SE)) + if (!addBoundsChecking(F, TLI, SE, ReportingOpts(Mode))) return PreservedAnalyses::all(); return PreservedAnalyses::none(); @@ -251,4 +311,4 @@ void BoundsCheckingPass::printPipeline( OS << ""; break; } -} \ No newline at end of file +} diff --git a/llvm/test/Instrumentation/BoundsChecking/runtimes.ll b/llvm/test/Instrumentation/BoundsChecking/runtimes.ll index fd27694c155d2..357f92aca85c0 100644 --- a/llvm/test/Instrumentation/BoundsChecking/runtimes.ll +++ b/llvm/test/Instrumentation/BoundsChecking/runtimes.ll @@ -38,8 +38,8 @@ define void @f1(i64 %x) nounwind { ; RT-NEXT: [[TMP8:%.*]] = load i128, ptr [[TMP2]], align 4 ; RT-NEXT: ret void ; RT: [[TRAP]]: -; RT-NEXT: call void @llvm.trap() #[[ATTR2:[0-9]+]] -; RT-NEXT: unreachable +; RT-NEXT: call void @__ubsan_handle_local_out_of_bounds() #[[ATTR0]] +; RT-NEXT: br label %[[BB7]] ; ; RTABORT-LABEL: define void @f1( ; RTABORT-SAME: i64 [[X:%.*]]) #[[ATTR0:[0-9]+]] { @@ -54,7 +54,7 @@ define void @f1(i64 %x) nounwind { ; RTABORT-NEXT: [[TMP8:%.*]] = load i128, ptr [[TMP2]], align 4 ; RTABORT-NEXT: ret void ; RTABORT: [[TRAP]]: -; RTABORT-NEXT: call void @llvm.trap() #[[ATTR2:[0-9]+]] +; RTABORT-NEXT: call void @__ubsan_handle_local_out_of_bounds_abort() #[[ATTR1:[0-9]+]] ; RTABORT-NEXT: unreachable ; ; MINRT-LABEL: define void @f1( @@ -70,8 +70,8 @@ define void @f1(i64 %x) nounwind { ; MINRT-NEXT: [[TMP8:%.*]] = load i128, ptr [[TMP2]], align 4 ; MINRT-NEXT: ret void ; MINRT: [[TRAP]]: -; MINRT-NEXT: call void @llvm.trap() #[[ATTR2:[0-9]+]] -; MINRT-NEXT: unreachable +; MINRT-NEXT: call void @__ubsan_handle_local_out_of_bounds_minimal() #[[ATTR0]] +; MINRT-NEXT: br label %[[BB7]] ; ; MINRTABORT-LABEL: define void @f1( ; MINRTABORT-SAME: i64 [[X:%.*]]) #[[ATTR0:[0-9]+]] { @@ -86,7 +86,7 @@ define void @f1(i64 %x) nounwind { ; MINRTABORT-NEXT: [[TMP8:%.*]] = load i128, ptr [[TMP2]], align 4 ; MINRTABORT-NEXT: ret void ; MINRTABORT: [[TRAP]]: -; MINRTABORT-NEXT: call void @llvm.trap() #[[ATTR2:[0-9]+]] +; MINRTABORT-NEXT: call void @__ubsan_handle_local_out_of_bounds_minimal_abort() #[[ATTR1:[0-9]+]] ; MINRTABORT-NEXT: unreachable ; %1 = alloca i128, i64 %x From 7492c844abc726abe8e2814661dca1af68fd70cf Mon Sep 17 00:00:00 2001 From: Vitaly Buka Date: Wed, 18 Dec 2024 19:25:18 -0800 Subject: [PATCH 2/5] hwaddress Created using spr 1.3.4 --- compiler-rt/test/ubsan/TestCases/Misc/local_bounds.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/compiler-rt/test/ubsan/TestCases/Misc/local_bounds.cpp b/compiler-rt/test/ubsan/TestCases/Misc/local_bounds.cpp index 0ff264bbd6d5b..cea9afca201af 100644 --- a/compiler-rt/test/ubsan/TestCases/Misc/local_bounds.cpp +++ b/compiler-rt/test/ubsan/TestCases/Misc/local_bounds.cpp @@ -15,7 +15,8 @@ __attribute__((noinline)) void init(S *s) { __asm__ __volatile__("" : : "r"(s) : "memory"); } -__attribute__((noinline, no_sanitize("memory", "address"))) int test(char i) { +__attribute__((noinline, no_sanitize("memory", "address", "hwaddress"))) int +test(char i) { S a; init(&a); S b; From a227fdc13f0f549f9eb21ff6ef20ce334428ce05 Mon Sep 17 00:00:00 2001 From: Vitaly Buka Date: Thu, 19 Dec 2024 16:17:10 -0800 Subject: [PATCH 3/5] =?UTF-8?q?[=F0=9D=98=80=F0=9D=97=BD=F0=9D=97=BF]=20ch?= =?UTF-8?q?anges=20introduced=20through=20rebase?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Created using spr 1.3.4 [skip ci] --- llvm/lib/Transforms/Instrumentation/BoundsChecking.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/llvm/lib/Transforms/Instrumentation/BoundsChecking.cpp b/llvm/lib/Transforms/Instrumentation/BoundsChecking.cpp index 10bf674db6808..f639d0628d605 100644 --- a/llvm/lib/Transforms/Instrumentation/BoundsChecking.cpp +++ b/llvm/lib/Transforms/Instrumentation/BoundsChecking.cpp @@ -108,6 +108,7 @@ static Value *getBoundsCheckCond(Value *Ptr, Value *InstVal, static CallInst *InsertTrap(BuilderTy &IRB) { if (!DebugTrapBB) return IRB.CreateIntrinsic(Intrinsic::trap, {}, {}); + // FIXME: Ideally we would use the SanitizerHandler::OutOfBounds constant. return IRB.CreateIntrinsic( Intrinsic::ubsantrap, {}, ConstantInt::get(IRB.getInt8Ty(), @@ -255,8 +256,10 @@ static bool addBoundsChecking(Function &F, TargetLibraryInfo &TLI, CallInst *TrapCall = Opts.UseTrap ? InsertTrap(IRB) : InsertCall(IRB, Opts.MayReturn, Opts.Name); - // Ideally we would use the SanitizerHandler::OutOfBounds constant + if (DebugTrapBB) { + // FIXME: Pass option form clang. TrapCall->addFnAttr(llvm::Attribute::NoMerge); + } TrapCall->setDoesNotThrow(); TrapCall->setDebugLoc(DebugLoc); From 10eeaeaaa19da4b1b4d270eafe590bc02a28648e Mon Sep 17 00:00:00 2001 From: Vitaly Buka Date: Thu, 19 Dec 2024 16:19:57 -0800 Subject: [PATCH 4/5] =?UTF-8?q?[=F0=9D=98=80=F0=9D=97=BD=F0=9D=97=BF]=20ch?= =?UTF-8?q?anges=20introduced=20through=20rebase?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Created using spr 1.3.4 [skip ci] --- llvm/test/Instrumentation/BoundsChecking/runtimes.ll | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/llvm/test/Instrumentation/BoundsChecking/runtimes.ll b/llvm/test/Instrumentation/BoundsChecking/runtimes.ll index 99aa99c86b5ca..357f92aca85c0 100644 --- a/llvm/test/Instrumentation/BoundsChecking/runtimes.ll +++ b/llvm/test/Instrumentation/BoundsChecking/runtimes.ll @@ -38,7 +38,7 @@ define void @f1(i64 %x) nounwind { ; RT-NEXT: [[TMP8:%.*]] = load i128, ptr [[TMP2]], align 4 ; RT-NEXT: ret void ; RT: [[TRAP]]: -; RT-NEXT: call void @__ubsan_handle_local_out_of_bounds() #[[ATTR1:[0-9]+]] +; RT-NEXT: call void @__ubsan_handle_local_out_of_bounds() #[[ATTR0]] ; RT-NEXT: br label %[[BB7]] ; ; RTABORT-LABEL: define void @f1( @@ -54,7 +54,7 @@ define void @f1(i64 %x) nounwind { ; RTABORT-NEXT: [[TMP8:%.*]] = load i128, ptr [[TMP2]], align 4 ; RTABORT-NEXT: ret void ; RTABORT: [[TRAP]]: -; RTABORT-NEXT: call void @__ubsan_handle_local_out_of_bounds_abort() #[[ATTR2:[0-9]+]] +; RTABORT-NEXT: call void @__ubsan_handle_local_out_of_bounds_abort() #[[ATTR1:[0-9]+]] ; RTABORT-NEXT: unreachable ; ; MINRT-LABEL: define void @f1( @@ -70,7 +70,7 @@ define void @f1(i64 %x) nounwind { ; MINRT-NEXT: [[TMP8:%.*]] = load i128, ptr [[TMP2]], align 4 ; MINRT-NEXT: ret void ; MINRT: [[TRAP]]: -; MINRT-NEXT: call void @__ubsan_handle_local_out_of_bounds_minimal() #[[ATTR1:[0-9]+]] +; MINRT-NEXT: call void @__ubsan_handle_local_out_of_bounds_minimal() #[[ATTR0]] ; MINRT-NEXT: br label %[[BB7]] ; ; MINRTABORT-LABEL: define void @f1( @@ -86,7 +86,7 @@ define void @f1(i64 %x) nounwind { ; MINRTABORT-NEXT: [[TMP8:%.*]] = load i128, ptr [[TMP2]], align 4 ; MINRTABORT-NEXT: ret void ; MINRTABORT: [[TRAP]]: -; MINRTABORT-NEXT: call void @__ubsan_handle_local_out_of_bounds_minimal_abort() #[[ATTR2:[0-9]+]] +; MINRTABORT-NEXT: call void @__ubsan_handle_local_out_of_bounds_minimal_abort() #[[ATTR1:[0-9]+]] ; MINRTABORT-NEXT: unreachable ; %1 = alloca i128, i64 %x From cc2133b80f76cfec23c23715a9be7e6c8c403550 Mon Sep 17 00:00:00 2001 From: Vitaly Buka Date: Thu, 19 Dec 2024 16:29:40 -0800 Subject: [PATCH 5/5] LINE Created using spr 1.3.4 --- compiler-rt/test/ubsan/TestCases/Misc/local_bounds.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler-rt/test/ubsan/TestCases/Misc/local_bounds.cpp b/compiler-rt/test/ubsan/TestCases/Misc/local_bounds.cpp index cea9afca201af..d5e0b46a0f8be 100644 --- a/compiler-rt/test/ubsan/TestCases/Misc/local_bounds.cpp +++ b/compiler-rt/test/ubsan/TestCases/Misc/local_bounds.cpp @@ -24,8 +24,8 @@ test(char i) { return ((int *)(&a))[i]; // CHECK: error: access out of bounds // CHECK: SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior - // LINE: local_bounds.cpp:[[@LINE-3]]:{{.*}}runtime error: access out of bounds - // LINE: SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior {{.*}}local_bounds.cpp:[[@LINE-4]]: + // LINE: local_bounds.cpp:[[#@LINE-3]]:{{.*}}runtime error: access out of bounds + // LINE: SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior {{.*}}local_bounds.cpp:[[#@LINE-4]]: } int main(int argc, char **argv) {