Skip to content

[C2y] Fix _Countof handling of VLAs #141621

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
May 27, 2025

Conversation

AaronBallman
Copy link
Collaborator

It turns out that getVLASize() does not get you the size of a single dimension of the VLA, it gets you the full count of all elements. This caused _Countof to return invalid values on VLA ranks. Now switched to using getVLAElements1D() instead, which only gets a single dimension.

Fixes #141409

It turns out that getVLASize() does not get you the size of a single
dimension of the VLA, it gets you the full count of all elements. This
caused _Countof to return invalid values on VLA ranks. Now switched to
using getVLAElements1D() instead, which only gets a single dimension.

Fixes llvm#141409
@AaronBallman AaronBallman added clang Clang issues not falling into any other category clang:codegen IR generation bugs: mangling, exceptions, etc. c2y labels May 27, 2025
@llvmbot
Copy link
Member

llvmbot commented May 27, 2025

@llvm/pr-subscribers-clang-codegen

@llvm/pr-subscribers-clang

Author: Aaron Ballman (AaronBallman)

Changes

It turns out that getVLASize() does not get you the size of a single dimension of the VLA, it gets you the full count of all elements. This caused _Countof to return invalid values on VLA ranks. Now switched to using getVLAElements1D() instead, which only gets a single dimension.

Fixes #141409


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

2 Files Affected:

  • (modified) clang/lib/CodeGen/CGExprScalar.cpp (+11-5)
  • (modified) clang/test/C/C2y/n3369_3.c (+54)
diff --git a/clang/lib/CodeGen/CGExprScalar.cpp b/clang/lib/CodeGen/CGExprScalar.cpp
index b9eb12efd6670..0b0428d1e916e 100644
--- a/clang/lib/CodeGen/CGExprScalar.cpp
+++ b/clang/lib/CodeGen/CGExprScalar.cpp
@@ -3574,20 +3574,26 @@ ScalarExprEmitter::VisitUnaryExprOrTypeTraitExpr(
           CGF.EmitIgnoredExpr(E->getArgumentExpr());
         }
 
-        auto VlaSize = CGF.getVLASize(VAT);
-        llvm::Value *size = VlaSize.NumElts;
-
         // For sizeof and __datasizeof, we need to scale the number of elements
         // by the size of the array element type. For _Countof, we just want to
         // return the size directly.
+        llvm::Value *Size;
         if (Kind != UETT_CountOf) {
+          auto VlaSize = CGF.getVLASize(VAT);
+          Size = VlaSize.NumElts;
+
           // Scale the number of non-VLA elements by the non-VLA element size.
           CharUnits eltSize = CGF.getContext().getTypeSizeInChars(VlaSize.Type);
           if (!eltSize.isOne())
-            size = CGF.Builder.CreateNUWMul(CGF.CGM.getSize(eltSize), size);
+            Size = CGF.Builder.CreateNUWMul(CGF.CGM.getSize(eltSize), Size);
+        } else {
+          // For _Countof, we don't want the size of the entire VLA, just the
+          // single dimension we're currently looking at.
+          auto VlaSize = CGF.getVLAElements1D(VAT);
+          Size = VlaSize.NumElts;
         }
 
