Author: ziqingluo-90 Date: 2023-10-20T14:27:14-07:00 New Revision: a4323586fcbb20f39f00d5d1bc4a94a1aeea15c4
URL: https://github.com/llvm/llvm-project/commit/a4323586fcbb20f39f00d5d1bc4a94a1aeea15c4 DIFF: https://github.com/llvm/llvm-project/commit/a4323586fcbb20f39f00d5d1bc4a94a1aeea15c4.diff LOG: [-Wunsafe-buffer-usage] Add AST info to the unclaimed DRE debug notes for 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 starting with `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. Reviewed by: t-rasmud (Rashmi Mudduluru), NoQ (Artem Dergachev) Differential revision: https://reviews.llvm.org/D158561 Added: clang/test/SemaCXX/warn-unsafe-buffer-usage-debug-unclaimed/lit.local.cfg clang/test/SemaCXX/warn-unsafe-buffer-usage-debug-unclaimed/warn-unsafe-buffer-usage-debug-unclaimed.cpp clang/utils/analyze_safe_buffer_debug_notes.py Modified: clang/lib/Analysis/UnsafeBufferUsage.cpp Removed: ################################################################################ diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp b/clang/lib/Analysis/UnsafeBufferUsage.cpp index 49cfa7c0d3e3b27..e332a3609290aac 100644 --- a/clang/lib/Analysis/UnsafeBufferUsage.cpp +++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp @@ -8,7 +8,9 @@ #include "clang/Analysis/Analyses/UnsafeBufferUsage.h" #include "clang/AST/Decl.h" +#include "clang/AST/Expr.h" #include "clang/AST/RecursiveASTVisitor.h" +#include "clang/AST/StmtVisitor.h" #include "clang/ASTMatchers/ASTMatchFinder.h" #include "clang/Lex/Lexer.h" #include "clang/Lex/Preprocessor.h" @@ -22,6 +24,52 @@ using namespace llvm; using namespace clang; using namespace ast_matchers; +#ifndef NDEBUG +namespace { +class StmtDebugPrinter + : public ConstStmtVisitor<StmtDebugPrinter, std::string> { +public: + std::string VisitStmt(const Stmt *S) { return S->getStmtClassName(); } + + std::string VisitBinaryOperator(const BinaryOperator *BO) { + return "BinaryOperator(" + BO->getOpcodeStr().str() + ")"; + } + + std::string VisitUnaryOperator(const UnaryOperator *UO) { + return "UnaryOperator(" + UO->getOpcodeStr(UO->getOpcode()).str() + ")"; + } + + std::string VisitImplicitCastExpr(const ImplicitCastExpr *ICE) { + return "ImplicitCastExpr(" + std::string(ICE->getCastKindName()) + ")"; + } +}; + +// 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; + StmtDebugPrinter StmtPriner; + + do { + SS << StmtPriner.Visit(St); + + 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 diff erent callable of "n". @@ -2589,11 +2637,15 @@ void clang::checkUnsafeBufferUsage(const Decl *D, #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()) { diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-debug-unclaimed/lit.local.cfg b/clang/test/SemaCXX/warn-unsafe-buffer-usage-debug-unclaimed/lit.local.cfg new file mode 100644 index 000000000000000..07bac415a1a64c0 --- /dev/null +++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-debug-unclaimed/lit.local.cfg @@ -0,0 +1,11 @@ +# -*- Python -*- + +config.substitutions.append( + ( + "%analyze_safe_buffer_debug_notes", + "'%s' %s" % ( + config.python_executable, + os.path.join(config.clang_src_dir, "utils", "analyze_safe_buffer_debug_notes.py") + ) + ) +) diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-debug-unclaimed/warn-unsafe-buffer-usage-debug-unclaimed.cpp b/clang/test/SemaCXX/warn-unsafe-buffer-usage-debug-unclaimed/warn-unsafe-buffer-usage-debug-unclaimed.cpp new file mode 100644 index 000000000000000..ab3d925753d4788 --- /dev/null +++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-debug-unclaimed/warn-unsafe-buffer-usage-debug-unclaimed.cpp @@ -0,0 +1,49 @@ +// RUN: %clang_cc1 -Wno-unused-value -Wunsafe-buffer-usage -fsafe-buffer-usage-suggestions \ +// RUN: -mllvm -debug-only=SafeBuffers \ +// RUN: -std=c++20 -verify=expected %s + +// RUN: %clang_cc1 -Wno-unused-value -Wunsafe-buffer-usage -fsafe-buffer-usage-suggestions \ +// RUN: -mllvm -debug-only=SafeBuffers \ +// RUN: -std=c++20 %s \ +// RUN: 2>&1 | grep 'The unclaimed DRE trace:' \ +// RUN: | sed 's/^The unclaimed DRE trace://' \ +// RUN: | %analyze_safe_buffer_debug_notes \ +// RUN: | FileCheck %s + +// This debugging facility is only available in debug builds. +// +// REQUIRES: asserts +// REQUIRES: shell + +void test_unclaimed_use(int *p) { // expected-warning{{'p' is an unsafe pointer used for buffer access}} + p++; // expected-note{{used in pointer arithmetic here}} \ + expected-note{{safe buffers debug: failed to produce fixit for 'p' : has an unclaimed use\n \ + The unclaimed DRE trace: DeclRefExpr, UnaryOperator(++), CompoundStmt}} + *((p + 1) + 1); // expected-warning{{unsafe pointer arithmetic}} \ + expected-note{{used in pointer arithmetic here}} \ + expected-note{{safe buffers debug: failed to produce fixit for 'p' : has an unclaimed use\n \ + The unclaimed DRE trace: DeclRefExpr, ImplicitCastExpr(LValueToRValue), BinaryOperator(+), ParenExpr, BinaryOperator(+), ParenExpr, UnaryOperator(*), CompoundStmt}} + p -= 1; // expected-note{{used in pointer arithmetic here}} \ + expected-note{{safe buffers debug: failed to produce fixit for 'p' : has an unclaimed use\n \ + The unclaimed DRE trace: DeclRefExpr, BinaryOperator(-=), CompoundStmt}} + p--; // expected-note{{used in pointer arithmetic here}} \ + expected-note{{safe buffers debug: failed to produce fixit for 'p' : has an unclaimed use\n \ + The unclaimed DRE trace: DeclRefExpr, UnaryOperator(--), CompoundStmt}} + p[5] = 5; // expected-note{{used in buffer access here}} +} + +// CHECK: Root # 1 +// CHECK: |- DeclRefExpr # 4 +// CHECK: |-- UnaryOperator(++) # 1 +// CHECK: |--- CompoundStmt # 1 +// CHECK: |-- ImplicitCastExpr(LValueToRValue) # 1 +// CHECK: |--- BinaryOperator(+) # 1 +// CHECK: |---- ParenExpr # 1 +// CHECK: |----- BinaryOperator(+) # 1 +// CHECK: |------ ParenExpr # 1 +// CHECK: |------- UnaryOperator(*) # 1 +// CHECK: |-------- CompoundStmt # 1 +// CHECK: |-- BinaryOperator(-=) # 1 +// CHECK: |--- CompoundStmt # 1 +// CHECK: |-- UnaryOperator(--) # 1 +// CHECK: |--- CompoundStmt # 1 diff --git a/clang/utils/analyze_safe_buffer_debug_notes.py b/clang/utils/analyze_safe_buffer_debug_notes.py new file mode 100644 index 000000000000000..f0a6e4de6fdbe6b --- /dev/null +++ b/clang/utils/analyze_safe_buffer_debug_notes.py @@ -0,0 +1,39 @@ +import sys +from collections import OrderedDict + +class Trie: + def __init__(self, name): + self.name = name + self.children = OrderedDict() + 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") + +if __name__ == "__main__": + for line in sys.stdin: + words = line.split('==>') + words = [word.strip() for word in words] + MyTrie = Root; + for word in words: + MyTrie = MyTrie.add(word) + + Root.print(0) _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
