xazax.hun added a comment. In https://reviews.llvm.org/D40560#947514, @NoQ wrote:
> Replaced the live expression hack with a slightly better approach. It doesn't > update the live variables analysis to take `CFGNewAllocator` into account, > but at least tests now pass. > > In order to keep the return value produced by the `operator new()` call > around until `CXXNewExpr` is evaluated, i added a program state trait, > `CXXNewAllocatorValueStack`: > > 1. Upon evaluating `CFGNewAllocator`, the return `SVal` of the evaluated > allocator call is put here; > 2. Upon evaluating `CXXConstructExpr`, that return value is retrieved from > here; > 3. Upon evaluating `CXXNewExpr`, the return value is retrieved from here > again and then wiped. > > In order to support nested allocator calls, this state trait is organized > as a stack/FIFO, with push in `CFGNewAllocator` and pop in `CXXNewExpr`. The > `llvm::ImmutableList` thing offers some asserts for warning us when we popped > more times than we pushed; we might want to add more asserts here to detect > other mismatches, but i don't see a need for implementing this as an > environment-like map from (`Expr`, `LocationContext`) to SVal. I think it would be great to have tests for such cases to make it apparent it is not trivial to do this only based on the cfg. Maybe something like: A *a = new A(new B, coin ? new C : new D, inlinedCallWithMoreAllocs()); Or in case it turns out to be an easy problem to match these from CFG, I prefer avoiding the state. Also having a new expression within an overloaded operator new might be interesting. > Why `SVal` and not `MemRegion`? Because i believe that ideally we want to > produce constructor calls with null or undefined `this`-arguments. > Constructors into null pointers should be skipped - this is how `operator > new(std::nothrow)` works, for instance. Constructors into garbage pointers > should be immediately caught by the checkers (to throw a warning and generate > a sink), but we still need to produce them so that the checkers could catch > them. But for now `CXXConstructorCall` has room only for one pointer, which > is currently `const MemRegion *`, so this still remains to be tackled. Are you sure we need to produce undef vals? Couldn't a checker subscribe to the post allocator call and check for the undefined value and generate a sink before the constructor call? I am not opposed to using SVals there, just curious. > Also we need to make sure that not only the expression lives, but also its > value lives, with all the various traits attached to it. Hence the new > facility in `ExprEngine::removeDead()` to mark the values in > `CXXNewAllocatorValueStack` as live. Test included in `new-ctor-inlined.cpp` > (the one with `s != 0`) covers this situation. > > Some doxygen comments updated. https://reviews.llvm.org/D40560 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
