Skip to content

Commit f507642

Browse files
[-Wunsafe-buffer-usage] Add a warning gadget for single pointer argument
Add a warning gadget for passing a pointer to a __single pointer parameter in an unsafe way. This commit adds support for recognition of the following safe patterns: - `nullptr` - `&var`, if `var` is a variable identifier - `&C[_]`, if `C` is a hardened container/view - `span.first(1).data()` and friends If the argument is not one of those patterns, it is considered unsafe and a warning is emitted. rdar://128157528 (cherry picked from commit db48230)
1 parent 9aaab48 commit f507642

File tree

6 files changed

+270
-1
lines changed

6 files changed

+270
-1
lines changed

clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,13 @@ class UnsafeBufferUsageHandler {
134134
ASTContext &Ctx) {
135135
handleUnsafeOperation(Arg, IsRelatedToDecl, Ctx);
136136
}
137+
138+
/// Invoked when an unsafe passing to __single pointer is found.
139+
virtual void handleUnsafeSinglePointerArgument(const Expr *Arg,
140+
bool IsRelatedToDecl,
141+
ASTContext &Ctx) {
142+
handleUnsafeOperation(Arg, IsRelatedToDecl, Ctx);
143+
}
137144
/* TO_UPSTREAM(BoundsSafety) OFF */
138145

139146
/// Invoked when a fix is suggested against a variable. This function groups

clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,10 @@ WARNING_GADGET(PointerArithmetic)
3838
WARNING_GADGET(UnsafeBufferUsageAttr)
3939
WARNING_GADGET(UnsafeBufferUsageCtorAttr)
4040
WARNING_GADGET(DataInvocation)
41-
// TO_UPSTREAM(BoundsSafety)
41+
// TO_UPSTREAM(BoundsSafety) ON
4242
WARNING_GADGET(CountAttributedPointerArgument)
43+
WARNING_GADGET(SinglePointerArgument)
44+
// TO_UPSTREAM(BoundsSafety) OFF
4345
WARNING_OPTIONAL_GADGET(UnsafeLibcFunctionCall)
4446
WARNING_OPTIONAL_GADGET(SpanTwoParamConstructor) // Uses of `std::span(arg0, arg1)`
4547
FIXABLE_GADGET(ULCArraySubscript) // `DRE[any]` in an Unspecified Lvalue Context

clang/include/clang/Basic/DiagnosticSemaKinds.td

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13365,6 +13365,9 @@ def note_unsafe_count_attributed_pointer_argument : Note<
1336513365
"consider using %select{|a safe container and passing '.data()' to the "
1336613366
"parameter%select{| '%3'}2 and '.size()' to its dependent parameter '%4' or }0"
1336713367
"'std::span' and passing '.first(...).data()' to the parameter%select{| '%3'}2">;
13368+
def warn_unsafe_single_pointer_argument : Warning<
13369+
"unsafe assignment to function parameter of __single pointer type">,
13370+
InGroup<UnsafeBufferUsage>, DefaultIgnore;
1336813371
#ifndef NDEBUG
1336913372
// Not a user-facing diagnostic. Useful for debugging false negatives in
1337013373
// -fsafe-buffer-usage-suggestions (i.e. lack of -Wunsafe-buffer-usage fixits).

clang/lib/Analysis/UnsafeBufferUsage.cpp

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -231,6 +231,10 @@ AST_MATCHER(QualType, isCountAttributedType) {
231231
return Node->isCountAttributedType();
232232
}
233233

234+
AST_MATCHER(QualType, isSinglePointerType) {
235+
return Node->isSinglePointerType();
236+
}
237+
234238
AST_MATCHER_P(Stmt, forEachDescendantEvaluatedStmt, internal::Matcher<Stmt>,
235239
innerMatcher) {
236240
const DynTypedMatcher &DTM = static_cast<DynTypedMatcher>(innerMatcher);
@@ -836,6 +840,58 @@ bool isCountAttributedPointerArgumentSafe(ASTContext &Context,
836840
&*ValuesOpt, Context);
837841
}
838842

