From: Anna Zaks <ganna@apple.com>
Date: Thu, 2 Aug 2012 23:41:05 +0000 (+0000)
Subject: [analyzer] Solve another source of non-determinism in the diagnostic
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=d015f4febe85d3e3340172d70042840c51bbd836;p=clang

[analyzer] Solve another source of non-determinism in the diagnostic
engine.

The code that was supposed to split the tie in a deterministic way is
not deterministic. Most likely one of the profile methods uses a
pointer. After this change we do finally get the consistent diagnostic
output. Testing this requires running the analyzer on large code bases
and diffing the results.

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

diff --git a/lib/StaticAnalyzer/Core/BugReporter.cpp b/lib/StaticAnalyzer/Core/BugReporter.cpp
index 1eaf1f9f76..7ba2fa7fdd 100644
--- a/lib/StaticAnalyzer/Core/BugReporter.cpp
+++ b/lib/StaticAnalyzer/Core/BugReporter.cpp
@@ -1536,6 +1536,8 @@ void BugReporter::FlushReports() {
          I = bugTypes.begin(), E = bugTypes.end(); I != E; ++I)
     const_cast<BugType*>(*I)->FlushReports(*this);
 
+  // We need to flush reports in deterministic order to ensure the order
+  // of the reports is consistent between runs.
   typedef std::vector<BugReportEquivClass *> ContVecTy;
   for (ContVecTy::iterator EI=EQClassesVector.begin(), EE=EQClassesVector.end();
        EI != EE; ++EI){
diff --git a/lib/StaticAnalyzer/Core/PathDiagnostic.cpp b/lib/StaticAnalyzer/Core/PathDiagnostic.cpp
index 394e975d4e..b4c9068784 100644
--- a/lib/StaticAnalyzer/Core/PathDiagnostic.cpp
+++ b/lib/StaticAnalyzer/Core/PathDiagnostic.cpp
@@ -148,23 +148,14 @@ void PathDiagnosticConsumer::HandlePathDiagnostic(PathDiagnostic *D) {
 
   if (PathDiagnostic *orig = Diags.FindNodeOrInsertPos(profile, InsertPos)) {
     // Keep the PathDiagnostic with the shorter path.
+    // Note, the enclosing rutine is called in deterministic order, so the
+    // results will be consistent between runs (no reason to break ties if the
+    // size is the same).
     const unsigned orig_size = orig->full_size();
     const unsigned new_size = D->full_size();
-    
-    if (orig_size <= new_size) {
-      bool shouldKeepOriginal = true;
-      if (orig_size == new_size) {
-        // Here we break ties in a fairly arbitrary, but deterministic, way.
-        llvm::FoldingSetNodeID fullProfile, fullProfileOrig;
-        D->FullProfile(fullProfile);
-        orig->FullProfile(fullProfileOrig);
-        if (fullProfile.ComputeHash() < fullProfileOrig.ComputeHash())
-          shouldKeepOriginal = false;
-      }
+    if (orig_size <= new_size)
+      return;
 
-      if (shouldKeepOriginal)
-        return;
-    }
     Diags.RemoveNode(orig);
     delete orig;
   }
diff --git a/test/Analysis/inline-unique-reports.c b/test/Analysis/inline-unique-reports.c
index 6ae77e257e..9248ad2632 100644
--- a/test/Analysis/inline-unique-reports.c
+++ b/test/Analysis/inline-unique-reports.c
@@ -34,12 +34,12 @@ void test_bug_2() {
 // CHECK:         <key>start</key>
 // CHECK:          <array>
 // CHECK:           <dict>
-// CHECK:            <key>line</key><integer>9</integer>
+// CHECK:            <key>line</key><integer>14</integer>
 // CHECK:            <key>col</key><integer>3</integer>
 // CHECK:            <key>file</key><integer>0</integer>
 // CHECK:           </dict>
 // CHECK:           <dict>
-// CHECK:            <key>line</key><integer>9</integer>
+// CHECK:            <key>line</key><integer>14</integer>
 // CHECK:            <key>col</key><integer>5</integer>
 // CHECK:            <key>file</key><integer>0</integer>
 // CHECK:           </dict>
@@ -47,12 +47,12 @@ void test_bug_2() {
 // CHECK:         <key>end</key>
 // CHECK:          <array>
 // CHECK:           <dict>
-// CHECK:            <key>line</key><integer>10</integer>
+// CHECK:            <key>line</key><integer>15</integer>
 // CHECK:            <key>col</key><integer>3</integer>
 // CHECK:            <key>file</key><integer>0</integer>
 // CHECK:           </dict>
 // CHECK:           <dict>
-// CHECK:            <key>line</key><integer>10</integer>
+// CHECK:            <key>line</key><integer>15</integer>
 // CHECK:            <key>col</key><integer>5</integer>
 // CHECK:            <key>file</key><integer>0</integer>
 // CHECK:           </dict>
@@ -64,7 +64,7 @@ void test_bug_2() {
 // CHECK:      <key>kind</key><string>event</string>
 // CHECK:      <key>location</key>
 // CHECK:      <dict>
-// CHECK:       <key>line</key><integer>10</integer>
+// CHECK:       <key>line</key><integer>15</integer>
 // CHECK:       <key>col</key><integer>3</integer>
 // CHECK:       <key>file</key><integer>0</integer>
 // CHECK:      </dict>
@@ -72,12 +72,12 @@ void test_bug_2() {
 // CHECK:      <array>
 // CHECK:        <array>
 // CHECK:         <dict>
-// CHECK:          <key>line</key><integer>10</integer>
+// CHECK:          <key>line</key><integer>15</integer>
 // CHECK:          <key>col</key><integer>3</integer>
 // CHECK:          <key>file</key><integer>0</integer>
 // CHECK:         </dict>
 // CHECK:         <dict>
-// CHECK:          <key>line</key><integer>10</integer>
+// CHECK:          <key>line</key><integer>15</integer>
 // CHECK:          <key>col</key><integer>8</integer>
 // CHECK:          <key>file</key><integer>0</integer>
 // CHECK:         </dict>
@@ -99,9 +99,9 @@ void test_bug_2() {
 // CHECK:      </dict>
 // CHECK:      <key>depth</key><integer>1</integer>
 // CHECK:      <key>extended_message</key>
-// CHECK:      <string>Entered call from &apos;test_bug_1&apos;</string>
+// CHECK:      <string>Entered call from &apos;test_bug_2&apos;</string>
 // CHECK:      <key>message</key>
-// CHECK: <string>Entered call from &apos;test_bug_1&apos;</string>
+// CHECK: <string>Entered call from &apos;test_bug_2&apos;</string>
 // CHECK:     </dict>
 // CHECK:     <dict>
 // CHECK:      <key>kind</key><string>control</string>
@@ -172,6 +172,7 @@ void test_bug_2() {
 // CHECK:    <key>type</key><string>Dereference of null pointer</string>
 // CHECK:   <key>issue_context_kind</key><string>function</string>
 // CHECK:   <key>issue_context</key><string>bug</string>
+// CHECK:   <key>issue_hash</key><integer>1</integer>
 // CHECK:   <key>location</key>
 // CHECK:   <dict>
 // CHECK:    <key>line</key><integer>5</integer>