Page MenuHomePhabricator

[analyzer] Assume that the allocated value is non-null before construction, not after.

Authored by NoQ on Jan 17 2018, 11:19 AM.



In the c++-allocator-inlining=true mode, we need to make the assumption that the conservatively evaluated operator new() has returned a non-null value. Previously we did this on CXXNewExpr, but now we have to do that before calling the constructor, because some clever constructors are sometimes assuming that their this is null and doing weird stuff. We would also crash upon evaluating CXXNewExpr when the allocator was inlined and returned null and had a throw specification; this is UB even for custom allocators, but we still need not to crash.

Added more FIXME tests to ensure that eventually we fix calling the constructor for null return values.

Diff Detail


Event Timeline

NoQ created this revision.Jan 17 2018, 11:19 AM
514 ↗(On Diff #130224)


516 ↗(On Diff #130224)

This is neither here nor there, but for this and many other cases I think we could be considerably more readable by defining helpers State->assumeIsTrue and State->assumeIsFalse (or assumeNonNull, or whatever is most descriptive)

NoQ added a comment.Jan 17 2018, 11:52 AM

Forgot to mention that this patch follows the tradition of duplicating code from VisitCXXNewExpr() to VisitCXXNewAllocatorCall() - until we enable c++-allocator-inlining by default and clean away the old mode support from VisitCXXNewExpr() (which would become pretty trivial).

NoQ updated this revision to Diff 130335.Jan 17 2018, 5:24 PM

Use auto.

NoQ marked an inline comment as done.Jan 17 2018, 5:25 PM
NoQ added inline comments.
516 ↗(On Diff #130224)

Dunno, assume(x, true) sounds descriptive enough to me, and also feels more like a natural spoken language.

This revision was not accepted when it landed; it landed in state Needs Review.Jan 24 2018, 12:34 PM
This revision was automatically updated to reflect the committed changes.