-        return size;
+        return Size;
       }
     }
   } else if (E->getKind() == UETT_OpenMPRequiredSimdAlign) {
diff --git a/clang/test/C/C2y/n3369_3.c b/clang/test/C/C2y/n3369_3.c
index d3909b2e5bc92..792c076413372 100644
--- a/clang/test/C/C2y/n3369_3.c
+++ b/clang/test/C/C2y/n3369_3.c
@@ -164,3 +164,57 @@ void test4(int n) {
   int y = _Countof(int[n++][7]);
 }
 
+// CHECK-64-LABEL: define dso_local void @test5(
+// CHECK-64-SAME: ) #[[ATTR0]] {
+// CHECK-64-NEXT:  [[ENTRY:.*:]]
+// CHECK-64-NEXT:    [[J:%.*]] = alloca i32, align
+// CHECK-64-NEXT:    [[SAVED_STACK:%.*]] = alloca ptr, align
+// CHECK-64-NEXT:    [[A:%.*]] = alloca i32, align
+// CHECK-64-NEXT:    [[B:%.*]] = alloca i32, align
+// CHECK-64-NEXT:    [[C:%.*]] = alloca i32, align
+// CHECK-64-NEXT:    [[D:%.*]] = alloca i32, align
+// CHECK-64-NEXT:    store i32 2, ptr [[J]], align
+// CHECK-64-NEXT:    [[TMP0:%.*]] = call ptr @llvm.stacksave.p0()
+// CHECK-64-NEXT:    store ptr [[TMP0]], ptr [[SAVED_STACK]], align
+// CHECK-64-NEXT:    [[VLA:%.*]] = alloca i32, i64 120, align
+// CHECK-64-NEXT:    store i32 1, ptr [[A]], align
+// CHECK-64-NEXT:    store i32 4, ptr [[B]], align
+// CHECK-64-NEXT:    store i32 5, ptr [[C]], align
+// CHECK-64-NEXT:    store i32 6, ptr [[D]], align
+// CHECK-64-NEXT:    [[TMP1:%.*]] = load ptr, ptr [[SAVED_STACK]], align
+// CHECK-64-NEXT:    call void @llvm.stackrestore.p0(ptr [[TMP1]])
+// CHECK-64-NEXT:    ret void
+//
+// CHECK-32-LABEL: define dso_local void @test5(
+// CHECK-32-SAME: ) #[[ATTR0]] {
+// CHECK-32-NEXT:  [[ENTRY:.*:]]
+// CHECK-32-NEXT:    [[J:%.*]] = alloca i32, align
+// CHECK-32-NEXT:    [[SAVED_STACK:%.*]] = alloca ptr, align
+// CHECK-32-NEXT:    [[A:%.*]] = alloca i32, align
+// CHECK-32-NEXT:    [[B:%.*]] = alloca i32, align
+// CHECK-32-NEXT:    [[C:%.*]] = alloca i32, align
+// CHECK-32-NEXT:    [[D:%.*]] = alloca i32, align
+// CHECK-32-NEXT:    store i32 2, ptr [[J]], align
+// CHECK-32-NEXT:    [[TMP0:%.*]] = call ptr @llvm.stacksave.p0()
+// CHECK-32-NEXT:    store ptr [[TMP0]], ptr [[SAVED_STACK]], align
+// CHECK-32-NEXT:    [[VLA:%.*]] = alloca i32, i32 120, align
+// CHECK-32-NEXT:    store i32 1, ptr [[A]], align
+// CHECK-32-NEXT:    store i32 4, ptr [[B]], align
+// CHECK-32-NEXT:    store i32 5, ptr [[C]], align
+// CHECK-32-NEXT:    store i32 6, ptr [[D]], align
+// CHECK-32-NEXT:    [[TMP1:%.*]] = load ptr, ptr [[SAVED_STACK]], align
+// CHECK-32-NEXT:    call void @llvm.stackrestore.p0(ptr [[TMP1]])
+// CHECK-32-NEXT:    ret void
+//
+void test5() {
+  // Ensure we're getting the variable-length dimensions correctly. We were
+  // previously returning the size of all VLA dimensions multiplied together
+  // to get the total element count rather than the element count for a single
+  // array rank. See GH141409
+  const int j = 2;
+  int arr[1][j + 2][j + 3][j + 4];
+  int a = _Countof arr;
+  int b = _Countof *arr;
+  int c = _Countof **arr;
+  int d = _Countof ***arr;
+}

Copy link
Collaborator

@efriedma-quic efriedma-quic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@AaronBallman AaronBallman merged commit d1a6327 into llvm:main May 27, 2025
10 of 11 checks passed
@AaronBallman AaronBallman deleted the aballman-gh141409 branch May 27, 2025 18:59
sivan-shani pushed a commit to sivan-shani/llvm-project that referenced this pull request Jun 3, 2025
It turns out that getVLASize() does not get you the size of a single
dimension of the VLA, it gets you the full count of all elements. This
caused _Countof to return invalid values on VLA ranks. Now switched to
using getVLAElements1D() instead, which only gets a single dimension.

Fixes llvm#141409
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c2y clang:codegen IR generation bugs: mangling, exceptions, etc. clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Clang] C2Y _Countof weird sub-array count with multidimensional VLA
4 participants