vsk added inline comments.
================ Comment at: lib/CodeGen/CodeGenFunction.h:380 + /// True if sanitizer checks for current pointer value are generated. + bool PointerChecksAreEmitted = false; + ---------------- rjmccall wrote: > sepavloff wrote: > > rjmccall wrote: > > > I don't think this is a good way of doing this. Using global state makes > > > this unnecessarily difficult to reason about and can have unintended > > > effects. For example, you are unintentionally suppressing any checks > > > that would be done as part of e.g. evaluating default arguments to the > > > default constructor. > > > > > > You should pass a parameter down that says whether checks are necessary. > > > You can also use that parameter to e.g. suppress checks when constructing > > > local variables and temporaries, although you may already have an > > > alternative mechanism for doing that. > > Passing parameter that inctructs whether to generate checks or not hardly > > can be directly implemented. This information is used in > > `EmitCXXConstructorCall` and it is formed during processing of `new` > > expression. The distance between these points can be 10 call frames, in > > some of them (`StmtVisitor` interface of `AggExprEmitter`) we cannot change > > function parameters. > > The case of `new` expression in default arguments indeed handled > > incorrectly, thank you for the catch. The updated version must process > > this case correctly. > I'm not going to accept a patch based on global state here. `AggValueSlot` > already propagates a bunch of flags about the slot into which we're emitting > an expression; I don't think it would be difficult to propagate some sort of > "alignment is statically known or already checked" flag along with that. + 1. @rjmccall offered similar advice in D25448, which led us to find a few more bugs / missed opportunities we otherwise wouldn't have. Repository: rC Clang https://reviews.llvm.org/D49589 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
