================ 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; ---------------- Alexey Samsonov wrote: > 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. Thanks, you're quite right :)
http://reviews.llvm.org/D4544 _______________________________________________ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits