Page MenuHomePhabricator

[analyzer] MIGChecker: Fix false negatives for releases in automatic destructors.

Authored by NoQ on Feb 19 2019, 10:47 AM.



The problem that was fixed in D25326 for inlined functions keeps biting us for the top-level functions as well. Namely, i had to use check::PreStmt<ReturnStmt> rather than check::EndFunction in D57558 because in the basic_test test the paths get merged before invoking the check::EndFunction callback, which makes it impossible to recover the return value (which never gets stored in the Environment because it's, well, a literal). I don't have an immediate fix for the whole overall problem, so for now, as a hack, check::PreStmt<ReturnStmt> keeps being used.

However, the problem with check::PreStmt<ReturnStmt> is that automatic destructors for function-local variables are not yet evaluated when this callback is invoked. This causes false negatives in MIGChecker: if memory is released in an automatic destructor and then an error code is returned, the bug is not found because the Static Analyzer evalues the destructor *after* the pre-return. Arguably, this may also be treated as a bug, depending on what does the "Pre" in PreStmt<ReturnStmt> stand for.

But regardless, for now it seems that we'd have to subscribe on both callbacks.

Diff Detail


Event Timeline

NoQ created this revision.Feb 19 2019, 10:47 AM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 19 2019, 10:47 AM
NoQ edited the summary of this revision. (Show Details)Feb 19 2019, 10:57 AM
NoQ updated this revision to Diff 187503.Feb 19 2019, 7:44 PM


dcoughlin accepted this revision.Feb 19 2019, 9:07 PM

Oh, interesting. I'm glad you thought of this!

This revision is now accepted and ready to land.Feb 19 2019, 9:07 PM
NoQ updated this revision to Diff 187875.Feb 21 2019, 3:14 PM


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