]> granicus.if.org Git - clang/commitdiff
[analyzer] Unbreak body farms in presence of multiple declarations.
authorArtem Dergachev <artem.dergachev@gmail.com>
Tue, 23 Apr 2019 02:56:00 +0000 (02:56 +0000)
committerArtem Dergachev <artem.dergachev@gmail.com>
Tue, 23 Apr 2019 02:56:00 +0000 (02:56 +0000)
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
ParmVarDecls 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.

Differential Revision: https://reviews.llvm.org/D60899

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@358946 91177308-0d34-0410-b5e6-96231b3b80d8

lib/Analysis/BodyFarm.cpp
lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
test/Analysis/OSAtomic_mac.c

index a36b0f337cac3ca2569eeb56991975e90a8cf8ab..bb70795d91a867609c8d88db4c9d2e36b36e9042 100644 (file)
@@ -665,8 +665,6 @@ static Stmt *create_OSAtomicCompareAndSwap(ASTContext &C, const FunctionDecl *D)
 }
 
 Stmt *BodyFarm::getBody(const FunctionDecl *D) {
-  D = D->getCanonicalDecl();
-
   Optional<Stmt *> &Val = Bodies[D];
   if (Val.hasValue())
     return Val.getValue();
index af92077ef981f316abfe5689b866358bd69623cb..fc8826567c428d922cda6d2eed4d16a1d1a02712 100644 (file)
@@ -579,6 +579,9 @@ private:
     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 a69d98078e1717b5318df54e589c61caaa8ccc09..b09c71f6c6e9de4046a4eda63cd5b3953826dc89 100644 (file)
@@ -8,13 +8,20 @@ int OSAtomicCompareAndSwapPtrBarrier() {
 }
 
 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}}
 }