NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a_sidorin, rnkovacs,
mikhail.ramalho, Szelethus, baloghadamsoftware, Charusso.
Herald added subscribers: cfe-commits, dkrupp, donat.nagy, jfb, a.sidorin,
JDevlieghere, szepet, kristof.beyls, javed.absar.
Herald added a project: clang.
NoQ added a parent revision: D60808: [analyzer] pr41335: NoStoreFuncVisitor:
Fix crash when no-store event is in a body-farmed function..
Split out of D60808 <https://reviews.llvm.org/D60808>.
When growing a body on a body farm, it's essential to use the same
redeclaration of the function that's going to be used during analysis.
Otherwise our `ParmVarDecl`s won't match the ones that are used to identify
argument regions. This boils down to trusting the reasoning in
`AnalysisDeclContext`. We shouldn't canonicalize the declaration before farming
the body because it makes us not obey the sophisticated decision-making process
of `AnalysisDeclContext`.
Repository:
rC Clang
https://reviews.llvm.org/D60899
Files:
clang/lib/Analysis/BodyFarm.cpp
clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
clang/test/Analysis/OSAtomic_mac.c
Index: clang/test/Analysis/OSAtomic_mac.c
===================================================================
--- clang/test/Analysis/OSAtomic_mac.c
+++ clang/test/Analysis/OSAtomic_mac.c
@@ -8,13 +8,20 @@
}
int *invalidSLocOnRedecl() {
- int *b; // expected-note{{'b' declared without an initial value}}
-
+ // Was crashing when trying to throw a report about returning an
uninitialized
+ // value to the caller. FIXME: We should probably still throw that report,
+ // something like "The "compare" part of CompareAndSwap depends on an
+ // undefined value".
+ int *b;
OSAtomicCompareAndSwapPtrBarrier(0, 0, &b); // no-crash
- // FIXME: We don't really need these notes.
- // expected-note@-2{{Calling 'OSAtomicCompareAndSwapPtrBarrier'}}
- // expected-note@-3{{Returning from 'OSAtomicCompareAndSwapPtrBarrier'}}
+ return b;
+}
- return b; // expected-warning{{Undefined or garbage value returned to
caller}}
- // expected-note@-1{{Undefined or garbage value returned to
caller}}
+void testThatItActuallyWorks() {
+ void *x = 0;
+ int res = OSAtomicCompareAndSwapPtrBarrier(0, &x, &x);
+ clang_analyzer_eval(res); // expected-warning{{TRUE}}
+ // expected-note@-1{{TRUE}}
+ clang_analyzer_eval(x == &x); // expected-warning{{TRUE}}
+ // expected-note@-1{{TRUE}}
}
Index: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
+++ clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
@@ -579,6 +579,9 @@
PathDiagnosticLocation L =
PathDiagnosticLocation::create(N->getLocation(), SM);
+ // For now this shouldn't trigger, but once it does (as we add more
+ // functions to the body farm), we'll need to decide if these reports
+ // are worth suppressing as well.
if (!L.hasValidLocation())
return nullptr;
Index: clang/lib/Analysis/BodyFarm.cpp
===================================================================
--- clang/lib/Analysis/BodyFarm.cpp
+++ clang/lib/Analysis/BodyFarm.cpp
@@ -665,8 +665,6 @@
}
Stmt *BodyFarm::getBody(const FunctionDecl *D) {
- D = D->getCanonicalDecl();
-
Optional<Stmt *> &Val = Bodies[D];
if (Val.hasValue())
return Val.getValue();
Index: clang/test/Analysis/OSAtomic_mac.c
===================================================================
--- clang/test/Analysis/OSAtomic_mac.c
+++ clang/test/Analysis/OSAtomic_mac.c
@@ -8,13 +8,20 @@
}
int *invalidSLocOnRedecl() {
- int *b; // expected-note{{'b' declared without an initial value}}
-
+ // Was crashing when trying to throw a report about returning an uninitialized
+ // value to the caller. FIXME: We should probably still throw that report,
+ // something like "The "compare" part of CompareAndSwap depends on an
+ // undefined value".
+ int *b;
OSAtomicCompareAndSwapPtrBarrier(0, 0, &b); // no-crash
- // FIXME: We don't really need these notes.
- // expected-note@-2{{Calling 'OSAtomicCompareAndSwapPtrBarrier'}}
- // expected-note@-3{{Returning from 'OSAtomicCompareAndSwapPtrBarrier'}}
+ return b;
+}
- return b; // expected-warning{{Undefined or garbage value returned to caller}}
- // expected-note@-1{{Undefined or garbage value returned to caller}}
+void testThatItActuallyWorks() {
+ void *x = 0;
+ int res = OSAtomicCompareAndSwapPtrBarrier(0, &x, &x);
+ clang_analyzer_eval(res); // expected-warning{{TRUE}}
+ // expected-note@-1{{TRUE}}
+ clang_analyzer_eval(x == &x); // expected-warning{{TRUE}}
+ // expected-note@-1{{TRUE}}
}
Index: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
+++ clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
@@ -579,6 +579,9 @@
PathDiagnosticLocation L =
PathDiagnosticLocation::create(N->getLocation(), SM);
+ // For now this shouldn't trigger, but once it does (as we add more
+ // functions to the body farm), we'll need to decide if these reports
+ // are worth suppressing as well.
if (!L.hasValidLocation())
return nullptr;
Index: clang/lib/Analysis/BodyFarm.cpp
===================================================================
--- clang/lib/Analysis/BodyFarm.cpp
+++ clang/lib/Analysis/BodyFarm.cpp
@@ -665,8 +665,6 @@
}
Stmt *BodyFarm::getBody(const FunctionDecl *D) {
- D = D->getCanonicalDecl();
-
Optional<Stmt *> &Val = Bodies[D];
if (Val.hasValue())
return Val.getValue();
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
