[PATCH] Don't instrument UBSan-generated code with ASan.
Alexey Samsonov
vonosmas at gmail.com
Wed Jul 16 17:47:16 PDT 2014
================
Comment at: tools/clang/lib/CodeGen/CGExprScalar.cpp:839
@@ -837,2 +838,3 @@
void ScalarExprEmitter::EmitBinOpCheck(Value *Check, const BinOpInfo &Info) {
+ assert(CGF.IsSanitizerScope);
StringRef CheckName;
----------------
Richard Smith wrote:
> Several callers of this are only guarding their call to this function; how about sinking the `SanitizerScope` down into here (and teaching the scope to cope with multiple `SanitizerScope` objects being live at once)?
On the contrary, there are no callers that have `SanitizerScope` only to guard the call to this function. Consider:
EmitBinOpCheck(Builder.CreateICmpULE(RHS, GetWidthMinusOneValue(Ops.LHS, RHS)), Ops);
we want to attach metadata to ICmpULE instruction created in this line. This is my main motivation for using asserts instead of re-entrant sanitizer scopes - if one adds a new UBSan check and writes the code like:
if (SanOpts->SanitizeFoo) {
Bar = Builder.CreateBar();
Baz = Builder.CreateBaz(Bar);
EmitBinOpCheck(Baz, Info);
}
this will die with an assertion failure. If we allow multiple scopes, we won't notice missing metadata for bar and baz instructions.
================
Comment at: tools/clang/lib/CodeGen/CodeGenFunction.cpp:1660
@@ +1659,3 @@
+ I->setMetadata(
+ CGM.getModule().getMDKindID("ubsan"),
+ llvm::MDNode::get(CGM.getLLVMContext(), ArrayRef<llvm::Value *>()));
----------------
Richard Smith wrote:
> It seems more future-safe to pick a more general name for this, describing its functionality not this particular use case. Maybe `"nosanitize"`?
Sounds good.
http://reviews.llvm.org/D4544
More information about the cfe-commits
mailing list