Page MenuHomePhabricator

[analyzer] ConditionBRVisitor: MemberExpr support
AcceptedPublic

Authored by Charusso on Feb 13 2019, 1:44 PM.

Details

Summary

-

Diff Detail

Event Timeline

Charusso created this revision.Feb 13 2019, 1:44 PM
test/Analysis/uninit-vals.m
222 ↗(On Diff #186734)

What happened to "assuming"?

Charusso updated this revision to Diff 187396.Feb 19 2019, 9:42 AM
Charusso marked 2 inline comments as done.
Charusso added inline comments.
test/Analysis/uninit-vals.m
222 ↗(On Diff #186734)

I just cannot copy-paste properly.

NoQ added inline comments.Mar 6 2019, 6:49 PM
lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
2066–2068 ↗(On Diff #187396)

I'm a bit worried that this source range can get veeeeeeeeery long because the object on which the member is taken may be relatively complicated to compute.

Why don't we just say field 'f' - i.e., something like Assuming field 'f' is null - either always, or when the sub-expression turns out to be too long to display?

2225–2229 ↗(On Diff #187396)

...and then de-duplicate this code snippet into a function(?)

2236–2237 ↗(On Diff #187396)

Is this a common thing to happen? Why don't other overrides of VisitTrueTest have such checks?

Charusso updated this revision to Diff 189924.Mar 8 2019, 1:59 PM
Charusso marked 4 inline comments as done.
  • Rebased and addressed comments.
lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
2236–2237 ↗(On Diff #187396)

It is rare of course. I do not know why this is happening but it is a straightforward way to bypass such errors.

If I remove this Loc validation:

  • Analysis/call_once.cpp fails at:
UnaryOperator 0x55e6814f8770 'int' prefix '!' cannot overflow
`-ImplicitCastExpr 0x55e6814f8758 'unsigned long' <IntegralToBoolean>
  `-ImplicitCastExpr 0x55e6814f8740 'unsigned long' <LValueToRValue>
    `-MemberExpr 0x55e6814f8710 'unsigned long' lvalue .__state_ 0x55e6814cc710
      `-DeclRefExpr 0x55e6814f86f0 'std::once_flag':'struct std::once_flag_s' lvalue ParmVar 0x55e6814ecbe0 'o' 'std::once_flag &'
  • Analysis/html_diagnostics/relevant_lines/synthesized_body.cpp fails at:
UnaryOperator 0x556bdac7cf80 'int' prefix '!' cannot overflow
`-ImplicitCastExpr 0x556bdac7cf68 'int' <IntegralToBoolean>
  `-ImplicitCastExpr 0x556bdac7cf50 'int' <LValueToRValue>
    `-MemberExpr 0x556bdac7cf20 'int' lvalue ._M_once 0x556bdac39b90
      `-DeclRefExpr 0x556bdac64440 'std::once_flag':'struct std::once_flag_s' lvalue ParmVar 0x556bdac63cc0 'o' 'std::once_flag &'
NoQ accepted this revision.Mar 25 2019, 2:23 PM

Looks great, thanks!

lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
2236–2237 ↗(On Diff #187396)

Hmm, body farms, okay. I guess nobody really knows what's going on in body farms, so let's keep it as is until we see more crashes. I'd expect other overrides of VisitTrueTest to grow such checks in the future.

This revision is now accepted and ready to land.Mar 25 2019, 2:23 PM
Charusso updated this revision to Diff 195073.Sun, Apr 14, 9:05 AM
Charusso marked an inline comment as done.
  • Rebased