Page MenuHomePhabricator

[analyzer] Move function declarations of PthreadLockChecker's tests to a separate system-like header
ClosedPublic

Authored by a.sidorin on Sep 8 2014, 10:43 AM.

Details

Summary

Regions passed to pthread* functions are invalidated in tests because the call is evaluated conservatively and declarations are not located in system header. This patch creates a system-like header with declarations previously located in test file.

Diff Detail

Event Timeline

a.sidorin updated this revision to Diff 13408.Sep 8 2014, 10:43 AM
a.sidorin retitled this revision from to [analyzer][Bugfix] Do not invalidate regions passed to pthread* functions.
a.sidorin updated this object.
a.sidorin edited the test plan for this revision. (Show Details)
a.sidorin edited the test plan for this revision. (Show Details)
a.sidorin added a subscriber: Unknown Object (MLST).
krememek edited edge metadata.Sep 9 2014, 11:40 AM

This feels like a big hammer to wield just to prevent the conservative invalidation done by the static analyzer for calls to functions whose bodies are not available.

I feel like a more general approach might be to provide new hooks for checkers to provide hints to the core analyzer engine. For example, a callback that indicates whether or not invalidation should take place. That seems like a solution that could be utilized by other checkers besides just this one, and would allow checkers to opt into this with very little code.

jordan_rose edited edge metadata.Sep 9 2014, 12:05 PM

I tend to agree. eval::Call has the downside that only one checker can provide an implementation (since it controls the return value). But I'm also surprised that the example is failing—because the pthread_* methods are declared in system headers, only globals declared in system headers should be invalidated.

You're right Jordan. I have made wrong suggestions from my investigation. It should be tests that should be fixed, not the checker itself.

a.sidorin updated this revision to Diff 13519.Sep 10 2014, 1:55 AM
a.sidorin retitled this revision from [analyzer][Bugfix] Do not invalidate regions passed to pthread* functions to [analyzer] Move function declarations of PthreadLockChecker's tests to a separate system-like header.
a.sidorin updated this object.
a.sidorin edited edge metadata.

Fix tests, not the checker itself. Sorry for mistake and thanks to Jordan for help.

jordan_rose accepted this revision.Sep 10 2014, 9:27 AM
jordan_rose edited edge metadata.

Looks good, committed in r217515.

This revision is now accepted and ready to land.Sep 10 2014, 9:27 AM
jordan_rose closed this revision.Sep 10 2014, 9:27 AM