This is an archive of the discontinued LLVM Phabricator instance.

[analyzer][UninitializedObjectChecker] Refactoring p6.: Move dereferencing to a function
ClosedPublic

Authored by Szelethus on Aug 9 2018, 6:30 AM.

Diff Detail

Event Timeline

Szelethus created this revision.Aug 9 2018, 6:30 AM
Szelethus updated this revision to Diff 159916.Aug 9 2018, 6:55 AM

Fixed a comment.

george.karpenkov requested changes to this revision.Aug 10 2018, 11:54 AM
george.karpenkov added inline comments.
lib/StaticAnalyzer/Checkers/UninitializedPointee.cpp
78 ↗(On Diff #159916)

In general, using return values is better than out-parameters.
Could you instead return Optional<std::pair<SVal, QualType>> ?

This revision now requires changes to proceed.Aug 10 2018, 11:54 AM
Szelethus updated this revision to Diff 161209.Aug 17 2018, 4:52 AM
Szelethus edited the summary of this revision. (Show Details)
NoQ added inline comments.Aug 17 2018, 12:18 PM
lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp
223

Hmm, i still have concerns about things like int *x = (int *)&x;. Why not just check the type to terminate the loop? Type hierarchy is guaranteed to be finite.

NoQ accepted this revision.Aug 17 2018, 12:19 PM

Which shouldn't prevent us from moving code around.

Szelethus added inline comments.Aug 17 2018, 1:22 PM
lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp
223

There actually is a testcase for that -- it would create a nonloc::LocAsInteger, not a loc::MemRegionVal.

I'll add a TODO to revisit this loop condition (again :) ).

NoQ added inline comments.Aug 17 2018, 1:31 PM
lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp
223

Ok, let's try with one more asterisk:

1 void test() {
2   int **x = (int **)&x;
3   int *y = *x;
4   int z = *y;
5 }

Here's what i get in the Store:

(x,0,direct) : &element{x,0 S64b,int *}
(y,0,direct) : &element{x,0 S64b,int *}
(z,0,direct) : &element{x,0 S64b,int *}
Szelethus added inline comments.Aug 17 2018, 2:18 PM
lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp
223

Sounds fun, I'll see how the checker behaves to these when I'm in the office.

Szelethus added inline comments.Aug 21 2018, 3:37 AM
lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp
223

Yup, you were correct, it ends up in an infinite loop. I'll add the testcase for it before commiting.

This revision was not accepted when it landed; it landed in state Needs Review.Aug 21 2018, 3:46 AM
This revision was automatically updated to reflect the committed changes.