ziqingluo-90 created this revision.
ziqingluo-90 added reviewers: jkorous, NoQ, t-rasmud, malavikasamak.
Herald added a project: All.
ziqingluo-90 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

The debug note for an unclaimed DRE highlights a usage of an unsafe pointer 
that is not supported by our analysis.

For a better understand of what the unsupported cases are,  we add more 
information to the debug note---a string of ancestor AST nodes of the unclaimed 
DRE.    For example, an unclaimed DRE `p` in an expression `*(p++)` will result 
in a string `DRE, UnaryOperator(++), Paren, UnaryOperator(*)`.

To find out the most common patterns of those unsupported use cases,  we add a 
simple script to build a prefix tree over those strings and count each prefix.  
 The script reads input line by line, assumes a line is a list of words 
separated by `,`s, and builds a prefix tree over those lists.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D158561

Files:
  clang/lib/Analysis/UnsafeBufferUsage.cpp
  clang/utils/analyze_safe_buffer_debug_notes.py

Index: clang/utils/analyze_safe_buffer_debug_notes.py
===================================================================
--- /dev/null
+++ clang/utils/analyze_safe_buffer_debug_notes.py
@@ -0,0 +1,44 @@
+class Trie:
+    def __init__(self, name):
+        self.name = name
+        self.children = {}
+        self.count = 1
+        
+    def add(self, name):
+        if name in self.children:
+            self.children[name].count += 1
+        else:
+            self.children[name] = Trie(name)
+        return self.children[name]
+            
+    def print(self, depth):
+        if depth > 0:
+            print('|', end="")
+        for i in range(depth):
+            print('-', end="")
+        if depth > 0:
+            print(end=" ")
+        print(self.name, '#', self.count)
+        for key, child in self.children.items():
+            child.print(depth + 1)
+            
+
+Root = Trie("Root")            
+
+def main():    
+    while True:
+        try:
+            line = input()
+        except EOFError:
+            break
+        
+        words = line.split(',')
+        words = [word.strip() for word in words]
+        MyTrie = Root;
+        for word in words:
+            MyTrie = MyTrie.add(word)            
+
+    Root.print(0)
+
+if __name__ == "__main__":
+    main()
Index: clang/lib/Analysis/UnsafeBufferUsage.cpp
===================================================================
--- clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -8,6 +8,7 @@
 
 #include "clang/Analysis/Analyses/UnsafeBufferUsage.h"
 #include "clang/AST/Decl.h"
+#include "clang/AST/Expr.h"
 #include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/Lex/Lexer.h"
@@ -22,6 +23,84 @@
 using namespace clang;
 using namespace ast_matchers;
 
+#ifndef NDEBUG
+namespace {
+static std::string printUnaryOperator(const Stmt *Stmt) {
+  const UnaryOperator *UO = cast<UnaryOperator>(Stmt);
+  return "UnaryOperator(" + UO->getOpcodeStr(UO->getOpcode()).str() + ")";
+}
+
+static std::string printBinaryOperator(const Stmt *Stmt) {
+  const BinaryOperator *BO = cast<BinaryOperator>(Stmt);
+  StringRef BOOpStr = BO->getOpcodeStr();
+
+  if (BO->getOpcode() == BinaryOperator::Opcode::BO_Comma)
+    BOOpStr = "COMMA"; // Do not print `,` as it is used as the separator
+  return "BinaryOperator(" + BOOpStr.str() + ")";
+}
+
+static std::string printCompoundAssignOperator(const Stmt *Stmt) {
+  return printBinaryOperator(Stmt);
+}
+
+static std::string printImplicitCastExpr(const Stmt *Stmt) {
+  const CastExpr *CE = cast<CastExpr>(Stmt);
+  return "ImplicitCastExpr(" + std::string(CE->getCastKindName()) + ")";
+}
+
+// Prints the `StmtClass` of the given `Stmt`.  For `UnaryOperator`s and
+// `BinaryOperator`s, it also prints the operator.  For `CastExpr`, it also
+// prints the cast kind.
+static std::optional<std::string> printStmtClass(const Stmt *Stmt) {
+  switch (Stmt->getStmtClass()) {
+#define STMT(CLASS, PARENT)                                                    \
+  case Stmt::CLASS##Class:                                                     \
+    return #CLASS;
+#define UNARYOPERATOR(TYPE, BASE)                                              \
+  case Stmt::TYPE##Class:                                                      \
+    return print##TYPE(Stmt);
+#define BINARYOPERATOR(TYPE, BASE)                                             \
+  case Stmt::TYPE##Class:                                                      \
+    return print##TYPE(Stmt);
+#define IMPLICITCASTEXPR(TYPE, BASE)                                           \
+  case Stmt::TYPE##Class:                                                      \
+    return print##TYPE(Stmt);
+#define ABSTRACT_STMT(STMT)
+#include "clang/AST/StmtNodes.inc"
+  default:
+    return std::nullopt;
+  }
+}
+
+// Returns a string of ancestor `Stmt`s of the given `DRE` in such a form:
+// "DRE, parent-of-DRE, grandparent-of-DRE, ...".
+static std::string getDREAncestorString(const DeclRefExpr *DRE,
+                                        ASTContext &Ctx) {
+  std::stringstream SS;
+  const Stmt *St = DRE;
+
+  do {
+    if (auto Printed = printStmtClass(St))
+      SS << *Printed;
+    else
+      return "unavailable due to unknown statement: " +
+             std::string(St->getStmtClassName());
+
+    DynTypedNodeList StParents = Ctx.getParents(*St);
+
+    if (StParents.size() > 1)
+      return "unavailable due to multiple parents";
+    if (StParents.size() == 0)
+      break;
+    St = StParents.begin()->get<Stmt>();
+    if (St)
+      SS << ", ";
+  } while (St);
+  return SS.str();
+}
+} // namespace
+#endif /* NDEBUG */
+
 namespace clang::ast_matchers {
 // A `RecursiveASTVisitor` that traverses all descendants of a given node "n"
 // except for those belonging to a different callable of "n".
@@ -2446,11 +2525,15 @@
 #ifndef NDEBUG
         auto AllUnclaimed = Tracker.getUnclaimedUses(it->first);
         for (auto UnclaimedDRE : AllUnclaimed) {
-          Handler.addDebugNoteForVar(
-              it->first, UnclaimedDRE->getBeginLoc(),
-                                     ("failed to produce fixit for '" + it->first->getNameAsString() +
-                                      "' : has an unclaimed use"));
-          }
+        std::string UnclaimedUseTrace =
+            getDREAncestorString(UnclaimedDRE, D->getASTContext());
+
+        Handler.addDebugNoteForVar(
+            it->first, UnclaimedDRE->getBeginLoc(),
+            ("failed to produce fixit for '" + it->first->getNameAsString() +
+             "' : has an unclaimed use\nThe unclaimed DRE trace: " +
+             UnclaimedUseTrace));
+        }
 #endif
         it = FixablesForAllVars.byVar.erase(it);
       } else if (it->first->isInitCapture()) {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
  • [PATCH] D158561: [-Wunsafe-buff... Ziqing Luo via Phabricator via cfe-commits

Reply via email to