This is an archive of the discontinued LLVM Phabricator instance.

[analyzer] Fix target region invalidation when returning into a ctor initializer.
ClosedPublic

Authored by NoQ on Jun 28 2019, 6:00 PM.

Details

Summary

Enabling pointer escape notification for the return value invalidation when conservatively evaluating calls that return C++ objects by value was supposed to be pointless. Indeed, we're looking at a freshly conjured value; why would any checker already track it at this point? Yet it does cause a change in results, so i decided to reduce and investigate a reproducer @Charusso sent me.

When i was thinking about it previously (as of D44131), i was imagining a function that constructs a temporary that would later be copied to its target destination. In this case there's indeed no need to notify checkers of pointer escape.

However, once RVO was implemented (D47671), the target region is no longer necessarily a temporary; it may be an arbitrary memory region. In particular, it may be a non-base region, such as FieldRegion if the construction context is a ConstructorInitializerConstructionContext. In this case the invalidation covers not only the target region but its whole base region that may already contain some values that might as well be tracked by the checkers.

The right solution, therefore, is to restrict invalidation so that it didn't propagate to the base region, but only wipe the target region itself. It probably doesn't work perfectly because RegionStore doesn't enjoy this sort of stuff, but it should be something.

In the newly added test case the previous behavior was to immediately forget about b1.p and b2.p, therefore evals on lines 21 and 23 yielded both TRUE and FALSE each (due to eagerly-assume) and the leak was diagnosed on line 22 (even though the pointer is used later).

Diff Detail

Event Timeline

NoQ created this revision.Jun 28 2019, 6:00 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 28 2019, 6:00 PM
Charusso accepted this revision.Jun 28 2019, 6:18 PM

You have made a great use of that mistake by me. So that is why everything is so perfect, because it fires, just it should fire later. Thanks you!

This revision is now accepted and ready to land.Jun 28 2019, 6:18 PM
xazax.hun accepted this revision.Jul 1 2019, 10:26 AM

LG! Thanks!

This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptJul 1 2019, 4:03 PM