Page MenuHomePhabricator

[analyzer] Track indices of arrays
ClosedPublic

Authored by Szelethus on Mon, Jun 10, 8:33 AM.

Details

Summary

Observe the test file before this patch:

I think this patch really improves on this case. The problem however, that some subexpressions of expressions should be tracked (like n in arr[n] here) is a broader one, and there may be other some cases I could add.

Diff Detail

Repository
rL LLVM

Event Timeline

Szelethus created this revision.Mon, Jun 10, 8:33 AM
NoQ added a comment.Mon, Jun 10, 7:05 PM

Whoa, this looks like a much needed improvement, i'm glad that you found it!

clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
1681 ↗(On Diff #203830)

Mmm, dunno about null fp suppression. We're, like, talking about integers. Integers are more often zero than null. We generally do support some FP suppressions for integers as well (i.e., core.DivideZero uses them), but in this case it doesn't sound as if 0 is anyhow special.

baloghadamsoftware requested changes to this revision.Wed, Jun 12, 4:31 AM
baloghadamsoftware added inline comments.
clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
1681 ↗(On Diff #203830)

I agree here. Forwarding EnableNUllFPSuppression does not really make sense for the array index even if it was true for the array element.

This revision now requires changes to proceed.Wed, Jun 12, 4:31 AM
Szelethus updated this revision to Diff 204888.Fri, Jun 14, 5:54 PM

One more test just for good measure, don't enable null fp suppression.

Szelethus marked 2 inline comments as done.Fri, Jun 14, 6:06 PM
NoQ accepted this revision.Fri, Jun 14, 6:14 PM

Great, thanks!!

This revision is now accepted and ready to land.Fri, Jun 14, 11:08 PM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptSun, Jun 16, 7:51 AM