Page MenuHomePhabricator

[analyzer] Do not emit redundant SymbolCasts
ClosedPublic

Authored by martong on Jun 17 2022, 9:22 AM.

Details

Summary

In RegionStore::getBinding we call evalCast unconditionally to align
the stored value's type to the one that is being queried. However, the
stored type might be the same, so we may end up having redundant
SymbolCasts emitted.

The solution is to check whether the to and from type are the same
in makeNonLoc.

Note, we can't just do type equivalence check at the beginning of evalCast
because when evalCast is called from getBinding then the original type
(OriginalTy) is not set, so one operand is missing for the comparison. In
evalCastSubKind(nonloc::SymbolVal) when the original type is not set,
we get the from type via SymbolVal::getType().

Diff Detail

Event Timeline

martong created this revision.Jun 17 2022, 9:22 AM
Herald added a project: Restricted Project. · View Herald Transcript
martong requested review of this revision.Jun 17 2022, 9:22 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 17 2022, 9:22 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript
martong edited the summary of this revision. (Show Details)Jun 17 2022, 9:30 AM
martong edited the summary of this revision. (Show Details)
steakhal accepted this revision.Jun 18 2022, 11:15 AM

We can't just do that check in evalCast because there are many additonal logic before we'd end up in makeNonLoc.

I'm not exactly up to date. Could you please extend the summary with an example to underpin this statement prior to committing?

clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
116–117

Should we account for qualified types?
If not, could we assert that?

clang/test/Analysis/symbolcast-floatingpoint.cpp
13–25
This revision is now accepted and ready to land.Jun 18 2022, 11:15 AM
martong edited the summary of this revision. (Show Details)Jul 5 2022, 9:37 AM

We can't just do that check in evalCast because there are many additonal logic before we'd end up in makeNonLoc.

I'm not exactly up to date. Could you please extend the summary with an example to underpin this statement prior to committing?

Yeah, okay, I've updated the summary and will do so with the commit message as well.

This revision was landed with ongoing or failed builds.Jul 5 2022, 9:59 AM
This revision was automatically updated to reflect the committed changes.