[LLD][ELF] Generically report "address assignment did not converge"

[mysterymath]

There are considerable number of changes done in the address assignment fixed point loop, and errors in any of them could cause address assignment not to converge. However, this is reported to the user as either "thunk creation not converged" or "relaxation not converged".

We saw a confused bug about this in the wild when spilling failed to converge. (I'm working on a fix for that.)

We may eventually want a complete reason system when reporting address assignment taking too many passes, but in the interim it seems prudent to generalize the error message to "address assignment did not converge".

[llvmbot]

@llvm/pr-subscribers-lld

Author: Daniel Thornburgh (mysterymath)

Changes

There are considerable number of changes done in the address assignment fixed point loop, and errors in any of them could cause address assignment not to converge. However, this is reported to the user as either "thunk creation not converged" or "relaxation not converged".

We saw a confused bug about this in the wild when spilling failed to converge. (I'm working on a fix for that.)

We may eventually want a complete reason system when reporting address assignment taking too many passes, but in the interim it seems prudent to generalize the error message to "address assignment did not converge".

---

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

2 Files Affected:

• (modified) lld/ELF/Writer.cpp (+1-2)
• (modified) lld/test/ELF/linkerscript/symbol-assign-many-passes2.test (+1-1)

diff --git a/lld/ELF/Writer.cpp b/lld/ELF/Writer.cpp
index a2c49343e5c8d..0d61e8d8d91a4 100644
--- a/lld/ELF/Writer.cpp
+++ b/lld/ELF/Writer.cpp
@@ -1531,8 +1531,7 @@ template <class ELFT> void Writer<ELFT>::finalizeAddressDependentContent() {
     // With Thunk Size much smaller than branch range we expect to
     // converge quickly; if we get to 30 something has gone wrong.
     if (changed && pass >= 30) {
-      Err(ctx) << (ctx.target->needsThunks ? "thunk creation not converged"
-                                           : "relaxation not converged");
+      Err(ctx) << "address assignment did not converge";
       break;
     }
 
diff --git a/lld/test/ELF/linkerscript/symbol-assign-many-passes2.test b/lld/test/ELF/linkerscript/symbol-assign-many-passes2.test
index 18dc5019ee1eb..b41467d6168c6 100644
--- a/lld/test/ELF/linkerscript/symbol-assign-many-passes2.test
+++ b/lld/test/ELF/linkerscript/symbol-assign-many-passes2.test
@@ -6,7 +6,7 @@
 ## arm-thunk-many-passes.s is worst case case of thunk generation that takes 9
 ## passes to converge. It takes a few more passes to make symbol assignment
 ## converge. Test that
-## 1. we don't error that "thunk creation not converged".
+## 1. we don't error that "address assignment not converged".
 ## 2. we check convergence of symbols defined in an output section descriptor.
 
 # CHECK: 01011050 T a

[llvmbot]

@llvm/pr-subscribers-lld-elf

Author: Daniel Thornburgh (mysterymath)

Changes

There are considerable number of changes done in the address assignment fixed point loop, and errors in any of them could cause address assignment not to converge. However, this is reported to the user as either "thunk creation not converged" or "relaxation not converged".

We saw a confused bug about this in the wild when spilling failed to converge. (I'm working on a fix for that.)

We may eventually want a complete reason system when reporting address assignment taking too many passes, but in the interim it seems prudent to generalize the error message to "address assignment did not converge".

---

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

2 Files Affected:

• (modified) lld/ELF/Writer.cpp (+1-2)
• (modified) lld/test/ELF/linkerscript/symbol-assign-many-passes2.test (+1-1)

diff --git a/lld/ELF/Writer.cpp b/lld/ELF/Writer.cpp
index a2c49343e5c8d..0d61e8d8d91a4 100644
--- a/lld/ELF/Writer.cpp
+++ b/lld/ELF/Writer.cpp
@@ -1531,8 +1531,7 @@ template <class ELFT> void Writer<ELFT>::finalizeAddressDependentContent() {
     // With Thunk Size much smaller than branch range we expect to
     // converge quickly; if we get to 30 something has gone wrong.
     if (changed && pass >= 30) {
-      Err(ctx) << (ctx.target->needsThunks ? "thunk creation not converged"
-                                           : "relaxation not converged");
+      Err(ctx) << "address assignment did not converge";
       break;
     }
 
diff --git a/lld/test/ELF/linkerscript/symbol-assign-many-passes2.test b/lld/test/ELF/linkerscript/symbol-assign-many-passes2.test
index 18dc5019ee1eb..b41467d6168c6 100644
--- a/lld/test/ELF/linkerscript/symbol-assign-many-passes2.test
+++ b/lld/test/ELF/linkerscript/symbol-assign-many-passes2.test
@@ -6,7 +6,7 @@
 ## arm-thunk-many-passes.s is worst case case of thunk generation that takes 9
 ## passes to converge. It takes a few more passes to make symbol assignment
 ## converge. Test that
-## 1. we don't error that "thunk creation not converged".
+## 1. we don't error that "address assignment not converged".
 ## 2. we check convergence of symbols defined in an output section descriptor.
 
 # CHECK: 01011050 T a

[mysterymath]

Another option would be to specifically call out spilling, but a pathological linker script symbol assignment could also cause linker failure to occur, so that should be called out too. Looking at finalizeAddressDependentContent, I'm not immediately sure what types of user input could cause the various parts of this loop to fail; there may be more we'd need to break out.

[smithp35]

At the time the error message was written, there was only Thunk generation that was at risk of non-convergence so it made sense to write that at the time.

I think this is a good change to make.

Although I can't immediately see an easy way to do so other than optional trace messages, it may be worth some effort to invest in some way to figure out which of the various systems is having difficulty.

Please give MaskRay some time to see if he has any opinions.