[clang] [-Wunsafe-buffer-usage] Fix the crash introduced by the unsafe invocation of span::data warning (PR #78815)
Malavika Samak via cfe-commits
cfe-commits at lists.llvm.org
Mon Jan 22 10:25:20 PST 2024
https://github.com/malavikasamak updated https://github.com/llvm/llvm-project/pull/78815
>From 6334cd361f79fc79f32b8ca95c6f31a083704332 Mon Sep 17 00:00:00 2001
From: MalavikaSamak <malavika2 at apple.com>
Date: Fri, 19 Jan 2024 15:16:12 -0800
Subject: [PATCH 1/2] [-Wunsafe-buffer-usage] Fix the crash introduced by the
unsafe invocation of span::data warning
Radar: 121223051
---
clang/lib/Analysis/UnsafeBufferUsage.cpp | 5 ++--
clang/lib/Sema/AnalysisBasedWarnings.cpp | 26 ++++++++++++++-----
...e-buffer-usage-warning-data-invocation.cpp | 24 +++++++++++------
3 files changed, 38 insertions(+), 17 deletions(-)
diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index 724c4304a072420..7df706beb22662c 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -739,9 +739,10 @@ class DataInvocationGadget : public WarningGadget {
}
static Matcher matcher() {
+ Matcher callExpr = cxxMemberCallExpr(
+ callee(cxxMethodDecl(hasName("data"), ofClass(hasName("std::span")))));
return stmt(
- explicitCastExpr(has(cxxMemberCallExpr(callee(cxxMethodDecl(
- hasName("data"), ofClass(hasName("std::span")))))))
+ explicitCastExpr(anyOf(has(callExpr), has(parenExpr(has(callExpr)))))
.bind(OpTag));
}
const Stmt *getBaseStmt() const override { return Op; }
diff --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp b/clang/lib/Sema/AnalysisBasedWarnings.cpp
index 9eb1df5f0240596..749655d03342cca 100644
--- a/clang/lib/Sema/AnalysisBasedWarnings.cpp
+++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp
@@ -2263,15 +2263,27 @@ class UnsafeBufferUsageReporter : public UnsafeBufferUsageHandler {
MsgParam = 3;
} else if (const auto *ECE = dyn_cast<ExplicitCastExpr>(Operation)) {
QualType destType = ECE->getType();
- const uint64_t dSize =
- Ctx.getTypeSize(destType.getTypePtr()->getPointeeType());
- if (const auto *CE = dyn_cast<CXXMemberCallExpr>(ECE->getSubExpr())) {
- QualType srcType = CE->getType();
- const uint64_t sSize =
- Ctx.getTypeSize(srcType.getTypePtr()->getPointeeType());
- if (sSize >= dSize)
+ if (!isa<PointerType>(destType))
+ return;
+
+ const Expr *subExpr = ECE->getSubExpr();
+ // Check if related to DataInvocation warning gadget.
+ if (!isa<CXXMemberCallExpr>(subExpr)) {
+ if (const auto *SE = dyn_cast<ParenExpr>(subExpr)) {
+ if (!isa<CXXMemberCallExpr>(SE->getSubExpr()))
+ return;
+ } else
return;
}
+ const uint64_t dSize =
+ Ctx.getTypeSize(destType.getTypePtr()->getPointeeType());
+
+ QualType srcType = ECE->getSubExpr()->getType();
+ const uint64_t sSize =
+ Ctx.getTypeSize(srcType.getTypePtr()->getPointeeType());
+ if (sSize >= dSize)
+ return;
+
MsgParam = 4;
}
Loc = Operation->getBeginLoc();
diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-warning-data-invocation.cpp b/clang/test/SemaCXX/warn-unsafe-buffer-usage-warning-data-invocation.cpp
index 79eb3bb4bacc6e7..7b39bef04114236 100644
--- a/clang/test/SemaCXX/warn-unsafe-buffer-usage-warning-data-invocation.cpp
+++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-warning-data-invocation.cpp
@@ -22,6 +22,8 @@ namespace std {
using size_t = __typeof(sizeof(int));
void *malloc(size_t);
+typedef long int intptr_t;
+
void foo(int v) {
}
@@ -90,15 +92,18 @@ void cast_without_data(int *ptr) {
void warned_patterns(std::span<int> span_ptr, std::span<Base> base_span, span<int> span_without_qual) {
A *a1 = (A*)span_ptr.data(); // expected-warning{{unsafe invocation of span::data}}
a1 = (A*)span_ptr.data(); // expected-warning{{unsafe invocation of span::data}}
-
- A *a2 = (A*) span_without_qual.data(); // expected-warning{{unsafe invocation of span::data}}
-
- // TODO:: Should we warn when we cast from base to derived type?
- Derived *b = dynamic_cast<Derived*> (base_span.data());// expected-warning{{unsafe invocation of span::data}}
- // TODO:: This pattern is safe. We can add special handling for it, if we decide this
- // is the recommended fixit for the unsafe invocations.
- A *a3 = (A*)span_ptr.subspan(0, sizeof(A)).data(); // expected-warning{{unsafe invocation of span::data}}
+ a1 = (A*)(span_ptr.data()); // expected-warning{{unsafe invocation of span::data}}
+ A *a2 = (A*) (span_without_qual.data()); // expected-warning{{unsafe invocation of span::data}}
+
+ a2 = (A*) span_without_qual.data(); // expected-warning{{unsafe invocation of span::data}}
+
+ // TODO:: Should we warn when we cast from base to derived type?
+ Derived *b = dynamic_cast<Derived*> (base_span.data());// expected-warning{{unsafe invocation of span::data}}
+
+ // TODO:: This pattern is safe. We can add special handling for it, if we decide this
+ // is the recommended fixit for the unsafe invocations.
+ A *a3 = (A*)span_ptr.subspan(0, sizeof(A)).data(); // expected-warning{{unsafe invocation of span::data}}
}
void not_warned_patterns(std::span<A> span_ptr, std::span<Base> base_span) {
@@ -108,6 +113,9 @@ void not_warned_patterns(std::span<A> span_ptr, std::span<Base> base_span) {
p = (int*) span_ptr.data();
A *a = (A*) span_ptr.hello(); // Invoking other methods.
+
+ intptr_t k = (intptr_t) span_ptr.data();
+ k = (intptr_t) (span_ptr.data());
}
// We do not want to warn about other types
>From b6a499430d74d7471d5697f08be62d2cd55f4611 Mon Sep 17 00:00:00 2001
From: MalavikaSamak <malavika2 at apple.com>
Date: Mon, 22 Jan 2024 10:04:53 -0800
Subject: [PATCH 2/2] Fixing a build failure and addressing some comments.
---
clang/lib/Sema/AnalysisBasedWarnings.cpp | 9 +--------
.../warn-unsafe-buffer-usage-warning-data-invocation.cpp | 3 +--
2 files changed, 2 insertions(+), 10 deletions(-)
diff --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp b/clang/lib/Sema/AnalysisBasedWarnings.cpp
index 749655d03342cca..649c3f533b8206e 100644
--- a/clang/lib/Sema/AnalysisBasedWarnings.cpp
+++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp
@@ -2267,14 +2267,7 @@ class UnsafeBufferUsageReporter : public UnsafeBufferUsageHandler {
return;
const Expr *subExpr = ECE->getSubExpr();
- // Check if related to DataInvocation warning gadget.
- if (!isa<CXXMemberCallExpr>(subExpr)) {
- if (const auto *SE = dyn_cast<ParenExpr>(subExpr)) {
- if (!isa<CXXMemberCallExpr>(SE->getSubExpr()))
- return;
- } else
- return;
- }
+
const uint64_t dSize =
Ctx.getTypeSize(destType.getTypePtr()->getPointeeType());
diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-warning-data-invocation.cpp b/clang/test/SemaCXX/warn-unsafe-buffer-usage-warning-data-invocation.cpp
index 7b39bef04114236..574afcd0eb6dce3 100644
--- a/clang/test/SemaCXX/warn-unsafe-buffer-usage-warning-data-invocation.cpp
+++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-warning-data-invocation.cpp
@@ -7,6 +7,7 @@
// RUN: %clang_cc1 -std=c++20 -fblocks -include %s %s 2>&1 | FileCheck --allow-empty %s
// CHECK-NOT: [-Wunsafe-buffer-usage]
+#include <stdint.h>
#ifndef INCLUDED
#define INCLUDED
#pragma clang system_header
@@ -22,8 +23,6 @@ namespace std {
using size_t = __typeof(sizeof(int));
void *malloc(size_t);
-typedef long int intptr_t;
-
void foo(int v) {
}
More information about the cfe-commits
mailing list