[PATCH] D89959: UBSAN: emit distinctive traps in trapping mode

t.p.northover closed this revision. t.p.northover added a comment. Committed: To github.com:llvm/llvm-project.git c54d827fdb12..c5978f42ec8e main -> main CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89959/new/ https://reviews.llvm.org/D89959 __

[PATCH] D89959: UBSAN: emit distinctive traps in trapping mode

t.p.northover added inline comments. Comment at: llvm/include/llvm/IR/Intrinsics.td:1242 +def int_ubsantrap : Intrinsic<[], [llvm_i8_ty], + [IntrNoReturn, IntrCold, ImmArg>]>; jdoerfert wrote: > should this be readonly and inaccesib

[PATCH] D89959: UBSAN: emit distinctive traps in trapping mode

jdoerfert added inline comments. Comment at: llvm/include/llvm/IR/Intrinsics.td:1242 +def int_ubsantrap : Intrinsic<[], [llvm_i8_ty], + [IntrNoReturn, IntrCold, ImmArg>]>; should this be readonly and inaccesiblememonly? CHANGES SI

[PATCH] D89959: UBSAN: emit distinctive traps in trapping mode

t.p.northover marked an inline comment as done. t.p.northover added a comment. > Was this measured with all of -fsanitize=undefined enabled? If so, the actual > size overhead is likely lower, as only a subset of these checks get enabled > in production settings. Yes, I used `-fsanitize=undefine

[PATCH] D89959: UBSAN: emit distinctive traps in trapping mode

t.p.northover added a comment. With current CodeGen by the time you reach the trap you have no idea what came before so I think you'd still need a separate trap instruction per failure kind. So the Clang and generic LLVM side would be unaffected. I suppose on X86 it could save a few bytes in .t

[PATCH] D89959: UBSAN: emit distinctive traps in trapping mode

kcc added a reviewer: morehouse. kcc added a comment. did you consider approaches where the emitted code doesn't change, but the binary contains a debug-like metadata that corresponds to the trap instructions? Matt (CC-ed) has a patch if this kind (for a different purpose) in the works . CHANGE

[PATCH] D89959: UBSAN: emit distinctive traps in trapping mode

vsk accepted this revision. vsk added subscribers: kcc, eugenis. vsk added a comment. This revision is now accepted and ready to land. Thanks, lgtm with two comments -- > Because of the extra traps there is a small code-size penalty, but it's > pretty small compared to what we accept just for th

[PATCH] D89959: UBSAN: emit distinctive traps in trapping mode

vsk added a comment. I might've missed it, but the debug location merging behavior could use a test. Here's one way to write it: https://godbolt.org/z/Yb9PY9. In `@_Z13keep_locationPi`, the !dbg attachment on the trap should match `!DILocation(line: 1`. In `@_Z15merge_locationsPi`, the attachme

[PATCH] D89959: UBSAN: emit distinctive traps in trapping mode

tschuett added inline comments. Comment at: llvm/docs/LangRef.rst:19494 + + declare void @llvm.ubsantrap(i32 immarg) cold noreturn nounwind + This is still i32. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89959/new/ https://reviews.llvm.org/D899

[PATCH] D89959: UBSAN: emit distinctive traps in trapping mode

vsk added inline comments. Comment at: clang/lib/CodeGen/CGExpr.cpp:3458 +llvm::CallInst *TrapCall = +Builder.CreateCall(CGM.getIntrinsic(llvm::Intrinsic::ubsantrap), + llvm::ConstantInt::get(CGM.Int32Ty, CheckHandlerID)); v

[PATCH] D89959: UBSAN: emit distinctive traps in trapping mode

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'?

[PATCH] D89959: UBSAN: emit distinctive traps in trapping mode

jkorous added a comment. Friendly ping 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-commit

[PATCH] D89959: UBSAN: emit distinctive traps in trapping mode

lebedev.ri added a comment. Missing langref changes for new intrinsic. 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