Skip to content

Commit a432358

Browse files
committed
[-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
1 parent a969404 commit a432358

File tree

4 files changed

+156
-5
lines changed

4 files changed

+156
-5
lines changed

clang/lib/Analysis/UnsafeBufferUsage.cpp

Lines changed: 57 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,9 @@
88

99
#include "clang/Analysis/Analyses/UnsafeBufferUsage.h"
1010
#include "clang/AST/Decl.h"
11+
#include "clang/AST/Expr.h"
1112
#include "clang/AST/RecursiveASTVisitor.h"
13+
#include "clang/AST/StmtVisitor.h"
1214
#include "clang/ASTMatchers/ASTMatchFinder.h"
1315
#include "clang/Lex/Lexer.h"
1416
#include "clang/Lex/Preprocessor.h"
@@ -22,6 +24,52 @@ using namespace llvm;
2224
using namespace clang;
2325
using namespace ast_matchers;
2426

27+
#ifndef NDEBUG
28+
namespace {
29+
class StmtDebugPrinter
30+
: public ConstStmtVisitor<StmtDebugPrinter, std::string> {
31+
public:
32+
std::string VisitStmt(const Stmt *S) { return S->getStmtClassName(); }
33+
34+
std::string VisitBinaryOperator(const BinaryOperator *BO) {
35+
return "BinaryOperator(" + BO->getOpcodeStr().str() + ")";
36+
}
37+
38+
std::string VisitUnaryOperator(const UnaryOperator *UO) {
39+
return "UnaryOperator(" + UO->getOpcodeStr(UO->getOpcode()).str() + ")";
40+
}
41+
42+
std::string VisitImplicitCastExpr(const ImplicitCastExpr *ICE) {
43+
return "ImplicitCastExpr(" + std::string(ICE->getCastKindName()) + ")";
44+
}
45+
};
46+
47+
// Returns a string of ancestor `Stmt`s of the given `DRE` in such a form:
48+
// "DRE ==> parent-of-DRE ==> grandparent-of-DRE ==> ...".
49+
static std::string getDREAncestorString(const DeclRefExpr *DRE,
50+
ASTContext &Ctx) {
51+
std::stringstream SS;
52+
const Stmt *St = DRE;
53+
StmtDebugPrinter StmtPriner;
54+
55+
do {
56+
SS << StmtPriner.Visit(St);
57+
58+
DynTypedNodeList StParents = Ctx.getParents(*St);
59+
60+
if (StParents.size() > 1)
61+
return "unavailable due to multiple parents";
62+
if (StParents.size() == 0)
63+
break;
64+
St = StParents.begin()->get<Stmt>();
65+
if (St)
66+
SS << " ==> ";
67+
} while (St);
68+
return SS.str();
69+
}
70+
} // namespace
71+
#endif /* NDEBUG */
72+
2573
namespace clang::ast_matchers {
2674
// A `RecursiveASTVisitor` that traverses all descendants of a given node "n"
2775
// except for those belonging to a different callable of "n".
@@ -2589,11 +2637,15 @@ void clang::checkUnsafeBufferUsage(const Decl *D,
25892637
#ifndef NDEBUG
25902638
auto AllUnclaimed = Tracker.getUnclaimedUses(it->first);
25912639
for (auto UnclaimedDRE : AllUnclaimed) {
2592-
Handler.addDebugNoteForVar(
2593-
it->first, UnclaimedDRE->getBeginLoc(),
2594-
("failed to produce fixit for '" + it->first->getNameAsString() +
2595-
"' : has an unclaimed use"));
2596-
}
2640+
std::string UnclaimedUseTrace =
2641+
getDREAncestorString(UnclaimedDRE, D->getASTContext());
2642+
2643+
Handler.addDebugNoteForVar(
2644+
it->first, UnclaimedDRE->getBeginLoc(),
2645+
("failed to produce fixit for '" + it->first->getNameAsString() +
2646+
"' : has an unclaimed use\nThe unclaimed DRE trace: " +
2647+
UnclaimedUseTrace));
2648+
}
25972649
#endif
25982650
it = FixablesForAllVars.byVar.erase(it);
25992651
} else if (it->first->isInitCapture()) {
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
# -*- Python -*-
2+
3+
config.substitutions.append(
4+
(
5+
"%analyze_safe_buffer_debug_notes",
6+
"'%s' %s" % (
7+
config.python_executable,
8+
os.path.join(config.clang_src_dir, "utils", "analyze_safe_buffer_debug_notes.py")
9+
)
10+
)
11+
)
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
// RUN: %clang_cc1 -Wno-unused-value -Wunsafe-buffer-usage -fsafe-buffer-usage-suggestions \
2+
// RUN: -mllvm -debug-only=SafeBuffers \
3+
// RUN: -std=c++20 -verify=expected %s
4+
5+
// RUN: %clang_cc1 -Wno-unused-value -Wunsafe-buffer-usage -fsafe-buffer-usage-suggestions \
6+
// RUN: -mllvm -debug-only=SafeBuffers \
7+
// RUN: -std=c++20 %s \
8+
// RUN: 2>&1 | grep 'The unclaimed DRE trace:' \
9+
// RUN: | sed 's/^The unclaimed DRE trace://' \
10+
// RUN: | %analyze_safe_buffer_debug_notes \
11+
// RUN: | FileCheck %s
12+
13+
// This debugging facility is only available in debug builds.
14+
//
15+
// REQUIRES: asserts
16+
// REQUIRES: shell
17+
18+
void test_unclaimed_use(int *p) { // expected-warning{{'p' is an unsafe pointer used for buffer access}}
19+
p++; // expected-note{{used in pointer arithmetic here}} \
20+
expected-note{{safe buffers debug: failed to produce fixit for 'p' : has an unclaimed use\n \
21+
The unclaimed DRE trace: DeclRefExpr, UnaryOperator(++), CompoundStmt}}
22+
*((p + 1) + 1); // expected-warning{{unsafe pointer arithmetic}} \
23+
expected-note{{used in pointer arithmetic here}} \
24+
expected-note{{safe buffers debug: failed to produce fixit for 'p' : has an unclaimed use\n \
25+
The unclaimed DRE trace: DeclRefExpr, ImplicitCastExpr(LValueToRValue), BinaryOperator(+), ParenExpr, BinaryOperator(+), ParenExpr, UnaryOperator(*), CompoundStmt}}
26+
p -= 1; // expected-note{{used in pointer arithmetic here}} \
27+
expected-note{{safe buffers debug: failed to produce fixit for 'p' : has an unclaimed use\n \
28+
The unclaimed DRE trace: DeclRefExpr, BinaryOperator(-=), CompoundStmt}}
29+
p--; // expected-note{{used in pointer arithmetic here}} \
30+
expected-note{{safe buffers debug: failed to produce fixit for 'p' : has an unclaimed use\n \
31+
The unclaimed DRE trace: DeclRefExpr, UnaryOperator(--), CompoundStmt}}
32+
p[5] = 5; // expected-note{{used in buffer access here}}
33+
}
34+
35+
// CHECK: Root # 1
36+
// CHECK: |- DeclRefExpr # 4
37+
// CHECK: |-- UnaryOperator(++) # 1
38+
// CHECK: |--- CompoundStmt # 1
39+
// CHECK: |-- ImplicitCastExpr(LValueToRValue) # 1
40+
// CHECK: |--- BinaryOperator(+) # 1
41+
// CHECK: |---- ParenExpr # 1
42+
// CHECK: |----- BinaryOperator(+) # 1
43+
// CHECK: |------ ParenExpr # 1
44+
// CHECK: |------- UnaryOperator(*) # 1
45+
// CHECK: |-------- CompoundStmt # 1
46+
// CHECK: |-- BinaryOperator(-=) # 1
47+
// CHECK: |--- CompoundStmt # 1
48+
// CHECK: |-- UnaryOperator(--) # 1
49+
// CHECK: |--- CompoundStmt # 1
Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
import sys
2+
from collections import OrderedDict
3+
4+
class Trie:
5+
def __init__(self, name):
6+
self.name = name
7+
self.children = OrderedDict()
8+
self.count = 1
9+
10+
def add(self, name):
11+
if name in self.children:
12+
self.children[name].count += 1
13+
else:
14+
self.children[name] = Trie(name)
15+
return self.children[name]
16+
17+
def print(self, depth):
18+
if depth > 0:
19+
print('|', end="")
20+
for i in range(depth):
21+
print('-', end="")
22+
if depth > 0:
23+
print(end=" ")
24+
print(self.name, '#', self.count)
25+
for key, child in self.children.items():
26+
child.print(depth + 1)
27+
28+
29+
Root = Trie("Root")
30+
31+
if __name__ == "__main__":
32+
for line in sys.stdin:
33+
words = line.split('==>')
34+
words = [word.strip() for word in words]
35+
MyTrie = Root;
36+
for word in words:
37+
MyTrie = MyTrie.add(word)
38+
39+
Root.print(0)

0 commit comments

Comments
 (0)