Skip to content

Commit 414df70

Browse files
malavikasamakMalavikaSamak
andauthored
[-Wunsafe-buffer-usage] Fix the crash introduced by the unsafe invocation of span::data warning (#78815)
The patch fixes the crash introduced by the DataInvocation warning gadget designed to warn against unsafe invocations of span::data method. It also now considers the invocation of span::data method inside parenthesis. Radar: 121223051 --------- Co-authored-by: MalavikaSamak <[email protected]>
1 parent 0cea54a commit 414df70

File tree

3 files changed

+30
-17
lines changed

3 files changed

+30
-17
lines changed

clang/lib/Analysis/UnsafeBufferUsage.cpp

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -739,9 +739,10 @@ class DataInvocationGadget : public WarningGadget {
739739
}
740740

741741
static Matcher matcher() {
742+
Matcher callExpr = cxxMemberCallExpr(
743+
callee(cxxMethodDecl(hasName("data"), ofClass(hasName("std::span")))));
742744
return stmt(
743-
explicitCastExpr(has(cxxMemberCallExpr(callee(cxxMethodDecl(
744-
hasName("data"), ofClass(hasName("std::span")))))))
745+
explicitCastExpr(anyOf(has(callExpr), has(parenExpr(has(callExpr)))))
745746
.bind(OpTag));
746747
}
747748
const Stmt *getBaseStmt() const override { return Op; }

clang/lib/Sema/AnalysisBasedWarnings.cpp

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2263,15 +2263,20 @@ class UnsafeBufferUsageReporter : public UnsafeBufferUsageHandler {
22632263
MsgParam = 3;
22642264
} else if (const auto *ECE = dyn_cast<ExplicitCastExpr>(Operation)) {
22652265
QualType destType = ECE->getType();
2266+
if (!isa<PointerType>(destType))
2267+
return;
2268+
2269+
const Expr *subExpr = ECE->getSubExpr();
2270+
22662271
const uint64_t dSize =
22672272
Ctx.getTypeSize(destType.getTypePtr()->getPointeeType());
2268-
if (const auto *CE = dyn_cast<CXXMemberCallExpr>(ECE->getSubExpr())) {
2269-
QualType srcType = CE->getType();
2270-
const uint64_t sSize =
2271-
Ctx.getTypeSize(srcType.getTypePtr()->getPointeeType());
2272-
if (sSize >= dSize)
2273-
return;
2274-
}
2273+
2274+
QualType srcType = ECE->getSubExpr()->getType();
2275+
const uint64_t sSize =
2276+
Ctx.getTypeSize(srcType.getTypePtr()->getPointeeType());
2277+
if (sSize >= dSize)
2278+
return;
2279+
22752280
MsgParam = 4;
22762281
}
22772282
Loc = Operation->getBeginLoc();

clang/test/SemaCXX/warn-unsafe-buffer-usage-warning-data-invocation.cpp

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
// RUN: %clang_cc1 -std=c++20 -fblocks -include %s %s 2>&1 | FileCheck --allow-empty %s
88
// CHECK-NOT: [-Wunsafe-buffer-usage]
99

10+
#include <stdint.h>
1011
#ifndef INCLUDED
1112
#define INCLUDED
1213
#pragma clang system_header
@@ -90,15 +91,18 @@ void cast_without_data(int *ptr) {
9091
void warned_patterns(std::span<int> span_ptr, std::span<Base> base_span, span<int> span_without_qual) {
9192
A *a1 = (A*)span_ptr.data(); // expected-warning{{unsafe invocation of span::data}}
9293
a1 = (A*)span_ptr.data(); // expected-warning{{unsafe invocation of span::data}}
93-
94-
A *a2 = (A*) span_without_qual.data(); // expected-warning{{unsafe invocation of span::data}}
95-
96-
// TODO:: Should we warn when we cast from base to derived type?
97-
Derived *b = dynamic_cast<Derived*> (base_span.data());// expected-warning{{unsafe invocation of span::data}}
9894

99-
// TODO:: This pattern is safe. We can add special handling for it, if we decide this
100-
// is the recommended fixit for the unsafe invocations.
101-
A *a3 = (A*)span_ptr.subspan(0, sizeof(A)).data(); // expected-warning{{unsafe invocation of span::data}}
95+
a1 = (A*)(span_ptr.data()); // expected-warning{{unsafe invocation of span::data}}
96+
A *a2 = (A*) (span_without_qual.data()); // expected-warning{{unsafe invocation of span::data}}
97+
98+
a2 = (A*) span_without_qual.data(); // expected-warning{{unsafe invocation of span::data}}
99+
100+
// TODO:: Should we warn when we cast from base to derived type?
101+
Derived *b = dynamic_cast<Derived*> (base_span.data());// expected-warning{{unsafe invocation of span::data}}
102+
103+
// TODO:: This pattern is safe. We can add special handling for it, if we decide this
104+
// is the recommended fixit for the unsafe invocations.
105+
A *a3 = (A*)span_ptr.subspan(0, sizeof(A)).data(); // expected-warning{{unsafe invocation of span::data}}
102106
}
103107

104108
void not_warned_patterns(std::span<A> span_ptr, std::span<Base> base_span) {
@@ -108,6 +112,9 @@ void not_warned_patterns(std::span<A> span_ptr, std::span<Base> base_span) {
108112

109113
p = (int*) span_ptr.data();
110114
A *a = (A*) span_ptr.hello(); // Invoking other methods.
115+
116+
intptr_t k = (intptr_t) span_ptr.data();
117+
k = (intptr_t) (span_ptr.data());
111118
}
112119

113120
// We do not want to warn about other types

0 commit comments

Comments
 (0)