Skip to content

Conversation

@ziqingluo-90
Copy link
Contributor

Fixed the crash coming from attempting to get size of incomplete types. Casting span.data() to a pointer-to-incomplete-type should be immediately considered unsafe.

Solving issue #116286.

@llvmbot
Copy link
Member

llvmbot commented Nov 16, 2024

@llvm/pr-subscribers-clang

Author: Ziqing Luo (ziqingluo-90)

Changes

Fixed the crash coming from attempting to get size of incomplete types. Casting span.data() to a pointer-to-incomplete-type should be immediately considered unsafe.

Solving issue #116286.


Full diff: https://github.com/llvm/llvm-project/pull/116433.diff

2 Files Affected:

  • (modified) clang/lib/Sema/AnalysisBasedWarnings.cpp (+16-7)
  • (modified) clang/test/SemaCXX/warn-unsafe-buffer-usage-warning-data-invocation.cpp (+17)
diff --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp b/clang/lib/Sema/AnalysisBasedWarnings.cpp
index c76733e9a774b6..129040115acb5c 100644
--- a/clang/lib/Sema/AnalysisBasedWarnings.cpp
+++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp
@@ -2270,19 +2270,28 @@ class UnsafeBufferUsageReporter : public UnsafeBufferUsageHandler {
         MsgParam = 5;
       } else if (const auto *ECE = dyn_cast<ExplicitCastExpr>(Operation)) {
         QualType destType = ECE->getType();
+        bool destTypeComplete = true;
+
         if (!isa<PointerType>(destType))
           return;
+        destType = destType.getTypePtr()->getPointeeType();
+        if (const auto *D = destType->getAsTagDecl())
+          destTypeComplete = D->isCompleteDefinition();
 
-        const uint64_t dSize =
-            Ctx.getTypeSize(destType.getTypePtr()->getPointeeType());
+        // If destination type is incomplete, it is unsafe to cast to anyway, no
+        // need to check its' type:
+        if (destTypeComplete) {
+          const uint64_t dSize = Ctx.getTypeSize(destType);
+          QualType srcType = ECE->getSubExpr()->getType();
 
-        QualType srcType = ECE->getSubExpr()->getType();
-        const uint64_t sSize =
-            Ctx.getTypeSize(srcType.getTypePtr()->getPointeeType());
+          assert(srcType->isPointerType());
 
-        if (sSize >= dSize)
-          return;
+          const uint64_t sSize =
+              Ctx.getTypeSize(srcType.getTypePtr()->getPointeeType());
 
+          if (sSize >= dSize)
+            return;
+        }
         if (const auto *CE = dyn_cast<CXXMemberCallExpr>(
                 ECE->getSubExpr()->IgnoreParens())) {
           D = CE->getMethodDecl();
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 0228e42652bd95..a6a334d247023c 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
@@ -173,4 +173,21 @@ A false_negatives(std::span<int> span_pt, span<A> span_A) {
   return *a2; // TODO: Can cause OOB if span_pt is empty
 
 }
+
+void test_incomplete_type(std::span<char> S) {
+  (struct IncompleteStruct *)S.data(); // expected-warning{{unsafe invocation of 'data'}}
+  (class IncompleteClass *)S.data();   // expected-warning{{unsafe invocation of 'data'}}
+  (union IncompleteUnion *)S.data();   // expected-warning{{unsafe invocation of 'data'}}
+}
+
+void test_complete_type(std::span<long> S) {
+  (struct CompleteStruct *)S.data(); // no warn as the struct size is smaller than long
+  (class CompleteClass *)S.data();   // no warn as the class size is smaller than long
+  (union CompleteUnion *)S.data();   // no warn as the union size is smaller than long
+
+  struct CompleteStruct {};
+  class CompleteClass {};
+  union CompleteUnion {};
+}
+
 #endif

Fixed the crash coming from attempting to get size of incomplete
types.  Casting `span.data()` to pointer-to-incomplete-type should be
directly considered unsafe.

Solving issue llvm#116286.
@ziqingluo-90 ziqingluo-90 force-pushed the dev/ziqing/bugfix-116286 branch from 5aacab9 to 429dd67 Compare November 18, 2024 18:39
@ziqingluo-90 ziqingluo-90 merged commit 78606af into llvm:main Nov 18, 2024
8 checks passed
ZequanWu added a commit that referenced this pull request Nov 27, 2024
…ld and constructor initializers (#91991)"

It was originally reverted due to an [existing bug](#116286). Relanding it as the bug was fixed by #116433.
@ziqingluo-90 ziqingluo-90 deleted the dev/ziqing/bugfix-116286 branch December 19, 2024 23:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants