This is an archive of the discontinued LLVM Phabricator instance.

[analyzer] Note last writes to a condition only in a nested stackframe
ClosedPublic

Authored by Szelethus on Jul 5 2019, 2:55 PM.

Diff Detail

Repository
rL LLVM

Event Timeline

Szelethus created this revision.Jul 5 2019, 2:55 PM
Szelethus updated this revision to Diff 208275.Jul 6 2019, 9:46 AM

Rebase after D64271 being abandoned.

NoQ accepted this revision.Jul 14 2019, 10:05 PM

Yup, looks good!

clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
146–149 ↗(On Diff #208275)

I definitely don't don't don't don't dislike quadruple negations.

This revision is now accepted and ready to land.Jul 14 2019, 10:05 PM

I have one small question otherwise looks good.

clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
1457 ↗(On Diff #210340)

I wonder if "nested" is a good term in the title of this revision. We could also show notes in the caller right? So it might also be an "enclosing" frame. And if so, is it desired? Since the "enclosing" frames should already be visible for the user without the additional notes. But correct me if I'm wrong :)

NoQ added inline comments.Jul 17 2019, 4:01 PM
clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
1447–1454 ↗(On Diff #210340)

Yeah, interesting, i guess we should also not track the value further. Otherwise it'd be weird that one piece in the middle is missing but everything else is still there.

Szelethus marked 2 inline comments as done.Jul 18 2019, 12:14 AM
Szelethus added inline comments.
clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
1447–1454 ↗(On Diff #210340)

That is what D64271 was for, and I think it would be a bad idea. What we should rather do is have a better note message:
"The value of 'flag' will be used in a condition expression later" or something like that.

clang/test/Analysis/track-control-dependency-conditions.cpp
263 ↗(On Diff #210340)

Like here. We'd lose this note if we didn't track further. I'm already gathering data tho!

Szelethus updated this revision to Diff 211758.Jul 25 2019, 8:00 AM
  • Only in a nested stackframe. Not different stackframe.
Szelethus marked 2 inline comments as done.Jul 25 2019, 8:00 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptAug 14 2019, 2:39 AM