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

[Alexey Samsonov via cfe-commits]

samsonov added a comment.

Please upload the patch with more context (see http://llvm.org/docs/Phabricator.html).


================
Comment at: tools/clang/include/clang/Driver/Options.td:610
@@ -608,1 +609,3 @@
+                                 Flags<[CC1Option, CoreOption]>,
+                                 HelpText<"Issue call to specified function rather than a trap instruction">;
 def fsanitize_undefined_trap_on_error : Flag<["-"], "fsanitize-undefined-trap-on-error">,
----------------
Fix the help text: Make trapping sanitizers issue a call to specified function rather than a trap instruction.

================
Comment at: tools/clang/include/clang/Frontend/CodeGenOptions.h:205
@@ +204,3 @@
+  /// If not an empty string, trap intrinsics are lowered to calls to this
+  /// function instead of to trap instructions.
+  std::string SanitizeTrapFuncName;
----------------
Fix the comment here as well.

================
Comment at: tools/clang/lib/CodeGen/CGExpr.cpp:2388
@@ +2387,3 @@
+  }
+  return EmitTrapCheck(Checked);
+}
----------------
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.



================
Comment at: tools/clang/lib/CodeGen/CGExpr.cpp:2415
@@ -2403,1 +2414,3 @@
 
+llvm::CallInst *CodeGenFunction::EmitSanitizeTrapCall(llvm::Intrinsic::ID IntrID) {
+  if (!CGM.getCodeGenOpts().SanitizeTrapFuncName.empty()) {
----------------
This function should not be needed.

================
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);
----------------
Isn't it easier to just kill this function, and always pass in CGM.getCodeGenOpts().TrapFuncName where applicable?

================
Comment at: tools/clang/lib/CodeGen/CGExprScalar.cpp:2386
@@ -2385,3 +2385,3 @@
     } else
-      CGF.EmitTrapCheck(Builder.CreateNot(overflow));
+      CGF.EmitSanitizeTrapCheck(Builder.CreateNot(overflow));
     return result;
----------------
Why? Looks like this check is not emitted as a part of `-fsanitize=signed-integer-overflow` 

================
Comment at: tools/clang/lib/Frontend/CompilerInvocation.cpp:507
@@ -506,2 +506,3 @@
   Opts.TrapFuncName = Args.getLastArgValue(OPT_ftrap_function_EQ);
+  Opts.SanitizeTrapFuncName = Args.getLastArgValue(OPT_fsanitize_trap_function_EQ);
   Opts.UseInitArray = Args.hasArg(OPT_fuse_init_array);
----------------
Please parse this argument next to another sanitizer arguments.


Repository:
  rL LLVM

http://reviews.llvm.org/D12181