https://github.com/ziqingluo-90 created
https://github.com/llvm/llvm-project/pull/113226
The analysis now searches for every descendant stmt of constructor initializers
and recurses if the descendant is another constructor call.
(rdar://137999201)
>From e27fccf11bb750e32453be923f6925abd4cfda31 Mon Sep 17 00:00:00 2001
From: Ziqing Luo <ziqing_...@apple.com>
Date: Mon, 21 Oct 2024 14:12:12 -0700
Subject: [PATCH] [-Wunsafe-buffer-usage] Fix false negatives of missing
2-param span ctor in constructor initializers
The analysis now searches for every descendant stmt of constructor
initializers and recurses if the descendant is another constructor
call.
(rdar://137999201)
---
.../Analysis/Analyses/UnsafeBufferUsage.h | 7 ++-
.../clang/Basic/DiagnosticSemaKinds.td | 2 +
clang/lib/Analysis/UnsafeBufferUsage.cpp | 61 ++++++++++++++-----
clang/lib/Sema/AnalysisBasedWarnings.cpp | 16 ++---
...ffer-usage-in-container-span-construct.cpp | 58 ++++++++++++++++++
5 files changed, 122 insertions(+), 22 deletions(-)
diff --git a/clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h
b/clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h
index 267cde64f8f23c..07f1a85efbea50 100644
--- a/clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h
+++ b/clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h
@@ -122,7 +122,12 @@ class UnsafeBufferUsageHandler {
const Expr *UnsafeArg = nullptr) = 0;
/// Invoked when an unsafe operation with a std container is found.
- virtual void handleUnsafeOperationInContainer(const Stmt *Operation,
+ /// \param Invocation the `Stmt` that either is a direct call to the unsafe
+ /// span constructor or involves a direct call to it
+ /// \param UnsafeSpanCall the `Stmt` that refers to the direct call to the
+ /// unsafe span constructor
+ virtual void handleUnsafeOperationInContainer(const Stmt *Invocation,
+ const Stmt *UnsafeSpanCall,
bool IsRelatedToDecl,
ASTContext &Ctx) = 0;
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 883db838ca0147..238446a40ce4bc 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -12521,6 +12521,8 @@ def note_safe_buffer_usage_suggestions_disabled : Note<
def warn_unsafe_buffer_usage_in_container : Warning<
"the two-parameter std::span construction is unsafe as it can introduce
mismatch between buffer size and the bound information">,
InGroup<UnsafeBufferUsageInContainer>, DefaultIgnore;
+def note_unsafe_buffer_usage_in_container : Note<
+ "the 'std::span' constructor call here">;
#ifndef NDEBUG
// Not a user-facing diagnostic. Useful for debugging false negatives in
// -fsafe-buffer-usage-suggestions (i.e. lack of -Wunsafe-buffer-usage fixits).
diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp
b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index 5e0ec9ecc92ea4..28469ab96812a3 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -455,6 +455,28 @@ AST_MATCHER_P(CallExpr, hasNumArgs, unsigned, Num) {
return Node.getNumArgs() == Num;
}
+// Matches a CXXConstructor call if it matches the `InnerMatcher` in a
recursive
+// manner. I.e., either
+// 1) `Node` matches `InnerMatcher` directly, or
+// 2) a descendant of one of `Node`'s initializers matches
+// `ctorOrForEachInitializerDescendant` with the `InnerMatcher`.
+AST_MATCHER_P(CXXConstructExpr, ctorOrForEachInitializerDescendant,
+ internal::Matcher<CXXConstructExpr>, InnerMatcher) {
+ if (InnerMatcher.matches(Node, Finder, Builder))
+ return true;
+
+ const CXXConstructorDecl *CD = Node.getConstructor();
+
+ // recursive on both base and member initializers:
+ for (const CXXCtorInitializer *Init : CD->inits()) {
+ if (forEachDescendantStmt(
+ cxxConstructExpr(ctorOrForEachInitializerDescendant(InnerMatcher)))
+ .matches(*Init->getInit(), Finder, Builder))
+ return true;
+ }
+ return false;
+}
+
namespace libc_func_matchers {
// Under `libc_func_matchers`, define a set of matchers that match unsafe
// functions in libc and unsafe calls to them.
@@ -1218,38 +1240,49 @@ class PointerArithmeticGadget : public WarningGadget {
};
class SpanTwoParamConstructorGadget : public WarningGadget {
- static constexpr const char *const SpanTwoParamConstructorTag =
- "spanTwoParamConstructor";
- const CXXConstructExpr *Ctor; // the span constructor expression
+ static constexpr const char *const InvocationExprTag =
+ "spanTwoParamConstructor_Invocation";
+ static constexpr const char *const SpanTwoParamCtorTag =
+ "spanTwoParamConstructor_Ctor";
+ // The constructor call that either 1) is a direct call to the unsafe span
+ // constructor, or 2) involves a call to the unsafe span constructor
+ // (recursively through constructor initializers):
+ const CXXConstructExpr *Invocation;
+ // In case 1) above, `Ctor == Invocation`; otherwise `Ctor` refers to the
+ // actual span constructor call involved in the `Invocation`.
+ const CXXConstructExpr *Ctor;
public:
SpanTwoParamConstructorGadget(const MatchFinder::MatchResult &Result)
: WarningGadget(Kind::SpanTwoParamConstructor),
- Ctor(Result.Nodes.getNodeAs<CXXConstructExpr>(
- SpanTwoParamConstructorTag)) {}
+
Invocation(Result.Nodes.getNodeAs<CXXConstructExpr>(InvocationExprTag)),
+ Ctor(Result.Nodes.getNodeAs<CXXConstructExpr>(SpanTwoParamCtorTag)) {}
static bool classof(const Gadget *G) {
return G->getKind() == Kind::SpanTwoParamConstructor;
}
static Matcher matcher() {
- auto HasTwoParamSpanCtorDecl = hasDeclaration(
- cxxConstructorDecl(hasDeclContext(isInStdNamespace()), hasName("span"),
- parameterCountIs(2)));
-
- return stmt(cxxConstructExpr(HasTwoParamSpanCtorDecl,
- unless(isSafeSpanTwoParamConstruct()))
- .bind(SpanTwoParamConstructorTag));
+ auto HasTwoParamSpanCtorDecl =
+ cxxConstructExpr(hasDeclaration(cxxConstructorDecl(
+ hasDeclContext(isInStdNamespace()),
+ hasName("span"), parameterCountIs(2))),
+ unless(isSafeSpanTwoParamConstruct()))
+ .bind(SpanTwoParamCtorTag);
+ return stmt(cxxConstructExpr(
+
ctorOrForEachInitializerDescendant(HasTwoParamSpanCtorDecl))
+ .bind(InvocationExprTag));
}
static Matcher matcher(const UnsafeBufferUsageHandler *Handler) {
- return stmt(unless(ignoreUnsafeBufferInContainer(Handler)), matcher());
+ return stmt(unless(ignoreUnsafeBufferInContainer(Handler)),
+ SpanTwoParamConstructorGadget::matcher());
}
void handleUnsafeOperation(UnsafeBufferUsageHandler &Handler,
bool IsRelatedToDecl,
ASTContext &Ctx) const override {
- Handler.handleUnsafeOperationInContainer(Ctor, IsRelatedToDecl, Ctx);
+ Handler.handleUnsafeOperationInContainer(Invocation, Ctor,
IsRelatedToDecl, Ctx);
}
SourceLocation getSourceLoc() const override { return Ctor->getBeginLoc(); }
diff --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp
b/clang/lib/Sema/AnalysisBasedWarnings.cpp
index c76733e9a774b6..06d078d5f0550c 100644
--- a/clang/lib/Sema/AnalysisBasedWarnings.cpp
+++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp
@@ -2327,19 +2327,21 @@ class UnsafeBufferUsageReporter : public
UnsafeBufferUsageHandler {
}
}
- void handleUnsafeOperationInContainer(const Stmt *Operation,
+ void handleUnsafeOperationInContainer(const Stmt *Invocation,
+ const Stmt *UnsafeSpanCall,
bool IsRelatedToDecl,
ASTContext &Ctx) override {
SourceLocation Loc;
SourceRange Range;
unsigned MsgParam = 0;
- // This function only handles SpanTwoParamConstructorGadget so far, which
- // always gives a CXXConstructExpr.
- const auto *CtorExpr = cast<CXXConstructExpr>(Operation);
- Loc = CtorExpr->getLocation();
-
- S.Diag(Loc, diag::warn_unsafe_buffer_usage_in_container);
+ Loc = Invocation->getBeginLoc();
+ S.Diag(Loc, diag::warn_unsafe_buffer_usage_in_container)
+ << Invocation->getSourceRange();
+ if (Invocation != UnsafeSpanCall)
+ S.Diag(UnsafeSpanCall->getBeginLoc(),
+ diag::note_unsafe_buffer_usage_in_container)
+ << UnsafeSpanCall->getSourceRange();
if (IsRelatedToDecl) {
assert(!SuggestSuggestions &&
"Variables blamed for unsafe buffer usage without suggestions!");
diff --git
a/clang/test/SemaCXX/warn-unsafe-buffer-usage-in-container-span-construct.cpp
b/clang/test/SemaCXX/warn-unsafe-buffer-usage-in-container-span-construct.cpp
index e97511593bbd81..7258d4c4ef640e 100644
---
a/clang/test/SemaCXX/warn-unsafe-buffer-usage-in-container-span-construct.cpp
+++
b/clang/test/SemaCXX/warn-unsafe-buffer-usage-in-container-span-construct.cpp
@@ -157,3 +157,61 @@ namespace test_flag {
}
} //namespace test_flag
+
+
+namespace ctor_initializer_list {
+ class Base {
+ std::span<int> S;
+
+ public:
+ Base(int *p, unsigned n) : S(p, n) {} // expected-note 4{{the 'std::span'
constructor call here}}
+ };
+
+ class Derived : public Base {
+ public:
+ Derived(int *p, unsigned n) : Base(p, n) {}
+ };
+
+ class Friend {
+ Derived D;
+ public:
+ Friend(int *p, unsigned n) : D(p, n) {}
+ };
+
+ class FriendToo {
+ Friend F;
+ public:
+ FriendToo(int *p) : F(p, 10) {}
+ };
+
+ class SpanData {
+ int * P;
+ public:
+ SpanData(int *p) : P(std::span<int>(p, 10).data()) {} // expected-note
2{{the 'std::span' constructor call here}}
+ };
+
+ class SpanDataDerived : public SpanData {
+ public:
+ SpanDataDerived(int *p) : SpanData(p) {}
+ };
+
+ void foo(int *p) {
+ Base B(p, 10); // expected-warning{{the two-parameter std::span
construction is unsafe as it can introduce mismatch between buffer size and the
bound information}}
+ Derived D(p, 10); // expected-warning{{the two-parameter std::span
construction is unsafe as it can introduce mismatch between buffer size and the
bound information}}
+ Friend F(p, 10); // expected-warning{{the two-parameter std::span
construction is unsafe as it can introduce mismatch between buffer size and the
bound information}}
+ FriendToo FT(p); // expected-warning{{the two-parameter std::span
construction is unsafe as it can introduce mismatch between buffer size and the
bound information}}
+ SpanData SD(p); // expected-warning{{the two-parameter std::span
construction is unsafe as it can introduce mismatch between buffer size and the
bound information}}
+ SpanDataDerived SDD(p); // expected-warning{{the two-parameter std::span
construction is unsafe as it can introduce mismatch between buffer size and the
bound information}}
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wunsafe-buffer-usage-in-container"
+ {
+ Base B(p, 10);
+ Derived D(p, 10);
+ Friend F(p, 10);
+ FriendToo FT(p);
+ SpanData SD(p);
+ SpanDataDerived SDD(p);
+ }
+#pragma clang diagnostic pop
+ }
+} // ctor_initializer_list
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
