[PATCH] D12181: [sanitizer] Add -fsanitize-trap-function.

Josh Gao via cfe-commits cfe-commits at lists.llvm.org
Thu Aug 20 14:50:41 PDT 2015


jmgao marked 8 inline comments as done.

================
Comment at: tools/clang/lib/CodeGen/CGExpr.cpp:2388
@@ +2387,3 @@
+  }
+  return EmitTrapCheck(Checked);
+}
----------------
samsonov wrote:
> This is confusing. So, you have the following behavior whenever you need to emit a check for `-fsanitize-trap=foo`:
>   # Emit a call to `-fsanitize-trap-function`, if it's specified
>   # Otherwise, emit a call to `-ftrap-function`, if it's specified
>   # Otherwise, emit a trap instruction.
> 
> Do you really want it? Don't you need to skip (2) instead? If you do, you should carefully document it.
> 
> 
My interpretation of the current `-ftrap-function` help text ("Issue call to specified function rather than a trap instruction") is that if `-ftrap-function` is specified, clang should never emit a trap instruction if defined. Does this seem reasonable?

I added a blurb to UsersManual.rst saying that it falls back to `-ftrap-function` if not specified.

================
Comment at: tools/clang/lib/CodeGen/CGExpr.cpp:2422
@@ -2404,1 +2421,3 @@
+
 llvm::CallInst *CodeGenFunction::EmitTrapCall(llvm::Intrinsic::ID IntrID) {
+  return EmitTrapCall(IntrID, CGM.getCodeGenOpts().TrapFuncName);
----------------
samsonov wrote:
> Isn't it easier to just kill this function, and always pass in CGM.getCodeGenOpts().TrapFuncName where applicable?
It's used in a few other places (`__builtin_trap`, `__builtin_debugtrap`, fall off the end of a function in -O0), so I think it's probably easier (and less error prone) to keep a copy of the function that fetches the TrapFuncName itself.


http://reviews.llvm.org/D12181





More information about the cfe-commits mailing list