[clang] 22a084c - [Analyzer] Report every bug if only uniqueing location differs.

[Balázs Kéri via cfe-commits]

Author: Balázs Kéri
Date: 2020-07-15T12:19:25+02:00
New Revision: 22a084cfa337d5e5ea90eba5261f7937e28d250b

URL: https://github.com/llvm/llvm-project/commit/22a084cfa337d5e5ea90eba5261f7937e28d250b
DIFF: https://github.com/llvm/llvm-project/commit/22a084cfa337d5e5ea90eba5261f7937e28d250b.diff

LOG: [Analyzer] Report every bug if only uniqueing location differs.

Summary:
Two CSA bug reports where only the uniqueing location is different
should be treated as different problems. The role of uniqueing location
is to differentiate bug reports.

Reviewers: Szelethus, baloghadamsoftware, NoQ, vsavchenko, xazax.hun, martong

Reviewed By: NoQ

Subscribers: NoQ, rnkovacs, xazax.hun, baloghadamsoftware, szepet, a.sidorin, mikhail.ramalho, Szelethus, donat.nagy, dkrupp, gamesh411, Charusso, martong, ASDenysPetrov, cfe-commits

Tags: #clang

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

Added: 
    

Modified: 
    clang/lib/Analysis/PathDiagnostic.cpp
    clang/test/Analysis/malloc.c
    clang/test/Analysis/pr22954.c

Removed: 
    


################################################################################
diff  --git a/clang/lib/Analysis/PathDiagnostic.cpp b/clang/lib/Analysis/PathDiagnostic.cpp
index c88e6c1e1535..9aa3386129d7 100644
--- a/clang/lib/Analysis/PathDiagnostic.cpp
+++ b/clang/lib/Analysis/PathDiagnostic.cpp
@@ -327,6 +327,10 @@ static Optional<bool> comparePath(const PathPieces &X, const PathPieces &Y) {
 }
 
 static bool compareCrossTUSourceLocs(FullSourceLoc XL, FullSourceLoc YL) {
+  if (XL.isInvalid() && YL.isValid())
+    return true;
+  if (XL.isValid() && YL.isInvalid())
+    return false;
   std::pair<FileID, unsigned> XOffs = XL.getDecomposedLoc();
   std::pair<FileID, unsigned> YOffs = YL.getDecomposedLoc();
   const SourceManager &SM = XL.getManager();
@@ -349,6 +353,10 @@ static bool compare(const PathDiagnostic &X, const PathDiagnostic &Y) {
   FullSourceLoc YL = Y.getLocation().asLocation();
   if (XL != YL)
     return compareCrossTUSourceLocs(XL, YL);
+  FullSourceLoc XUL = X.getUniqueingLoc().asLocation();
+  FullSourceLoc YUL = Y.getUniqueingLoc().asLocation();
+  if (XUL != YUL)
+    return compareCrossTUSourceLocs(XUL, YUL);
   if (X.getBugType() != Y.getBugType())
     return X.getBugType() < Y.getBugType();
   if (X.getCategory() != Y.getCategory())
@@ -357,20 +365,27 @@ static bool compare(const PathDiagnostic &X, const PathDiagnostic &Y) {
     return X.getVerboseDescription() < Y.getVerboseDescription();
   if (X.getShortDescription() != Y.getShortDescription())
     return X.getShortDescription() < Y.getShortDescription();
-  if (X.getDeclWithIssue() != Y.getDeclWithIssue()) {
-    const Decl *XD = X.getDeclWithIssue();
-    if (!XD)
+  auto CompareDecls = [&XL](const Decl *D1, const Decl *D2) -> Optional<bool> {
+    if (D1 == D2)
+      return None;
+    if (!D1)
       return true;
-    const Decl *YD = Y.getDeclWithIssue();
-    if (!YD)
+    if (!D2)
       return false;
-    SourceLocation XDL = XD->getLocation();
-    SourceLocation YDL = YD->getLocation();
-    if (XDL != YDL) {
+    SourceLocation D1L = D1->getLocation();
+    SourceLocation D2L = D2->getLocation();
+    if (D1L != D2L) {
       const SourceManager &SM = XL.getManager();
-      return compareCrossTUSourceLocs(FullSourceLoc(XDL, SM),
-                                      FullSourceLoc(YDL, SM));
+      return compareCrossTUSourceLocs(FullSourceLoc(D1L, SM),
+                                      FullSourceLoc(D2L, SM));
     }
+    return None;
+  };
+  if (auto Result = CompareDecls(X.getDeclWithIssue(), Y.getDeclWithIssue()))
+    return *Result;
+  if (XUL.isValid()) {
+    if (auto Result = CompareDecls(X.getUniqueingDecl(), Y.getUniqueingDecl()))
+      return *Result;
   }
   PathDiagnostic::meta_iterator XI = X.meta_begin(), XE = X.meta_end();
   PathDiagnostic::meta_iterator YI = Y.meta_begin(), YE = Y.meta_end();
@@ -1118,6 +1133,8 @@ void PathDiagnosticPopUpPiece::Profile(llvm::FoldingSetNodeID &ID) const {
 
 void PathDiagnostic::Profile(llvm::FoldingSetNodeID &ID) const {
   ID.Add(getLocation());
+  ID.Add(getUniqueingLoc());
+  ID.AddPointer(getUniqueingLoc().isValid() ? getUniqueingDecl() : nullptr);
   ID.AddString(BugType);
   ID.AddString(VerboseDesc);
   ID.AddString(Category);

diff  --git a/clang/test/Analysis/malloc.c b/clang/test/Analysis/malloc.c
index 714c73c3c793..a26b51196781 100644
--- a/clang/test/Analysis/malloc.c
+++ b/clang/test/Analysis/malloc.c
@@ -791,7 +791,8 @@ void mallocEscapeMalloc() {
 void mallocMalloc() {
   int *p = malloc(12);
   p = malloc(12);
-} // expected-warning {{Potential leak of memory pointed to by}}
+} // expected-warning {{Potential leak of memory pointed to by}}\
+  // expected-warning {{Potential leak of memory pointed to by}}
 
 void mallocFreeMalloc() {
   int *p = malloc(12);

diff  --git a/clang/test/Analysis/pr22954.c b/clang/test/Analysis/pr22954.c
index e88acdc29d39..093f6311a505 100644
--- a/clang/test/Analysis/pr22954.c
+++ b/clang/test/Analysis/pr22954.c
@@ -352,6 +352,8 @@ int f19(int i) {
   memcpy(J0.s1[i].s1, input, 2);
   clang_analyzer_eval(J0.s1[0].s1[0] == 1); // expected-warning{{UNKNOWN}}\
   expected-warning{{Potential leak of memory pointed to by field 's2'}}\
+  expected-warning{{Potential leak of memory pointed to by field 's2'}}\
+  expected-warning{{Potential leak of memory pointed to by field 's2'}}\
   expected-warning{{Potential leak of memory pointed to by 'J0.s2'}}
   clang_analyzer_eval(J0.s1[0].s1[1] == 2); // expected-warning{{UNKNOWN}}
   clang_analyzer_eval(J0.s1[1].s1[0] == 3); // expected-warning{{UNKNOWN}}