[Attributor] Prevent infinite loop in AAGlobalValueInfoFloating

[EthanLuisMcDonough]

Global variables that reference themselves alongside a function that is called indirectly can cause an infinite loop in AAGlobalValueInfoFloating. The recursive reference is continually pushed back into the workload, causing the attributor to hang indefinitely.

[llvmbot]

@llvm/pr-subscribers-llvm-transforms

Author: Ethan Luis McDonough (EthanLuisMcDonough)

Changes

Global variables that reference themselves alongside a function that is called indirectly can cause an infinite loop in AAGlobalValueInfoFloating. The recursive reference is continually pushed back into the workload, causing the attributor to hang indefinitely. This patch fixes the issue by preventing the attributor from following a previously visited use.

---

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

2 Files Affected:

• (modified) llvm/lib/Transforms/IPO/AttributorAttributes.cpp (+1-1)
• (added) llvm/test/Transforms/Attributor/recursive_globals.ll (+24)

diff --git a/llvm/lib/Transforms/IPO/AttributorAttributes.cpp b/llvm/lib/Transforms/IPO/AttributorAttributes.cpp
index 549d03645f939..8c268e3faf177 100644
--- a/llvm/lib/Transforms/IPO/AttributorAttributes.cpp
+++ b/llvm/lib/Transforms/IPO/AttributorAttributes.cpp
@@ -12031,7 +12031,7 @@ struct AAGlobalValueInfoFloating : public AAGlobalValueInfo {
                 SmallVectorImpl<const Value *> &Worklist) {
     Instruction *UInst = dyn_cast<Instruction>(U.getUser());
     if (!UInst) {
-      Follow = true;
+      Follow = !Uses.contains(&U);
       return true;
     }
 
diff --git a/llvm/test/Transforms/Attributor/recursive_globals.ll b/llvm/test/Transforms/Attributor/recursive_globals.ll
new file mode 100644
index 0000000000000..1af6383b063f4
--- /dev/null
+++ b/llvm/test/Transforms/Attributor/recursive_globals.ll
@@ -0,0 +1,24 @@
+; RUN: opt < %s -passes=attributor
+
+@glob1 = global { ptr, ptr } { ptr @glob1, ptr @fnc1 }
+@glob2 = global { ptr, ptr } { ptr @glob3, ptr @fnc2 }
+@glob3 = global { ptr, ptr } { ptr @glob2, ptr @fnc2 }
+
+define internal void @fnc1() {
+  ret void
+}
+
+define internal void @fnc2() {
+  ret void
+}
+
+define internal void @indr_caller(ptr %0) {
+  call void %0()
+  ret void
+}
+
+define void @main() {
+  call void @indr_caller(ptr @fnc1)
+  call void @indr_caller(ptr @fnc2)
+  ret void
+}

[jdoerfert]

The last time I tried to fix this was
96da6dd

I think this needs to go be similar, so, in checkForAllUses, as other places (not AAGlobalValueInfo) have the same problem at the end of the day.
So, above it checks for PHI users, we probably need to add non-init users.

[jdoerfert]

Add a brief comment what this tests, basically what went wrong and why.

[vidsinghal]

I'd recommend squashing all your commits to one.

[jdoerfert]

Suggested change

	bool canMarkAsVisited(const User *user) {
	return isa<PHINode>(user) || !isa<Instruction>(user);
	bool canMarkAsVisited(const User *Usr) {
	return isa<PHINode>(Usr) || !isa<Instruction>(Usr);