[PATCH] D148654: Modify BoundsSan to improve debuggability

[Shoaib Meenai via Phabricator via cfe-commits]

smeenai added inline comments.


================
Comment at: llvm/lib/Transforms/Instrumentation/BoundsChecking.cpp:189
   auto GetTrapBB = [&TrapBB](BuilderTy &IRB) {
-    if (TrapBB && SingleTrapBB)
-      return TrapBB;
-
-    Function *Fn = IRB.GetInsertBlock()->getParent();
-    // FIXME: This debug location doesn't make a lot of sense in the
-    // `SingleTrapBB` case.
-    auto DebugLoc = IRB.getCurrentDebugLocation();
-    IRBuilder<>::InsertPointGuard Guard(IRB);
-    TrapBB = BasicBlock::Create(Fn->getContext(), "trap", Fn);
-    IRB.SetInsertPoint(TrapBB);
-
-    auto *F = Intrinsic::getDeclaration(Fn->getParent(), Intrinsic::trap);
-    CallInst *TrapCall = IRB.CreateCall(F, {});
-    TrapCall->setDoesNotReturn();
-    TrapCall->setDoesNotThrow();
-    TrapCall->setDebugLoc(DebugLoc);
-    IRB.CreateUnreachable();
-
+    if (DebugTrapBB) {
+      Function *Fn = IRB.GetInsertBlock()->getParent();
----------------
oskarwirga wrote:
> nlopes wrote:
> > this seems like code duplication. This pass already has the single-trap flag to exactly control if you get a single trap BB or one per check for better debug info.
> Unfortunately, even with the single trap flag it gets optimized out in later passes because the machine code emitted is the exact same 
I believe we end up tail merging the trap instructions. A previous iteration of this patch attempted to use the `nomerge` attribute to directly avoid the tail merging, but that only works for function calls, not for the `trap` instruction ultimately emitted here.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D148654/new/

https://reviews.llvm.org/D148654