https://github.com/malavikasamak updated https://github.com/llvm/llvm-project/pull/78815
>From 6334cd361f79fc79f32b8ca95c6f31a083704332 Mon Sep 17 00:00:00 2001 From: MalavikaSamak <malavi...@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 <malavi...@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) { } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits