NoQ added a comment.
All right, it seems that i'm completely misunderstanding this problem and we've
been talking past each other this whole time.
The problem is not that we need to skip the `CXXConstructExpr`. The problem is
that we need to skip `CXXNewExpr` (!!). CFG elements for an operator-new call
go like this:
1. CFGAllocatorCall
2. CXXConstructExpr
3. CXXNewExpr
We're staring at element 3. The original loop says "hey, that's our statement!
we've found it!". However, even though this is the statement that's taken as
the call site, the respective ExplodedNode does not correspond to the end of
the call. Instead, it corresponds to a moment of time after construction has
finished and the new-expression is fully evaluated. So we need to ignore
element 3. Then, element 2 is ignored automatically because the statements
don't match. Then, we need to learn to recognize that element 1 is the right
element, which is going to be either a `CallExitEnd` (if the allocator call was
inlined) or dunno-but-definitely-not-`PostStmt` if the allocator was evaluated
conservatively.
Bonus: the constructor is never inlined in our scenario, so it's easy to skip
element 2, as it's going to be just PreStmt/PostStmt and no inlined call within
it. Why? Because how the hell do you inline a constructor of a null pointer.
The Standard guarantees that when the allocator returns a null pointer, the
constructor is gracefully omitted. We currently have a FIXME in
`prepareForObjectConstruction()` to implement this behavior:
179 // TODO: Detect when the allocator returns a null pointer.
180 // Constructor shall not be called in this case.
The current incorrect behavior is to instead evaluate the constructor
conservatively, with a fake temporary region instead of the null pointer (this
is generally the conservative approach to evaluating the constructor when
object-under-construction is not known).
So my request to come up with a test case in which the constructor is going to
be inlined was completely pointless. But in the general case we aren't really
sure that it's going to be a null pointer (we may want to track an arbitrary
value), so we shouldn't rely on that.
================
Comment at: clang/test/Analysis/new-ctor-null.cpp:37
S *s = new S();
// FIXME: Should be FALSE - we should not invalidate globals.
clang_analyzer_eval(global); // expected-warning{{UNKNOWN}}
----------------
This is essentially the same FIXME: the constructor should not have been
evaluated, so in particular we should not invalidate globals.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D62926/new/
https://reviews.llvm.org/D62926
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