843+
// Checks if the argument passed to __single pointer is one of the following
844+
// forms:
845+
// 0. `nullptr`.
846+
// 1. `&var`, if `var` is a variable identifier.
847+
// 2. `&C[_]`, if `C` is a hardened container/view.
848+
// 3. `sp.first(1).data()` and friends.
849+
bool isSinglePointerArgumentSafe(ASTContext &Context, const Expr *Arg) {
850+
const Expr *ArgNoImp = Arg->IgnoreParenImpCasts();
851+
852+
// Check form 0:
853+
if (ArgNoImp->getType()->isNullPtrType())
854+
return true;
855+
856+
// Check form 1:
857+
{
858+
auto AddrOfDREMatcher = expr(
859+
unaryOperator(hasOperatorName("&"),
860+
hasUnaryOperand(ignoringParenImpCasts(declRefExpr()))));
861+
bool Matches = !match(AddrOfDREMatcher, *ArgNoImp, Context).empty();
862+
if (Matches)
863+
return true;
864+
}
865+
866+
// Check form 2:
867+
{
868+
// TODO: Add more classes.
869+
auto HardenedClassNameMatcher =
870+
anyOf(hasName("::std::array"), hasName("::std::basic_string"),
871+
hasName("::std::basic_string_view"), hasName("::std::span"),
872+
hasName("::std::vector"));
873+
auto SubscriptOpMatcher = cxxOperatorCallExpr(callee(cxxMethodDecl(
874+
hasName("operator[]"), ofClass(HardenedClassNameMatcher))));
875+
auto AddrOfMatcher = expr(unaryOperator(
876+
hasOperatorName("&"),
877+
hasUnaryOperand(ignoringParenImpCasts(SubscriptOpMatcher))));
878+
bool Matches = !match(AddrOfMatcher, *ArgNoImp, Context).empty();
879+
if (Matches)
880+
return true;
881+
}
882+
883+
// Check form 3:
884+
if (const Expr *ExtentExpr =
885+
extractExtentFromSubviewDataCall(Context, ArgNoImp)) {
886+
std::optional<llvm::APSInt> ExtentVal =
887+
ExtentExpr->getIntegerConstantExpr(Context);
888+
if (ExtentVal.has_value() && ExtentVal->isOne())
889+
return true;
890+
}
891+
892+
return false;
893+
}
894+
839895
} // namespace
840896

841897
// Given a two-param std::span construct call, matches iff the call has the
@@ -1019,6 +1075,12 @@ AST_MATCHER_P(CallExpr, forEachUnsafeCountAttributedPointerArgument,
10191075
return Matched;
10201076
}
10211077

1078+
// Matches iff the argument passed to __single pointer type is safe.
1079+
AST_MATCHER(Expr, isSinglePointerArgumentSafe) {
1080+
ASTContext &Context = Finder->getASTContext();
1081+
return isSinglePointerArgumentSafe(Context, &Node);
1082+
}
1083+
10221084
AST_MATCHER_P(CallExpr, hasNumArgs, unsigned, Num) {
10231085
return Node.getNumArgs() == Num;
10241086
}
@@ -2586,6 +2648,45 @@ class CountAttributedPointerArgumentGadget : public WarningGadget {
25862648
}
25872649
};
25882650

