Skip to content

Conversation

cor3ntin
Copy link
Contributor

The order of operation was slightly incorrect, as we were checking for incomplete types before handling reference types.

Fixes #129397


@cor3ntin cor3ntin added this to the LLVM 20.X Release milestone May 12, 2025
@github-project-automation github-project-automation bot moved this to Needs Triage in LLVM Release Status May 12, 2025
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels May 12, 2025
@cor3ntin cor3ntin added release:backport regression:20 Regression in 20 release and removed clang Clang issues not falling into any other category labels May 12, 2025
@llvmbot
Copy link
Member

llvmbot commented May 12, 2025

@llvm/pr-subscribers-clang

Author: cor3ntin (cor3ntin)

Changes

The order of operation was slightly incorrect, as we were checking for incomplete types before handling reference types.

Fixes #129397



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

2 Files Affected:

  • (modified) clang/lib/AST/ExprConstant.cpp (+5-3)
  • (modified) clang/test/SemaCXX/builtin-object-size-cxx14.cpp (+12)
diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index 5aae78dd2fee7..23602362eaa79 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -12710,11 +12710,13 @@ static bool determineEndOffset(EvalInfo &Info, SourceLocation ExprLoc,
   bool DetermineForCompleteObject = refersToCompleteObject(LVal);
 
   auto CheckedHandleSizeof = [&](QualType Ty, CharUnits &Result) {
-    if (Ty.isNull() || Ty->isIncompleteType() || Ty->isFunctionType())
+    if (Ty.isNull())
       return false;
 
-    if (Ty->isReferenceType())
-      Ty = Ty.getNonReferenceType();
+    Ty = Ty.getNonReferenceType();
+
+    if (Ty->isIncompleteType() || Ty->isFunctionType())
+      return false;
 
     return HandleSizeof(Info, ExprLoc, Ty, Result);
   };
diff --git a/clang/test/SemaCXX/builtin-object-size-cxx14.cpp b/clang/test/SemaCXX/builtin-object-size-cxx14.cpp
index b7c6f6be01f54..fdd3cb7af088f 100644
--- a/clang/test/SemaCXX/builtin-object-size-cxx14.cpp
+++ b/clang/test/SemaCXX/builtin-object-size-cxx14.cpp
@@ -1,5 +1,7 @@
 // RUN: %clang_cc1 -fsyntax-only -verify=expected,cxx14 -std=c++14 %s
 // RUN: %clang_cc1 -fsyntax-only -verify -std=c++2a %s
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++2b %s
+
 
 typedef __SIZE_TYPE__ size_t;
 
@@ -119,3 +121,13 @@ constexpr int bos_new() { // cxx14-error {{constant expression}}
   void *p = new int; // cxx14-note {{until C++20}}
   return __builtin_object_size(p, 0);
 }
+
+
+namespace GH129397 {
+
+struct incomplete;
+void test(incomplete &ref) {
+  __builtin_object_size(&ref, 1);
+}
+
+}

@github-project-automation github-project-automation bot moved this from Needs Triage to Needs Merge in LLVM Release Status May 12, 2025
…ze (llvm#138247)

The order of operation was slightly incorrect, as we were checking for
incomplete types *before* handling reference types.

Fixes llvm#129397

---------

Co-authored-by: Erich Keane <[email protected]>
@tstellar tstellar force-pushed the corentin/cherry-pick-cb068dc branch from 814e1d2 to 0439d1d Compare May 13, 2025 21:36
@llvmbot llvmbot added the clang Clang issues not falling into any other category label May 13, 2025
@tstellar tstellar merged commit 0439d1d into llvm:release/20.x May 13, 2025
7 of 9 checks passed
@github-project-automation github-project-automation bot moved this from Needs Merge to Done in LLVM Release Status May 13, 2025
Copy link

@cor3ntin (or anyone else). If you would like to add a note about this fix in the release notes (completely optional). Please reply to this comment with a one or two sentence description of the fix. When you are done, please add the release:note label to this PR.

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 regression:20 Regression in 20 release release:backport
Projects
Development

Successfully merging this pull request may close these issues.

4 participants