[analyzer] CloneDetector allows comparing clones for suspicious variable pattern errors.

[teemperor]

Summary

One of the goals of the project was to find bugs caused by copy-pasting, which happen when a piece of code is copied but not all variables in this piece of code are correctly adapted to the new environment (e.g. like this 'if (mutex1.try_lock()) { foo(); mutex2.unlock(); }' where the mutex2 variable is a leftover from the original source code).

This patch adds support vor analyzing the pattern of the referenced variables for similar errors. So far only 'one-off' errors are supported, i.e. pattern errors where only one variable is not following the pattern of its related clones.

Additionally, the CloneChecker - which is currently the primary user of the CloneDetector API - now also supports reporting these suspicious clones.

[v.g.vassilev]

lib/StaticAnalyzer/Checkers/CloneChecker.cpp

83 ↗

(On Diff #67324)

More common in clang is to use "Did you mean to use...". If it is a warning it should be saying something like: "Suspicious code clone detected; did you mean to use %0"

86 ↗

(On Diff #67324)

Typo: usage.

89 ↗

(On Diff #67324)

I think this should be worded a bit better, too.

test/Analysis/copypaste/suspicious-clones.cpp

11 ↗

(On Diff #67324)

This note here is very obscure. It doesn't contain a lot of meaningful information. It should give a hint where is this similar piece of code.

[NoQ]

I couldn't find any real problems - the code's getting mature! Please accept a few very important "please rename this variable" comments :)

include/clang/Analysis/CloneDetection.h

236 ↗

(On Diff #67324)

It surprised me that you cannot really retrieve the clone (i.e. statement sequence) from the SuspiciousClone object. Maybe rename to eg. SuspiciousCloneInfo?

lib/Analysis/CloneDetection.cpp

92 ↗

(On Diff #67324)

It's surprising that Location is not of type SourceLocation, so much that i'd agree to rename it to Range, no matter how generic this word is.

174 ↗

(On Diff #67324)

Because this function doesn't return pattern differences, maybe rename to countPatternDifferences()? And the optional parameter kinda comes as a bonus feature.

183 ↗

(On Diff #67324)

Maybe reduce indent here through if (... == ...) continue, like in other places?

lib/StaticAnalyzer/Checkers/CloneChecker.cpp

39 ↗

(On Diff #67324)

Maybe move those out-of-line? They look pretty big.

43 ↗

(On Diff #67324)

I renamed the member variable to Detector in r276791, because it caused compile errors on buildbots (because class and member have the same name), so this patch would need to be patched up a bit - here and in one more place.

97 ↗

(On Diff #67324)

This line probably explains why i'm so much into renaming variables: it reads as if we're putting the location of the clone into the report, even though we're putting the location of a variable reference into it. Pair.FirstCloneInfo.VarRange would have been much more intuitive.

[teemperor]

• Made warning messages more 'clangish' as pointed out by Vassil (Thanks!)
• Improved the class/variable names as pointed out by Artem (Thanks a lot!)
• CloneChecker's functions are now out-of-line.

test/Analysis/copypaste/suspicious-clones.cpp

11 ↗

(On Diff #67324)

The note is paired with a warning as discussed in last meeting, so I'll mark this as done :).

[v.g.vassilev]

LGTM.

cfe/trunk/include/clang/Analysis/CloneDetection.h

 #include "llvm/ADT/StringMap.h"
 
 #include <vector>
 
 namespace clang {
 
 class Stmt;
 class Decl;
+class VarDecl;
 class ASTContext;
 class CompoundStmt;
 
 /// \brief Identifies a list of statements.
 ///
 /// Can either identify a single arbitrary Stmt object, a continuous sequence of
 /// child statements inside a CompoundStmt or no statements at all.
 class StmtSequence {
public:
 
   /// \brief Searches the provided statements for clones.
   ///
   /// \param Result Output parameter that is filled with a list of found
   ///               clone groups. Each group contains multiple StmtSequences
   ///               that were identified to be clones of each other.
   /// \param MinGroupComplexity Only return clones which have at least this
   ///                           complexity value.
-  void findClones(std::vector<CloneGroup> &Result, unsigned MinGroupComplexity);
+  /// \param CheckPatterns Returns only clone groups in which the referenced
+  ///                      variables follow the same pattern.
+  void findClones(std::vector<CloneGroup> &Result, unsigned MinGroupComplexity,
+                  bool CheckPatterns = true);
+
+  /// \brief Describes two clones that reference their variables in a different
+  ///        pattern which could indicate a programming error.
+  struct SuspiciousClonePair {
+    /// \brief Utility class holding the relevant information about a single
+    ///        clone in this pair.
+    struct SuspiciousCloneInfo {
+      /// The variable which referencing in this clone was against the pattern.
+      const VarDecl *Variable;
+      /// Where the variable was referenced.
+      SourceRange VarRange;
+      /// The variable that should have been referenced to follow the pattern.
+      /// If Suggestion is a nullptr then it's not possible to fix the pattern
+      /// by referencing a different variable in this clone.
+      const VarDecl *Suggestion;
+      SuspiciousCloneInfo(const VarDecl *Variable, SourceRange Range,
+                          const VarDecl *Suggestion)
+          : Variable(Variable), VarRange(Range), Suggestion(Suggestion) {}
+      SuspiciousCloneInfo() {}
+    };
+    /// The first clone in the pair which always has a suggested variable.
+    SuspiciousCloneInfo FirstCloneInfo;
+    /// This other clone in the pair which can have a suggested variable.
+    SuspiciousCloneInfo SecondCloneInfo;
+  };
+
+  /// \brief Searches the provided statements for pairs of clones that don't
+  ///        follow the same pattern when referencing variables.
+  /// \param Result Output parameter that will contain the clone pairs.
+  /// \param MinGroupComplexity Only clone pairs in which the clones have at
+  ///                           least this complexity value.
+  void findSuspiciousClones(std::vector<SuspiciousClonePair> &Result,
+                            unsigned MinGroupComplexity);
 
 private:
   /// Stores all found clone groups including invalid groups with only a single
   /// statement.
   std::vector<CloneGroup> CloneGroups;
   /// Maps search data to its related index in the \p CloneGroups vector.
   llvm::StringMap<std::size_t> CloneGroupIndexes;
 };
 
 } // end namespace clang
 
 #endif // LLVM_CLANG_AST_CLONEDETECTION_H

cfe/trunk/lib/Analysis/CloneDetection.cpp

 
 /// \brief Analyzes the pattern of the referenced variables in a statement.
 class VariablePattern {
 
   /// \brief Describes an occurence of a variable reference in a statement.
   struct VariableOccurence {
     /// The index of the associated VarDecl in the Variables vector.
     size_t KindID;
+    /// The source range in the code where the variable was referenced.
+    SourceRange Range;
 
-    VariableOccurence(size_t KindID) : KindID(KindID) {}
+    VariableOccurence(size_t KindID, SourceRange Range)
+        : KindID(KindID), Range(Range) {}
   };
 
   /// All occurences of referenced variables in the order of appearance.
   std::vector<VariableOccurence> Occurences;
   /// List of referenced variables in the order of appearance.
   /// Every item in this list is unique.
   std::vector<const VarDecl *> Variables;
 
   /// \brief Adds a new variable referenced to this pattern.
   /// \param VarDecl The declaration of the variable that is referenced.
-  void addVariableOccurence(const VarDecl *VarDecl) {
+  /// \param Range The SourceRange where this variable is referenced.
+  void addVariableOccurence(const VarDecl *VarDecl, SourceRange Range) {
     // First check if we already reference this variable
     for (size_t KindIndex = 0; KindIndex < Variables.size(); ++KindIndex) {
       if (Variables[KindIndex] == VarDecl) {
         // If yes, add a new occurence that points to the existing entry in
         // the Variables vector.
-        Occurences.emplace_back(KindIndex);
+        Occurences.emplace_back(KindIndex, Range);
         return;
       }
     }
     // If this variable wasn't already referenced, add it to the list of
     // referenced variables and add a occurence that points to this new entry.
-    Occurences.emplace_back(Variables.size());
+    Occurences.emplace_back(Variables.size(), Range);
     Variables.push_back(VarDecl);
   }
 
   /// \brief Adds each referenced variable from the given statement.
   void addVariables(const Stmt *S) {
     // Sometimes we get a nullptr (such as from IfStmts which often have nullptr
     // children). We skip such statements as they don't reference any
     // variables.
     if (!S)
       return;
 
     // Check if S is a reference to a variable. If yes, add it to the pattern.
     if (auto D = dyn_cast<DeclRefExpr>(S)) {
       if (auto VD = dyn_cast<VarDecl>(D->getDecl()->getCanonicalDecl()))
-        addVariableOccurence(VD);
+        addVariableOccurence(VD, D->getSourceRange());
     }
 
     // Recursively check all children of the given statement.
     for (const Stmt *Child : S->children()) {
       addVariables(Child);
     }
   }
 
 public:
   /// \brief Creates an VariablePattern object with information about the given
   ///        StmtSequence.
   VariablePattern(const StmtSequence &Sequence) {
     for (const Stmt *S : Sequence)
       addVariables(S);
   }
 
-  /// \brief Compares this pattern with the given one.
+  /// \brief Counts the differences between this pattern and the given one.
   /// \param Other The given VariablePattern to compare with.
-  /// \return Returns true if and only if the references variables in this
-  ///         object follow the same pattern than the ones in the given
-  ///         VariablePattern.
+  /// \param FirstMismatch Output parameter that will be filled with information
+  ///        about the first difference between the two patterns. This parameter
+  ///        can be a nullptr, in which case it will be ignored.
+  /// \return Returns the number of differences between the pattern this object
+  ///         is following and the given VariablePattern.
   ///
-  /// For example, the following statements all have the same pattern:
+  /// For example, the following statements all have the same pattern and this
+  /// function would return zero:
   ///
   ///   if (a < b) return a; return b;
   ///   if (x < y) return x; return y;
   ///   if (u2 < u1) return u2; return u1;
   ///
-  /// but the following statement has a different pattern (note the changed
-  /// variables in the return statements).
+  /// But the following statement has a different pattern (note the changed
+  /// variables in the return statements) and would have two differences when
+  /// compared with one of the statements above.
   ///
   ///   if (a < b) return b; return a;
   ///
   /// This function should only be called if the related statements of the given
   /// pattern and the statements of this objects are clones of each other.
-  bool comparePattern(const VariablePattern &Other) {
+  unsigned countPatternDifferences(
+      const VariablePattern &Other,
+      CloneDetector::SuspiciousClonePair *FirstMismatch = nullptr) {
+    unsigned NumberOfDifferences = 0;
+
     assert(Other.Occurences.size() == Occurences.size());
     for (unsigned i = 0; i < Occurences.size(); ++i) {
-      if (Occurences[i].KindID != Other.Occurences[i].KindID) {
-        return false;
-      }
+      auto ThisOccurence = Occurences[i];
+      auto OtherOccurence = Other.Occurences[i];
+      if (ThisOccurence.KindID == OtherOccurence.KindID)
+        continue;
+
+      ++NumberOfDifferences;
+
+      // If FirstMismatch is not a nullptr, we need to store information about
+      // the first difference between the two patterns.
+      if (FirstMismatch == nullptr)
+        continue;
+
+      // Only proceed if we just found the first difference as we only store
+      // information about the first difference.
+      if (NumberOfDifferences != 1)
+        continue;
+
+      const VarDecl *FirstSuggestion = nullptr;
+      // If there is a variable available in the list of referenced variables
+      // which wouldn't break the pattern if it is used in place of the
+      // current variable, we provide this variable as the suggested fix.
+      if (OtherOccurence.KindID < Variables.size())
+        FirstSuggestion = Variables[OtherOccurence.KindID];
+
+      // Store information about the first clone.
+      FirstMismatch->FirstCloneInfo =
+          CloneDetector::SuspiciousClonePair::SuspiciousCloneInfo(
+              Variables[ThisOccurence.KindID], ThisOccurence.Range,
+              FirstSuggestion);
+
+      // Same as above but with the other clone. We do this for both clones as
+      // we don't know which clone is the one containing the unintended
+      // pattern error.
+      const VarDecl *SecondSuggestion = nullptr;
+      if (ThisOccurence.KindID < Other.Variables.size())
+        SecondSuggestion = Other.Variables[ThisOccurence.KindID];
+
+      // Store information about the second clone.
+      FirstMismatch->SecondCloneInfo =
+          CloneDetector::SuspiciousClonePair::SuspiciousCloneInfo(
+              Variables[ThisOccurence.KindID], OtherOccurence.Range,
+              SecondSuggestion);
+
+      // SuspiciousClonePair guarantees that the first clone always has a
+      // suggested variable associated with it. As we know that one of the two
+      // clones in the pair always has suggestion, we swap the two clones
+      // in case the first clone has no suggested variable which means that
+      // the second clone has a suggested variable and should be first.
+      if (!FirstMismatch->FirstCloneInfo.Suggestion)
+        std::swap(FirstMismatch->FirstCloneInfo,
+                  FirstMismatch->SecondCloneInfo);
+
+      // This ensures that we always have at least one suggestion in a pair.
+      assert(FirstMismatch->FirstCloneInfo.Suggestion);
     }
-    return true;
+
+    return NumberOfDifferences;
   }
 };
 }
 
 namespace {
 /// \brief Collects the data of a single Stmt.
 ///
 /// This class defines what a code clone is: If it collects for two statements
while (UnassignedSequences.size() > 1) {
     // Analyze the variable pattern of the prototype. Every other StmtSequence
     // needs to have the same pattern to get into the new clone group.
     VariablePattern PrototypeFeatures(Prototype);
 
     // Search all remaining StmtSequences for an identical variable pattern
     // and assign them to our new clone group.
     auto I = UnassignedSequences.begin(), E = UnassignedSequences.end();
     while (I != E) {
-      if (VariablePattern(*I).comparePattern(PrototypeFeatures)) {
+
+      if (VariablePattern(*I).countPatternDifferences(PrototypeFeatures) == 0) {
         FilteredGroup.Sequences.push_back(*I);
         I = UnassignedSequences.erase(I);
         continue;
       }
       ++I;
     }
 
     // Add a valid clone group to the list of found clone groups.
     if (!FilteredGroup.isValid())
       continue;
 
     Result.push_back(FilteredGroup);
   }
 }
 
 void CloneDetector::findClones(std::vector<CloneGroup> &Result,
-                               unsigned MinGroupComplexity) {
+                               unsigned MinGroupComplexity,
+                               bool CheckPatterns) {
   // Add every valid clone group that fulfills the complexity requirement.
   for (const CloneGroup &Group : CloneGroups) {
     if (Group.isValid() && Group.Complexity >= MinGroupComplexity) {
+      if (CheckPatterns)
         createCloneGroups(Result, Group);
+      else
+        Result.push_back(Group);
     }
   }
 
   std::vector<unsigned> IndexesToRemove;
 
   // Compare every group in the result with the rest. If one groups contains
   // another group, we only need to return the bigger group.
   // Note: This doesn't scale well, so if possible avoid calling any heavy
void CloneDetector::findClones(std::vector<CloneGroup> &Result,
 
   // Erasing a list of indexes from the vector should be done with decreasing
   // indexes. As IndexesToRemove is constructed with increasing values, we just
   // reverse iterate over it to get the desired order.
   for (auto I = IndexesToRemove.rbegin(); I != IndexesToRemove.rend(); ++I) {
     Result.erase(Result.begin() + *I);
   }
 }
+
+void CloneDetector::findSuspiciousClones(
+    std::vector<CloneDetector::SuspiciousClonePair> &Result,
+    unsigned MinGroupComplexity) {
+  std::vector<CloneGroup> Clones;
+  // Reuse the normal search for clones but specify that the clone groups don't
+  // need to have a common referenced variable pattern so that we can manually
+  // search for the kind of pattern errors this function is supposed to find.
+  findClones(Clones, MinGroupComplexity, false);
+
+  for (const CloneGroup &Group : Clones) {
+    for (unsigned i = 0; i < Group.Sequences.size(); ++i) {
+      VariablePattern PatternA(Group.Sequences[i]);
+
+      for (unsigned j = i + 1; j < Group.Sequences.size(); ++j) {
+        VariablePattern PatternB(Group.Sequences[j]);
+
+        CloneDetector::SuspiciousClonePair ClonePair;
+        // For now, we only report clones which break the variable pattern just
+        // once because multiple differences in a pattern are an indicator that
+        // those differences are maybe intended (e.g. because it's actually
+        // a different algorithm).
+        // TODO: In very big clones even multiple variables can be unintended,
+        // so replacing this number with a percentage could better handle such
+        // cases. On the other hand it could increase the false-positive rate
+        // for all clones if the percentage is too high.
+        if (PatternA.countPatternDifferences(PatternB, &ClonePair) == 1) {
+          Result.push_back(ClonePair);
+          break;
+        }
+      }
+    }
+  }
+}

cfe/trunk/lib/StaticAnalyzer/Checkers/CloneChecker.cpp

class CloneChecker
   mutable CloneDetector Detector;
 
 public:
   void checkASTCodeBody(const Decl *D, AnalysisManager &Mgr,
                         BugReporter &BR) const;
 
   void checkEndOfTranslationUnit(const TranslationUnitDecl *TU,
                                  AnalysisManager &Mgr, BugReporter &BR) const;
+
+  /// \brief Reports all clones to the user.
+  void reportClones(SourceManager &SM, AnalysisManager &Mgr,
+                    int MinComplexity) const;
+
+  /// \brief Reports only suspicious clones to the user along with informaton
+  ///        that explain why they are suspicious.
+  void reportSuspiciousClones(SourceManager &SM, AnalysisManager &Mgr,
+                              int MinComplexity) const;
 };
 } // end anonymous namespace
 
 void CloneChecker::checkASTCodeBody(const Decl *D, AnalysisManager &Mgr,
                                     BugReporter &BR) const {
   // Every statement that should be included in the search for clones needs to
   // be passed to the CloneDetector.
   Detector.analyzeCodeBody(D);
 }
 
 void CloneChecker::checkEndOfTranslationUnit(const TranslationUnitDecl *TU,
                                              AnalysisManager &Mgr,
                                              BugReporter &BR) const {
   // At this point, every statement in the translation unit has been analyzed by
   // the CloneDetector. The only thing left to do is to report the found clones.
 
   int MinComplexity = Mgr.getAnalyzerOptions().getOptionAsInteger(
       "MinimumCloneComplexity", 10, this);
-
   assert(MinComplexity >= 0);
 
-  SourceManager &SM = BR.getSourceManager();
+  bool ReportSuspiciousClones = Mgr.getAnalyzerOptions().getBooleanOption(
+      "ReportSuspiciousClones", true, this);
+
+  bool ReportNormalClones = Mgr.getAnalyzerOptions().getBooleanOption(
+      "ReportNormalClones", true, this);
+
+  if (ReportSuspiciousClones)
+    reportSuspiciousClones(BR.getSourceManager(), Mgr, MinComplexity);
+
+  if (ReportNormalClones)
+    reportClones(BR.getSourceManager(), Mgr, MinComplexity);
+}
+
+void CloneChecker::reportClones(SourceManager &SM, AnalysisManager &Mgr,
+                                int MinComplexity) const {
 
   std::vector<CloneDetector::CloneGroup> CloneGroups;
   Detector.findClones(CloneGroups, MinComplexity);
 
   DiagnosticsEngine &DiagEngine = Mgr.getDiagnostic();
 
   unsigned WarnID = DiagEngine.getCustomDiagID(DiagnosticsEngine::Warning,
                                                "Detected code clone.");
for (CloneDetector::CloneGroup &Group : CloneGroups) {
     // as a note.
     DiagEngine.Report(Group.Sequences.front().getStartLoc(), WarnID);
     for (unsigned i = 1; i < Group.Sequences.size(); ++i) {
       DiagEngine.Report(Group.Sequences[i].getStartLoc(), NoteID);
     }
   }
 }
 
+void CloneChecker::reportSuspiciousClones(SourceManager &SM,
+                                          AnalysisManager &Mgr,
+                                          int MinComplexity) const {
+
+  std::vector<CloneDetector::SuspiciousClonePair> Clones;
+  Detector.findSuspiciousClones(Clones, MinComplexity);
+
+  DiagnosticsEngine &DiagEngine = Mgr.getDiagnostic();
+
+  auto SuspiciousCloneWarning = DiagEngine.getCustomDiagID(
+      DiagnosticsEngine::Warning, "suspicious code clone detected; did you "
+                                  "mean to use %0?");
+
+  auto RelatedCloneNote = DiagEngine.getCustomDiagID(
+      DiagnosticsEngine::Note, "suggestion is based on the usage of this "
+                               "variable in a similar piece of code");
+
+  auto RelatedSuspiciousCloneNote = DiagEngine.getCustomDiagID(
+      DiagnosticsEngine::Note, "suggestion is based on the usage of this "
+                               "variable in a similar piece of code; did you "
+                               "mean to use %0?");
+
+  for (CloneDetector::SuspiciousClonePair &Pair : Clones) {
+    // The first clone always has a suggestion and we report it to the user
+    // along with the place where the suggestion should be used.
+    DiagEngine.Report(Pair.FirstCloneInfo.VarRange.getBegin(),
+                      SuspiciousCloneWarning)
+        << Pair.FirstCloneInfo.VarRange << Pair.FirstCloneInfo.Suggestion;
+
+    // The second clone can have a suggestion and if there is one, we report
+    // that suggestion to the user.
+    if (Pair.SecondCloneInfo.Suggestion) {
+      DiagEngine.Report(Pair.SecondCloneInfo.VarRange.getBegin(),
+                        RelatedSuspiciousCloneNote)
+          << Pair.SecondCloneInfo.VarRange << Pair.SecondCloneInfo.Suggestion;
+      continue;
+    }
+
+    // If there isn't a suggestion in the second clone, we only inform the
+    // user where we got the idea that his code could contain an error.
+    DiagEngine.Report(Pair.SecondCloneInfo.VarRange.getBegin(),
+                      RelatedCloneNote)
+        << Pair.SecondCloneInfo.VarRange;
+  }
+}
+
 //===----------------------------------------------------------------------===//
 // Register CloneChecker
 //===----------------------------------------------------------------------===//
 
 void ento::registerCloneChecker(CheckerManager &Mgr) {
   Mgr.registerChecker<CloneChecker>();
 }

cfe/trunk/test/Analysis/copypaste/suspicious-clones.cpp

+// RUN: %clang_cc1 -analyze -analyzer-checker=alpha.clone.CloneChecker -analyzer-config alpha.clone.CloneChecker:ReportSuspiciousClones=true  -analyzer-config alpha.clone.CloneChecker:ReportNormalClones=false -verify %s
+
+// Tests finding a suspicious clone that references local variables.
+
+void log();
+
+int max(int a, int b) {
+  log();
+  if (a > b)
+    return a;
+  return b; // expected-note{{suggestion is based on the usage of this variable in a similar piece of code}}
+}
+
+int maxClone(int x, int y, int z) {
+  log();
+  if (x > y)
+    return x;
+  return z; // expected-warning{{suspicious code clone detected; did you mean to use 'y'?}}
+}
+
+// Tests finding a suspicious clone that references global variables.
+
+struct mutex {
+  bool try_lock();
+  void unlock();
+};
+
+mutex m1;
+mutex m2;
+int i;
+
+void busyIncrement() {
+  while (true) {
+    if (m1.try_lock()) {
+      ++i;
+      m1.unlock(); // expected-note{{suggestion is based on the usage of this variable in a similar piece of code}}
+      if (i > 1000) {
+        return;
+      }
+    }
+  }
+}
+
+void faultyBusyIncrement() {
+  while (true) {
+    if (m1.try_lock()) {
+      ++i;
+      m2.unlock();  // expected-warning{{suspicious code clone detected; did you mean to use 'm1'?}}
+      if (i > 1000) {
+        return;
+      }
+    }
+  }
+}
+
+// Tests that we provide two suggestions in cases where two fixes are possible.
+
+int foo(int a, int b, int c) {
+  a += b + c;
+  b /= a + b;
+  c -= b * a; // expected-warning{{suspicious code clone detected; did you mean to use 'a'?}}
+  return c;
+}
+
+int fooClone(int a, int b, int c) {
+  a += b + c;
+  b /= a + b;
+  c -= a * a; // expected-note{{suggestion is based on the usage of this variable in a similar piece of code; did you mean to use 'b'?}}
+  return c;
+}
+
+
+// Tests that for clone groups with a many possible suspicious clone pairs, at
+// most one warning per clone group is generated and every relevant clone is
+// reported through either a warning or a note.
+
+long bar1(long a, long b, long c, long d) {
+  c = a - b;
+  c = c / d * a;
+  d = b * b - c; // expected-warning{{suspicious code clone detected; did you mean to use 'c'?}}
+  return d;
+}
+
+long bar2(long a, long b, long c, long d) {
+  c = a - b;
+  c = c / d * a;
+  d = c * b - c; // expected-note{{suggestion is based on the usage of this variable in a similar piece of code; did you mean to use 'b'?}} \
+                 // expected-warning{{suspicious code clone detected; did you mean to use 'a'?}}
+  return d;
+}
+
+long bar3(long a, long b, long c, long d) {
+  c = a - b;
+  c = c / d * a;
+  d = a * b - c; // expected-note{{suggestion is based on the usage of this variable in a similar piece of code; did you mean to use 'c'?}}
+  return d;
+}