[llvm] b67c16f - Verifier: Disallow uses of intrinsic global variables

[Matt Arsenault via llvm-commits]

Author: Matt Arsenault
Date: 2023-01-05T14:51:37-05:00
New Revision: b67c16ff6af557174e498d6d82df9936e333f3d6

URL: https://github.com/llvm/llvm-project/commit/b67c16ff6af557174e498d6d82df9936e333f3d6
DIFF: https://github.com/llvm/llvm-project/commit/b67c16ff6af557174e498d6d82df9936e333f3d6.diff

LOG: Verifier: Disallow uses of intrinsic global variables

appendToGlobalCtors implicitly assumes this is the case, since it
deletes and recreates without trying to update any uses.

This ran into an interesting problem in a few linker tests. During the
link, ConstantExpr casts were speculatively created to replace any
uses that might need them for replacement. These unused ConstantExprs
would hang around and still appear on the use list. It seems like a
bug that those stick around, but I'm not sure where those are supposed
to be cleaned up. Avoid this by not creating the casts for appending
linkage.

Delete one of the casts entirely as it breaks no tests. The verifier
has enforced a specific type for these since 2011, so I don't see why
we would need to handle linking modules with a wrong types. One test
does fail without the second cast (Linker/appending-global-proto.ll,
added by D95126). This test looks contrived; it's using appending
linkage with a regular variable. The LangRef suggests this is illegal
(and suggests another missing verifier check).

Added: 
    llvm/test/Linker/Inputs/funcimport_appending_global_used.ll
    llvm/test/Linker/funcimport_appending_global_used.ll
    llvm/test/Verifier/global-ctors-dtors-uses.ll
    llvm/test/Verifier/used-uses.ll

Modified: 
    llvm/lib/IR/Verifier.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/lib/IR/Verifier.cpp b/llvm/lib/IR/Verifier.cpp
index 6b9a2c38a969b..e4948b1d82800 100644
--- a/llvm/lib/IR/Verifier.cpp
+++ b/llvm/lib/IR/Verifier.cpp
@@ -729,6 +729,9 @@ void Verifier::visitGlobalVariable(const GlobalVariable &GV) {
                        GV.getName() == "llvm.global_dtors")) {
     Check(!GV.hasInitializer() || GV.hasAppendingLinkage(),
           "invalid linkage for intrinsic global variable", &GV);
+    Check(GV.materialized_use_empty(),
+          "invalid uses of intrinsic global variable", &GV);
+
     // Don't worry about emitting an error for it not being an array,
     // visitGlobalValue will complain on appending non-array.
     if (ArrayType *ATy = dyn_cast<ArrayType>(GV.getValueType())) {
@@ -755,6 +758,9 @@ void Verifier::visitGlobalVariable(const GlobalVariable &GV) {
                        GV.getName() == "llvm.compiler.used")) {
     Check(!GV.hasInitializer() || GV.hasAppendingLinkage(),
           "invalid linkage for intrinsic global variable", &GV);
+    Check(GV.materialized_use_empty(),
+          "invalid uses of intrinsic global variable", &GV);
+
     Type *GVType = GV.getValueType();
     if (ArrayType *ATy = dyn_cast<ArrayType>(GVType)) {
       PointerType *PTy = dyn_cast<PointerType>(ATy->getElementType());

diff  --git a/llvm/test/Linker/Inputs/funcimport_appending_global_used.ll b/llvm/test/Linker/Inputs/funcimport_appending_global_used.ll
new file mode 100644
index 0000000000000..5783c0a3ef1ca
--- /dev/null
+++ b/llvm/test/Linker/Inputs/funcimport_appending_global_used.ll
@@ -0,0 +1,6 @@
+ at v = weak global i8 1
+ at llvm.used = appending global [2 x ptr] [ptr @foo, ptr @v]
+
+define void @foo() {
+  ret void
+}

diff  --git a/llvm/test/Linker/funcimport_appending_global_used.ll b/llvm/test/Linker/funcimport_appending_global_used.ll
new file mode 100644
index 0000000000000..b54e6ea71fa7c
--- /dev/null
+++ b/llvm/test/Linker/funcimport_appending_global_used.ll
@@ -0,0 +1,22 @@
+; RUN: opt -module-summary %s -o %t.bc
+; RUN: opt -module-summary %p/Inputs/funcimport_appending_global_used.ll -o %t2.bc
+; RUN: llvm-lto -thinlto -o %t3 %t.bc %t2.bc
+
+; Do the import now
+; RUN: llvm-link %t.bc -summary-index=%t3.thinlto.bc -import=foo:%t2.bc -S | FileCheck %s
+
+; Test case where the verifier would fail if checking use_empty
+; instead of materialized_use_empty on llvm.used.
+
+; CHECK: @llvm.used = appending global [1 x ptr] [ptr @f]
+
+declare void @f()
+ at llvm.used = appending global [1 x ptr] [ptr @f]
+
+define i32 @main() {
+entry:
+  call void @foo()
+  ret i32 0
+}
+
+declare void @foo()

diff  --git a/llvm/test/Verifier/global-ctors-dtors-uses.ll b/llvm/test/Verifier/global-ctors-dtors-uses.ll
new file mode 100644
index 0000000000000..1af4fb7ca9c0e
--- /dev/null
+++ b/llvm/test/Verifier/global-ctors-dtors-uses.ll
@@ -0,0 +1,16 @@
+; RUN: not llvm-as %s -o /dev/null 2>&1 | FileCheck %s
+
+; CHECK: invalid uses of intrinsic global variable
+; CHECK-NEXT: ptr @llvm.global_ctors
+ at llvm.global_ctors = appending global [1 x { i32, ptr, ptr } ] [
+  { i32, ptr, ptr } { i32 65535, ptr null, ptr null }
+]
+
+; CHECK: invalid uses of intrinsic global variable
+; CHECK-NEXT: ptr @llvm.global_dtors
+ at llvm.global_dtors = appending global [1 x { i32, ptr, ptr } ] [
+  { i32, ptr, ptr } { i32 65535, ptr null, ptr null }
+]
+
+ at ctor_user = global ptr @llvm.global_ctors
+ at dtor_user = global ptr @llvm.global_dtors

diff  --git a/llvm/test/Verifier/used-uses.ll b/llvm/test/Verifier/used-uses.ll
new file mode 100644
index 0000000000000..2dda638021e18
--- /dev/null
+++ b/llvm/test/Verifier/used-uses.ll
@@ -0,0 +1,20 @@
+; RUN: not llvm-as %s -o /dev/null 2>&1 | FileCheck %s
+
+; CHECK: invalid uses of intrinsic global variable
+; CHECK-NEXT: ptr @llvm.used
+ at llvm.used = appending global [1 x ptr] [ptr @foo]
+
+; CHECK: invalid uses of intrinsic global variable
+; CHECK-NEXT: ptr @llvm.compiler.used
+ at llvm.compiler.used = appending global [1 x ptr] [ptr @bar]
+
+define void @foo() {
+  ret void
+}
+
+define void @bar() {
+  ret void
+}
+
+ at used_user = global ptr @llvm.used
+ at compiler_used_user = global ptr @llvm.compiler.used