[PATCH] D15208: Patch for inline abort code generation

Dan Austin via cfe-commits cfe-commits at lists.llvm.org
Wed Dec 9 14:04:40 PST 2015


danielaustin added a comment.

Test makes sense, will add it with the revisions.


================
Comment at: include/clang/Basic/LangOptions.h:95
@@ -94,1 +94,3 @@
 
+  /// \brief Flag controlling whether or not trap calls are merged
+  /// at the end of each function.
----------------
samsonov wrote:
> Why is it a language, not codegen option?
Wasn't aware, will relocate it there, as it makes more sense

================
Comment at: include/clang/Driver/Options.td:614
@@ +613,3 @@
+def fno_sanitize_merge_traps : Flag<["-"], "fno-sanitize-merge-traps">, 
+                               Group<f_clang_Group>,
+                               HelpText<"Generate traps for sanitizers inline to aid in debugging.">; 
----------------
samsonov wrote:
> These should probably be CC1Option as well.
Agree

================
Comment at: lib/CodeGen/CGExpr.cpp:2543
@@ +2542,3 @@
+  // RE: Bug: 25682
+  if(!getLangOpts().mergeTraps) {
+      llvm::InlineAsm *EmptyAsm = llvm::InlineAsm::get(llvm::FunctionType::get(CGM.VoidTy, false), 
----------------
samsonov wrote:
> Note that this will also affect `-ftrapv`, which might be unexpected, as we mention "sanitize" in the flag name.
Would it be better in your opinion to remove sanitize from the name, or check for the presence of the integer sanitizers here?

================
Comment at: lib/CodeGen/CGExpr.cpp:2549
@@ +2548,3 @@
+      EmitBlock(TrapBB);
+      llvm::CallInst *TrapCall = EmitTrapCall(llvm::Intrinsic::trap);
+      TrapCall->setDoesNotReturn();
----------------
samsonov wrote:
> Looks like most of this block (everything except for the empty asm statement) is duplicated below.
Yea, I can simplify that.


Repository:
  rL LLVM

http://reviews.llvm.org/D15208





More information about the cfe-commits mailing list