Skip to content

Commit 7dbc61d

Browse files
malavikasamakMalavikaSamak
andcommitted
[-Wunsafe-buffer-usage] Fix the crash introduced by the unsafe invocation of span::data warning (llvm#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]> (cherry picked from commit 414df70)
1 parent 4b42851 commit 7dbc61d

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
@@ -737,9 +737,10 @@ class DataInvocationGadget : public WarningGadget {
737737
}
738738

739739
static Matcher matcher() {
740+
Matcher callExpr = cxxMemberCallExpr(
741+
callee(cxxMethodDecl(hasName("data"), ofClass(hasName("std::span")))));
740742
return stmt(
741-
explicitCastExpr(has(cxxMemberCallExpr(callee(cxxMethodDecl(
742-
hasName("data"), ofClass(hasName("std::span")))))))
743+
explicitCastExpr(anyOf(has(callExpr), has(parenExpr(has(callExpr)))))
743744
.bind(OpTag));
744745
}
745746
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
@@ -2239,15 +2239,20 @@ class UnsafeBufferUsageReporter : public UnsafeBufferUsageHandler {
22392239
MsgParam = 3;
22402240
} else if (const auto *ECE = dyn_cast<ExplicitCastExpr>(Operation)) {
22412241
QualType destType = ECE->getType();
2242+
if (!isa<PointerType>(destType))
2243+
return;
2244+
2245+
const Expr *subExpr = ECE->getSubExpr();
2246+
22422247
const uint64_t dSize =
22432248
Ctx.getTypeSize(destType.getTypePtr()->getPointeeType());
2244-
if (const auto *CE = dyn_cast<CXXMemberCallExpr>(ECE->getSubExpr())) {
2245-
QualType srcType = CE->getType();
2246-
const uint64_t sSize =
2247-
Ctx.getTypeSize(srcType.getTypePtr()->getPointeeType());
2248-
if (sSize >= dSize)
2249-
return;
2250-
}
2249+
2250+
QualType srcType = ECE->getSubExpr()->getType();
2251+
const uint64_t sSize =
2252+
Ctx.getTypeSize(srcType.getTypePtr()->getPointeeType());
2253+
if (sSize >= dSize)
2254+
return;
2255+
22512256
MsgParam = 4;
22522257
}
22532258
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)