2651+
// Represents an argument that is being passed to a __single pointer.
2652+
class SinglePointerArgumentGadget : public WarningGadget {
2653+
private:
2654+
static constexpr const char *const ArgTag = "SinglePointerArgument_Arg";
2655+
const Expr *Arg;
2656+
2657+
public:
2658+
explicit SinglePointerArgumentGadget(const MatchFinder::MatchResult &Result)
2659+
: WarningGadget(Kind::SinglePointerArgument),
2660+
Arg(Result.Nodes.getNodeAs<Expr>(ArgTag)) {
2661+
assert(Arg != nullptr && "Expecting a non-null matching result");
2662+
}
2663+
2664+
static bool classof(const Gadget *G) {
2665+
return G->getKind() == Kind::SinglePointerArgument;
2666+
}
2667+
2668+
static Matcher matcher() {
2669+
return stmt(callExpr(forEachArgumentWithParamType(
2670+
expr(unless(isSinglePointerArgumentSafe())).bind(ArgTag),
2671+
isSinglePointerType())));
2672+
}
2673+
2674+
void handleUnsafeOperation(UnsafeBufferUsageHandler &Handler,
2675+
bool IsRelatedToDecl,
2676+
ASTContext &Ctx) const override {
2677+
Handler.handleUnsafeSinglePointerArgument(Arg, IsRelatedToDecl, Ctx);
2678+
}
2679+
2680+
SourceLocation getSourceLoc() const override { return Arg->getBeginLoc(); }
2681+
2682+
virtual DeclUseList getClaimedVarUseSites() const override {
2683+
if (const auto *DRE = dyn_cast<DeclRefExpr>(Arg)) {
2684+
return {DRE};
2685+
}
2686+
return {};
2687+
}
2688+
};
2689+
25892690
/// Scan the function and return a list of gadgets found with provided kits.
25902691
static void findGadgets(const Stmt *S, ASTContext &Ctx,
25912692
const UnsafeBufferUsageHandler &Handler,

clang/lib/Sema/AnalysisBasedWarnings.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2385,6 +2385,11 @@ class UnsafeBufferUsageReporter : public UnsafeBufferUsageHandler {
23852385
<< IsSimpleCount << QualType(CATy, 0) << !PtrParamName.empty()
23862386
<< PtrParamName << CountParamName;
23872387
}
2388+
2389+
void handleUnsafeSinglePointerArgument(const Expr *Arg, bool IsRelatedToDecl,
2390+
ASTContext &Ctx) override {
2391+
S.Diag(Arg->getBeginLoc(), diag::warn_unsafe_single_pointer_argument);
2392+
}
23882393
/* TO_UPSTREAM(BoundsSafety) OFF */
23892394

23902395
void handleUnsafeVariableGroup(const VarDecl *Variable,
Lines changed: 151 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,151 @@
1+
// RUN: %clang_cc1 -fsyntax-only -std=c++20 -Wno-all -Wunsafe-buffer-usage -fexperimental-bounds-safety-attributes -verify %s
2+
3+
#include <ptrcheck.h>
4+
#include <stddef.h>
5+
6+
namespace std {
7+
8+
template <typename T, size_t N>
9+
struct array {
10+
T &operator[](size_t n) noexcept;
11+
};
12+
13+
template <typename CharT>
14+
struct basic_string {
15+
CharT &operator[](size_t n) noexcept;
16+
};
17+
18+
typedef basic_string<char> string;
19+
20+
template <typename CharT>
21+
struct basic_string_view {
22+
const CharT &operator[](size_t n) const noexcept;
23+
};
24+
25+
typedef basic_string_view<char> string_view;
26+
27+
template <typename T>
28+
struct span {
29+
T *data() const noexcept;
30+
span<T> first(size_t count) const noexcept;
31+
span<T> last(size_t count) const noexcept;
32+
span<T> subspan(size_t offset, size_t count) const noexcept;
33+
T &operator[](size_t n) noexcept;
34+
};
35+
36+
template <typename T>
37+
struct vector {
38+
T &operator[](size_t n) noexcept;
39+
};
40+
41+
} // namespace std
42+
43+
template <typename T>
44+
struct my_vec {
45+
T &operator[](size_t n) noexcept;
46+
};
47+
48+
extern "C" {
49+
50+
void single_char(char *__single s);
51+
void single_cchar(const char *__single s);
52+
void single_int(int *__single p);
53+
void single_void(void *__single p);
54+
55+
void single_int_int(int *__single p, int *__single q);
56+
57+
} // extern "C"
58+
59+
// Check passing `nullptr`.
60+
61+
void null() {
62+
single_char(nullptr);
63+
single_cchar(nullptr);
64+
single_int(nullptr);
65+
single_void(nullptr);
66+
}
67+
68+
// Check `&var` pattern.
69+
70+
void addr_of_var() {
71+
char c = 0;
72+
single_char(&c);
73+
single_cchar(&c);
74+
single_void(&c);
75+
76+
int i = 0;
77+
single_int(&i);
78+
single_void(&i);
79+
}
80+
81+
// Check allowed classes in `&C[index]` pattern.
82+
83+
void allowed_class(std::array<int, 42> &a, std::string &s, std::string_view sv,
84+
std::span<int> sp, std::vector<int> &v) {
85+
single_int(&a[0]);
86+
single_void(&a[0]);
87+
88+
single_char(&s[0]);
89+
single_cchar(&s[0]);
90+
single_void(&s[0]);
91+
92+
single_cchar(&sv[0]);
93+
94+
single_int(&sp[0]);
95+
single_void(&sp[0]);
96+
97+
single_int(&v[0]);
98+
single_void(&v[0]);
99+
}
100+
101+
void not_allowed_class(my_vec<int> &mv) {
102+
single_int(&mv[0]); // expected-warning{{unsafe assignment to function parameter of __single pointer type}}
103+
}
104+
105+
// Check if index doesn't matter in `&C[index]` pattern.
106+
107+
void index_does_not_matter(std::span<int> sp, size_t index) {
108+
single_int(&sp[0]);
109+
single_int(&sp[1]);
110+
single_int(&sp[index]);
111+
single_int(&sp[42 - index]);
112+
}
113+
114+
// Check span's subview pattern.
115+
116+
void span_subview(std::span<int> sp, int n) {
117+
single_int(sp.first(1).data());
118+
single_int(sp.first(0).data()); // expected-warning{{unsafe assignment to function parameter of __single pointer type}}
119+
single_int(sp.first(n).data()); // expected-warning{{unsafe assignment to function parameter of __single pointer type}}
120+
121+
single_int(sp.last(1).data());
122+
single_int(sp.last(0).data()); // expected-warning{{unsafe assignment to function parameter of __single pointer type}}
123+
single_int(sp.last(n).data()); // expected-warning{{unsafe assignment to function parameter of __single pointer type}}
124+
125+
single_int(sp.subspan(0, 1).data());
126+
single_int(sp.subspan(42, 1).data());
127+
single_int(sp.subspan(n, 1).data());
128+
single_int(sp.subspan(0, 0).data()); // expected-warning{{unsafe assignment to function parameter of __single pointer type}}
129+
single_int(sp.subspan(0, n).data()); // expected-warning{{unsafe assignment to function parameter of __single pointer type}}
130+
}
131+
132+
// Check multiple args.
133+
134+
void multiple_args(int i, int *p) {
135+
single_int_int(&i, &i);
136+
single_int_int(&i, p); // expected-warning{{unsafe assignment to function parameter of __single pointer type}}
137+
single_int_int(p, &i); // expected-warning{{unsafe assignment to function parameter of __single pointer type}}
138+
139+
single_int_int(
140+
p, // expected-warning{{unsafe assignment to function parameter of __single pointer type}}
141+
p // expected-warning{{unsafe assignment to function parameter of __single pointer type}}
142+
);
143+
}
144+
145+
// Check common unsafe patterns.
146+
147+
void unsafe(std::span<int> sp, int *p) {
148+
single_int(sp.data()); // expected-warning{{unsafe assignment to function parameter of __single pointer type}}
149+
150+
single_int(p); // expected-warning{{unsafe assignment to function parameter of __single pointer type}}
151+
}

0 commit comments

Comments
 (0)