[PATCH] D49589: [UBSan] Strengthen pointer checks in 'new' expressions

Serge Pavlov via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Jul 24 12:25:41 PDT 2018


sepavloff 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:
> 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.


Repository:
  rC Clang

https://reviews.llvm.org/D49589





More information about the cfe-commits mailing list