vsk added inline comments.

================
Comment at: clang/lib/CodeGen/CGExpr.cpp:3447
   // If we're optimizing, collapse all calls to trap down to just one per
   // function to save on code size.
+  if (TrapBBs.size() <= CheckHandlerID)
----------------
'per check, per function'?


================
Comment at: clang/lib/CodeGen/CGExpr.cpp:3458
+    llvm::CallInst *TrapCall =
+        Builder.CreateCall(CGM.getIntrinsic(llvm::Intrinsic::ubsantrap),
+                           llvm::ConstantInt::get(CGM.Int32Ty, 
CheckHandlerID));
----------------
This seems to apply the current DebugLoc from Builder to the shared trap call 
when optimizing. That's potentially misleading (say you have two trapping 
additions -- if the second one traps, the crashlog will make it look like the 
first one trapped).

I think the fix is just: `if (optimizing) TrapCall->dropLocation();`. This can 
be fixed before/after/in this patch, whatever you prefer.


================
Comment at: llvm/lib/Target/AArch64/AArch64InstrInfo.td:6702
 
+def ubsan_trap_xform : SDNodeXForm<timm, [{
+  return CurDAG->getTargetConstant(N->getZExtValue() | ('U' << 8), SDLoc(N), 
MVT::i32);
----------------
assert(N->getZExtValue() < 256)? Or alternatively, maybe it'd be simpler to 
define `@llvm.ubsantrap(i8 immarg)`?


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

https://reviews.llvm.org/D89959

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to