r358946 - [analyzer] Unbreak body farms in presence of multiple declarations.

[Artem Dergachev via cfe-commits]

Author: dergachev
Date: Mon Apr 22 19:56:00 2019
New Revision: 358946

URL: http://llvm.org/viewvc/llvm-project?rev=358946&view=rev
Log:
[analyzer] Unbreak body farms in presence of multiple declarations.

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

Modified:
    cfe/trunk/lib/Analysis/BodyFarm.cpp
    cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
    cfe/trunk/test/Analysis/OSAtomic_mac.c

Modified: cfe/trunk/lib/Analysis/BodyFarm.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/BodyFarm.cpp?rev=358946&r1=358945&r2=358946&view=diff
==============================================================================
--- cfe/trunk/lib/Analysis/BodyFarm.cpp (original)
+++ cfe/trunk/lib/Analysis/BodyFarm.cpp Mon Apr 22 19:56:00 2019
@@ -665,8 +665,6 @@ static Stmt *create_OSAtomicCompareAndSw
 }
 
 Stmt *BodyFarm::getBody(const FunctionDecl *D) {
-  D = D->getCanonicalDecl();
-
   Optional<Stmt *> &Val = Bodies[D];
   if (Val.hasValue())
     return Val.getValue();

Modified: cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp?rev=358946&r1=358945&r2=358946&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp Mon Apr 22 19:56:00 2019
@@ -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;
 

Modified: cfe/trunk/test/Analysis/OSAtomic_mac.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/OSAtomic_mac.c?rev=358946&r1=358945&r2=358946&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/OSAtomic_mac.c (original)
+++ cfe/trunk/test/Analysis/OSAtomic_mac.c Mon Apr 22 19:56:00 2019
@@ -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 at -2{{Calling 'OSAtomicCompareAndSwapPtrBarrier'}}
-  // expected-note at -3{{Returning from 'OSAtomicCompareAndSwapPtrBarrier'}}
+  return b;
+}
 
-  return b; // expected-warning{{Undefined or garbage value returned to caller}}
-            // expected-note at -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 at -1{{TRUE}}
+  clang_analyzer_eval(x == &x); // expected-warning{{TRUE}}
+                                // expected-note at -1{{TRUE}}
 }