[Verifier] Make verifier fail when global variable size exceeds address space size

[steffenlarsen]

When a global variable has a size that exceeds the size of the address space it resides in, the verifier should fail as the variable can neither be materialized nor fully accessed. This patch adds a check to the verifier to enforce it.

[llvmbot]

@llvm/pr-subscribers-llvm-transforms

@llvm/pr-subscribers-backend-loongarch

Author: Steffen Larsen (steffenlarsen)

Changes

When a global variable has a size that exceeds the size of the address space it resides in, the verifier should fail as the variable can neither be materialized nor fully accessed. This patch adds a check to the verifier to enforce it.

---

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

5 Files Affected:

• (modified) llvm/lib/IR/Verifier.cpp (+10)
• (modified) llvm/test/CodeGen/LoongArch/merge-base-offset-tlsle.ll (+1-1)
• (modified) llvm/test/CodeGen/LoongArch/merge-base-offset.ll (+1-1)
• (modified) llvm/test/Transforms/GlobalOpt/large-element-size.ll (+1-1)
• (added) llvm/test/Verifier/global-var-too-big.ll (+21)

diff --git a/llvm/lib/IR/Verifier.cpp b/llvm/lib/IR/Verifier.cpp
index 01d32d4a1825f..8d2d36c813207 100644
--- a/llvm/lib/IR/Verifier.cpp
+++ b/llvm/lib/IR/Verifier.cpp
@@ -955,6 +955,16 @@ void Verifier::visitGlobalVariable(const GlobalVariable &GV) {
         "Global @" + GV.getName() + " has illegal target extension type",
         GVType);
 
+  // Check that the the address space can hold all bits of the type, recognized
+  // by an access in the address space being able to reach all bytes of the
+  // type.
+  Check(GVType->isScalableTy() || !GVType->isSized() ||
+            GV.getGlobalSize(DL) == 0 ||
+            isUIntN(DL.getAddressSizeInBits(GV.getAddressSpace()),
+                    GV.getGlobalSize(DL) - 1),
+        "Global variable is too large to fit into the address space", &GV,
+        GVType);
+
   if (!GV.hasInitializer()) {
     visitGlobalValue(GV);
     return;
diff --git a/llvm/test/CodeGen/LoongArch/merge-base-offset-tlsle.ll b/llvm/test/CodeGen/LoongArch/merge-base-offset-tlsle.ll
index 97d33379913e5..6605d5961aa6a 100644
--- a/llvm/test/CodeGen/LoongArch/merge-base-offset-tlsle.ll
+++ b/llvm/test/CodeGen/LoongArch/merge-base-offset-tlsle.ll
@@ -661,7 +661,7 @@ if.end:
   ret void
 }
 
-@g_a64 = dso_local thread_local(localexec) global [614750729487779976 x i64] zeroinitializer, align 8
+@g_a64 = dso_local thread_local(localexec) global [536870912 x i64] zeroinitializer, align 8
 
 define dso_local ptr @tlsle_load_addr_offset_1() nounwind {
 ; LA32-LABEL: tlsle_load_addr_offset_1:
diff --git a/llvm/test/CodeGen/LoongArch/merge-base-offset.ll b/llvm/test/CodeGen/LoongArch/merge-base-offset.ll
index ea0fe3d0b0178..4cda6a708a80c 100644
--- a/llvm/test/CodeGen/LoongArch/merge-base-offset.ll
+++ b/llvm/test/CodeGen/LoongArch/merge-base-offset.ll
@@ -959,7 +959,7 @@ label:
   ret ptr %0
 }
 
-@g_a64 = dso_local global [614750729487779976 x i64] zeroinitializer, align 8
+@g_a64 = dso_local global [536870912 x i64] zeroinitializer, align 8
 
 define dso_local ptr @load_addr_offset_1() nounwind {
 ; LA32-LABEL: load_addr_offset_1:
diff --git a/llvm/test/Transforms/GlobalOpt/large-element-size.ll b/llvm/test/Transforms/GlobalOpt/large-element-size.ll
index 914112af313a5..14a3cd035d26f 100644
--- a/llvm/test/Transforms/GlobalOpt/large-element-size.ll
+++ b/llvm/test/Transforms/GlobalOpt/large-element-size.ll
@@ -6,7 +6,7 @@ target datalayout = "p:32:32"
 %struct.t.1 = type { %struct.u.0, %struct.u.0, %struct.u.0, %struct.u.0, i32, i32, i32, i32 }
 %struct.u.0 = type { i32, i32, i32, i8 }
 
-@s = external global [700 x [24000 x %struct.s.2]], align 1
+@s = external global [700 x [2400 x %struct.s.2]], align 1
 @p = global ptr getelementptr (i8, ptr @s, i64 2247483647), align 1
 
 ; CHECK: @p = local_unnamed_addr global ptr getelementptr (i8, ptr @s, i32 -2047483649), align 1
diff --git a/llvm/test/Verifier/global-var-too-big.ll b/llvm/test/Verifier/global-var-too-big.ll
new file mode 100644
index 0000000000000..478b093eb0bae
--- /dev/null
+++ b/llvm/test/Verifier/global-var-too-big.ll
@@ -0,0 +1,21 @@
+; RUN: not opt -S -passes=verify < %s 2>&1 | FileCheck %s
+
+target datalayout = "e-m:e-p1:16:16-p2:32:32-p3:64:64-i8:8-i32:32-i64:64"
+
+; Too large for 16-bit address space.
+@G1 = internal addrspace(1) global [65537 x i8] zeroinitializer, align 4
+
+; Too large for 32-bit address space.
+@G2 = internal addrspace(2) global [2147483649 x i16] zeroinitializer, align 4
+
+; Fit within the address spaces
+@G3 = internal addrspace(1) global [65536 x i8] zeroinitializer, align 4
+@G4 = internal addrspace(2) global [2147483648 x i16] zeroinitializer, align 4
+
+; CHECK: Global variable is too large to fit into the address space
+; CHECK-NEXT: ptr addrspace(1) @G1
+; CHECK-NEXT: [65537 x i8]
+; CHECK: Global variable is too large to fit into the address space
+; CHECK-NEXT: ptr addrspace(2) @G2
+; CHECK-NEXT: [2147483649 x i16]
+; CHECK-NOT: Global variable is too large to fit into the address space

[steffenlarsen]

Tag @nikic & @boomanaiden154 from #178391 for awareness. ⭐

[nikic]

Strictly speaking objects can only take up half the address space. But even if we don't enforce that as an IR well-formedness requirement, I think we should at least be dropping the -1 here, otherwise we're still going to be dealing with overflow issues.

[nikic]

Probably worth noting that this doesn't catch cases that overflow during the size calculation, but that's something we can handle separately...

[nikic]

LGTM