================
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

Reply via email to