Skip to content

Conversation

@hubert-reinterpretcast
Copy link
Collaborator

Unsigned subtraction wrap-around occurs in emitGlobalConstantImpl on
an AIX-specific code path from 8e4423e when a structure type has
zero elements.

With assertions enabled, this manifests as:

TypeSize llvm::StructLayout::getElementOffset(unsigned int) const: Assertion `Idx < NumElements && "Invalid element idx!"' failed.

Unsigned subtraction wrap-around occurs in `emitGlobalConstantImpl` on
an AIX-specific code path from 8e4423e when a structure type has
zero elements.

With assertions enabled, this manifests as:
```
TypeSize llvm::StructLayout::getElementOffset(unsigned int) const: Assertion `Idx < NumElements && "Invalid element idx!"' failed.
```
@llvmbot
Copy link
Member

llvmbot commented Jan 9, 2025

@llvm/pr-subscribers-backend-powerpc

Author: Hubert Tong (hubert-reinterpretcast)

Changes

Unsigned subtraction wrap-around occurs in emitGlobalConstantImpl on
an AIX-specific code path from 8e4423e when a structure type has
zero elements.

With assertions enabled, this manifests as:

TypeSize llvm::StructLayout::getElementOffset(unsigned int) const: Assertion `Idx &lt; NumElements &amp;&amp; "Invalid element idx!"' failed.

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

2 Files Affected:

  • (modified) llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp (+14-13)
  • (added) llvm/test/CodeGen/PowerPC/global-merge-aix-zero-size-struct.ll (+20)
diff --git a/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp b/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
index 7bd3fb33b47d2b..3ba45900e45691 100644
--- a/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
@@ -3914,21 +3914,22 @@ static void emitGlobalConstantImpl(const DataLayout &DL, const Constant *CV,
   if (isa<ConstantAggregateZero>(CV)) {
     StructType *structType;
     if (AliasList && (structType = llvm::dyn_cast<StructType>(CV->getType()))) {
-      // Handle cases of aliases to direct struct elements
-      const StructLayout *Layout = DL.getStructLayout(structType);
-      uint64_t SizeSoFar = 0;
-      for (unsigned int i = 0, n = structType->getNumElements(); i < n - 1;
-           ++i) {
-        uint64_t GapToNext = Layout->getElementOffset(i + 1) - SizeSoFar;
-        AP.OutStreamer->emitZeros(GapToNext);
-        SizeSoFar += GapToNext;
-        emitGlobalAliasInline(AP, Offset + SizeSoFar, AliasList);
+      unsigned numElements = {structType->getNumElements()};
+      if (numElements != 0) {
+        // Handle cases of aliases to direct struct elements
+        const StructLayout *Layout = DL.getStructLayout(structType);
+        uint64_t SizeSoFar = 0;
+        for (unsigned int i = 0; i < numElements - 1; ++i) {
+          uint64_t GapToNext = Layout->getElementOffset(i + 1) - SizeSoFar;
+          AP.OutStreamer->emitZeros(GapToNext);
+          SizeSoFar += GapToNext;
+          emitGlobalAliasInline(AP, Offset + SizeSoFar, AliasList);
+        }
+        AP.OutStreamer->emitZeros(Size - SizeSoFar);
+        return;
       }
-      AP.OutStreamer->emitZeros(Size - SizeSoFar);
-      return;
-    } else {
-      return AP.OutStreamer->emitZeros(Size);
     }
+    return AP.OutStreamer->emitZeros(Size);
   }
 
   if (isa<UndefValue>(CV))
diff --git a/llvm/test/CodeGen/PowerPC/global-merge-aix-zero-size-struct.ll b/llvm/test/CodeGen/PowerPC/global-merge-aix-zero-size-struct.ll
new file mode 100644
index 00000000000000..ec6fd7ee4cf4ca
--- /dev/null
+++ b/llvm/test/CodeGen/PowerPC/global-merge-aix-zero-size-struct.ll
@@ -0,0 +1,20 @@
+; RUN: llc -verify-machineinstrs -mtriple powerpc64-ibm-aix-xcoff -mcpu=pwr7 < %s | FileCheck %s
+
+; RUN: llc -verify-machineinstrs -mtriple powerpc64-ibm-aix-xcoff -mcpu=pwr7 --filetype=obj -o %t.o < %s
+; RUN: llvm-objdump --syms %t.o | FileCheck %s --check-prefix=OBJ
+
+%struct.anon = type {}
+
+@a = internal constant %struct.anon zeroinitializer, align 1
+@b = internal constant [6 x i8] c"hello\00", align 1
+
+; CHECK:      	.csect L.._MergedGlobals[RO],2
+; CHECK-NEXT: 	.lglobl	a                               # @_MergedGlobals
+; CHECK-NEXT: 	.lglobl	b
+; CHECK-NEXT: a:
+; CHECK-NEXT: b:
+; CHECK-NEXT: 	.string	"hello"
+
+; OBJ:      0000000000000000 l       .text	0000000000000006 L.._MergedGlobals
+; OBJ-NEXT: 0000000000000000 l       .text (csect: L.._MergedGlobals) 	0000000000000000 a
+; OBJ-NEXT: 0000000000000000 l       .text (csect: L.._MergedGlobals) 	0000000000000000 b

Copy link
Member

@mustartt mustartt left a comment

Choose a reason for hiding this comment

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

Thanks for fixing this. LGTM

@hubert-reinterpretcast hubert-reinterpretcast merged commit e438513 into main Jan 9, 2025
8 of 11 checks passed
@hubert-reinterpretcast hubert-reinterpretcast deleted the users/hubert-reinterpretcast/AsmPrinter-FixAIXUnsignedWraparoundWithZeroElementStructType branch January 9, 2025 04:07
@llvm-ci
Copy link
Collaborator

llvm-ci commented Jan 9, 2025

LLVM Buildbot has detected a new failure on builder clang-cmake-x86_64-avx512-linux running on avx512-intel64 while building llvm at step 1 "Checkout lnt".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/133/builds/9352

Here is the relevant piece of the build log for the reference
Step 1 (Checkout lnt) failure: update (failure)
git version 2.31.1
From https://github.com/llvm/llvm-lnt
 * branch            HEAD       -> FETCH_HEAD
fatal: unable to write new index file
From https://github.com/llvm/llvm-lnt
 * branch            HEAD       -> FETCH_HEAD
fatal: unable to write new index file

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants