Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

IRSymTab: Record _GLOBAL_OFFSET_TABLE_ for ELF x86 #89463

Conversation

MaskRay
Copy link
Member

@MaskRay MaskRay commented Apr 19, 2024

In ELF, relocatable files generated for x86-32 and some code models of
x86-64 (medium, large) may reference the special symbol
_GLOBAL_OFFSET_TABLE_ that is not used in the IR. In an LTO link, if
there is no regular relocatable file referencing the special symbol, the
linker may not define the symbol and lead to a spurious "undefined
symbol" error.

Fix #61101: record that _GLOBAL_OFFSET_TABLE_ is used in the IR symbol
table.

Note: The PreservedSymbols mechanism
(https://reviews.llvm.org/D112595) that just sets FB_used is not
applicable.
The getRuntimeLibcallSymbols for extracting lazy runtime library
symbols is for symbols that are "always" potentially used, but linkers
don't have the code model information to make a precise decision.

Created using spr 1.3.5-bogner
@MaskRay MaskRay requested a review from aeubanks April 19, 2024 22:21
@llvmbot llvmbot added lld lld:ELF LTO Link time optimization (regular/full LTO or ThinLTO) llvm:binary-utilities labels Apr 19, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Apr 19, 2024

@llvm/pr-subscribers-llvm-binary-utilities

@llvm/pr-subscribers-lld-elf

Author: Fangrui Song (MaskRay)

Changes

In ELF, relocatable files generated for x86-32 and some code models of
x86-64 (medium, large) may reference the special symbol
_GLOBAL_OFFSET_TABLE_ that is not used in the IR. In an LTO link, if
there is no regular relocatable file referencing the special symbol, the
linker may not define the symbol and lead to a spurious "undefined
symbol" error.

Fix #61101: record that _GLOBAL_OFFSET_TABLE_ is used in the IR symbol
table. The PreservedSymbols mechanism
(https://reviews.llvm.org/D112595) that just sets FB_used is not
applicable.


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

8 Files Affected:

  • (added) lld/test/ELF/lto/i386-global-offset-table.ll (+22)
  • (added) lld/test/ELF/lto/x86-64-global-offset-table.ll (+57)
  • (modified) llvm/lib/Object/ModuleSymbolTable.cpp (+14)
  • (modified) llvm/test/LTO/X86/codemodel-2.ll (+1-1)
  • (modified) llvm/test/LTO/X86/codemodel-3.ll (+2-1)
  • (modified) llvm/test/LTO/X86/largedatathreshold-1.ll (+1-1)
  • (modified) llvm/test/LTO/X86/largedatathreshold-2.ll (+1-1)
  • (modified) llvm/test/LTO/X86/largedatathreshold-3.ll (+2-1)
diff --git a/lld/test/ELF/lto/i386-global-offset-table.ll b/lld/test/ELF/lto/i386-global-offset-table.ll
new file mode 100644
index 00000000000000..e470763a3eb2ec
--- /dev/null
+++ b/lld/test/ELF/lto/i386-global-offset-table.ll
@@ -0,0 +1,22 @@
+; REQUIRES: x86
+; RUN: llvm-as %s -o %t.bc
+; RUN: ld.lld %t.bc -o %t
+
+target datalayout = "e-m:e-p:32:32-p270:32:32-p271:32:32-p272:64:64-i128:128-f64:32:64-f80:32-n8:16:32-S128"
+target triple = "i386-pc-linux-gnu"
+
+define dso_local void @f() {
+entry:
+  ret void
+}
+
+define dso_local void @_start() {
+entry:
+  call void @f()
+  ret void
+}
+
+!llvm.module.flags = !{!0, !1}
+
+!0 = !{i32 8, !"PIC Level", i32 2}
+!1 = !{i32 7, !"PIE Level", i32 2}
diff --git a/lld/test/ELF/lto/x86-64-global-offset-table.ll b/lld/test/ELF/lto/x86-64-global-offset-table.ll
new file mode 100644
index 00000000000000..1db1027e7a21c5
--- /dev/null
+++ b/lld/test/ELF/lto/x86-64-global-offset-table.ll
@@ -0,0 +1,57 @@
+; REQUIRES: x86
+; RUN: rm -rf %t && split-file %s %t && cd %t
+; RUN: llvm-mc -filetype=obj -triple=x86_64 b.s -o b.o
+
+; RUN: cat a.ll medium.ll | llvm-as - -o medium.bc
+; RUN: ld.lld -pie --no-relax medium.bc b.o -o medium
+; RUN: llvm-objdump -d medium | FileCheck %s
+
+; RUN: cat a.ll large.ll | llvm-as - -o large.bc
+; RUN: ld.lld -pie large.bc b.o -o large
+; RUN: llvm-objdump -d large | FileCheck %s
+
+; RUN: cat a.ll medium.ll ref.ll | llvm-as - -o ref.bc
+; RUN: ld.lld -pie --no-relax -u ref ref.bc b.o -o ref
+; RUN: llvm-objdump -d ref | FileCheck %s
+
+; CHECK: movabsq
+
+;--- a.ll
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-i128:128-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+@i = external global i32
+
+define dso_local void @_start() {
+entry:
+  %0 = load i32, ptr @i
+  %inc = add nsw i32 %0, 1
+  store i32 %inc, ptr @i
+  ret void
+}
+
+!llvm.module.flags = !{!0, !1, !2, !3}
+
+!0 = !{i32 8, !"PIC Level", i32 2}
+!1 = !{i32 7, !"PIE Level", i32 2}
+!2 = !{i32 1, !"Large Data Threshold", i64 0}
+
+;--- medium.ll
+!3 = !{i32 1, !"Code Model", i32 3}
+
+;--- large.ll
+!3 = !{i32 1, !"Code Model", i32 4}
+
+;--- ref.ll
+@_GLOBAL_OFFSET_TABLE_ = external global [0 x i8]
+
+define dso_local ptr @ref() {
+entry:
+  ret ptr @_GLOBAL_OFFSET_TABLE_
+}
+
+;--- b.s
+.data
+.globl i
+i:
+.long 0
diff --git a/llvm/lib/Object/ModuleSymbolTable.cpp b/llvm/lib/Object/ModuleSymbolTable.cpp
index 07f76688fa43e7..d8f520ad02c2f2 100644
--- a/llvm/lib/Object/ModuleSymbolTable.cpp
+++ b/llvm/lib/Object/ModuleSymbolTable.cpp
@@ -175,6 +175,20 @@ void ModuleSymbolTable::CollectAsmSymbols(
       AsmSymbol(Key, BasicSymbolRef::Flags(Res));
     }
   });
+
+  // In ELF, object code generated for x86-32 and some code models of x86-64 may
+  // reference the special symbol _GLOBAL_OFFSET_TABLE_ that is not used in the
+  // IR. Record it like inline asm symbols.
+  Triple TT(M.getTargetTriple());
+  if (!TT.isOSBinFormatELF() || !TT.isX86())
+    return;
+  auto CM = M.getCodeModel();
+  if (TT.getArch() == Triple::x86 || CM == CodeModel::Medium ||
+      CM == CodeModel::Large) {
+    AsmSymbol("_GLOBAL_OFFSET_TABLE_",
+              BasicSymbolRef::Flags(BasicSymbolRef::SF_Undefined |
+                                    BasicSymbolRef::SF_Global));
+  }
 }
 
 void ModuleSymbolTable::CollectAsmSymvers(
diff --git a/llvm/test/LTO/X86/codemodel-2.ll b/llvm/test/LTO/X86/codemodel-2.ll
index 5cd9731606f2bd..fc1074bcf2235c 100644
--- a/llvm/test/LTO/X86/codemodel-2.ll
+++ b/llvm/test/LTO/X86/codemodel-2.ll
@@ -1,5 +1,5 @@
 ; RUN: llvm-as %s -o %t.o
-; RUN: llvm-lto2 run -r %t.o,_start,px %t.o -o %t.s
+; RUN: llvm-lto2 run -r %t.o,_start,px -r %t.o,_GLOBAL_OFFSET_TABLE_, %t.o -o %t.s
 ; RUN: llvm-objdump --no-print-imm-hex -d %t.s.0 | FileCheck %s --check-prefix=CHECK-LARGE
 
 target triple = "x86_64-unknown-linux-gnu"
diff --git a/llvm/test/LTO/X86/codemodel-3.ll b/llvm/test/LTO/X86/codemodel-3.ll
index 947221e9f36dc5..13702dfbca2da4 100644
--- a/llvm/test/LTO/X86/codemodel-3.ll
+++ b/llvm/test/LTO/X86/codemodel-3.ll
@@ -1,6 +1,7 @@
 ; RUN: llvm-as %s -o %t0.o
 ; RUN: llvm-as < %p/Inputs/codemodel-3.ll > %t1.o
-; RUN: not llvm-lto2 run -r %t0.o,_start,px -r %t1.o,bar,px %t0.o %t1.o -o %t2.s 2>&1 | FileCheck %s 
+; RUN: not llvm-lto2 run -r %t0.o,_start,px -r %t1.o,bar,px -r %t0.o,_GLOBAL_OFFSET_TABLE_, \
+; RUN:   -r %t1.o,_GLOBAL_OFFSET_TABLE_, %t0.o %t1.o -o %t2.s 2>&1 | FileCheck %s 
 
 target triple = "x86_64-unknown-linux-gnu"
 target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
diff --git a/llvm/test/LTO/X86/largedatathreshold-1.ll b/llvm/test/LTO/X86/largedatathreshold-1.ll
index e3be5c11baaac2..dfd8319511b617 100644
--- a/llvm/test/LTO/X86/largedatathreshold-1.ll
+++ b/llvm/test/LTO/X86/largedatathreshold-1.ll
@@ -1,5 +1,5 @@
 ; RUN: llvm-as %s -o %t.o
-; RUN: llvm-lto2 run -r %t.o,_start,px %t.o -o %t.s
+; RUN: llvm-lto2 run -r %t.o,_start,px -r %t.o,_GLOBAL_OFFSET_TABLE_, %t.o -o %t.s
 ; RUN: llvm-objdump -d %t.s.0 | FileCheck %s
 
 target triple = "x86_64-unknown-linux-gnu"
diff --git a/llvm/test/LTO/X86/largedatathreshold-2.ll b/llvm/test/LTO/X86/largedatathreshold-2.ll
index 103c066b744d0f..59438bbdb5027f 100644
--- a/llvm/test/LTO/X86/largedatathreshold-2.ll
+++ b/llvm/test/LTO/X86/largedatathreshold-2.ll
@@ -1,5 +1,5 @@
 ; RUN: llvm-as %s -o %t.o
-; RUN: llvm-lto2 run -r %t.o,_start,px %t.o -o %t.s
+; RUN: llvm-lto2 run -r %t.o,_start,px -r %t.o,_GLOBAL_OFFSET_TABLE_, %t.o -o %t.s
 ; RUN: llvm-objdump -d %t.s.0 | FileCheck %s
 
 target triple = "x86_64-unknown-linux-gnu"
diff --git a/llvm/test/LTO/X86/largedatathreshold-3.ll b/llvm/test/LTO/X86/largedatathreshold-3.ll
index 3c0653db334d85..fea7987ff15566 100644
--- a/llvm/test/LTO/X86/largedatathreshold-3.ll
+++ b/llvm/test/LTO/X86/largedatathreshold-3.ll
@@ -1,6 +1,7 @@
 ; RUN: llvm-as %s -o %t0.o
 ; RUN: llvm-as < %p/Inputs/largedatathreshold.ll > %t1.o
-; RUN: not llvm-lto2 run -r %t0.o,_start,px -r %t1.o,bar,px %t0.o %t1.o -o %t2.s 2>&1 | FileCheck %s
+; RUN: not llvm-lto2 run -r %t0.o,_start,px -r %t1.o,bar,px -r %t0.o,_GLOBAL_OFFSET_TABLE_, \
+; RUN:   -r %t1.o,_GLOBAL_OFFSET_TABLE_, %t0.o %t1.o -o %t2.s 2>&1 | FileCheck %s
 
 ; CHECK: 'Large Data Threshold': IDs have conflicting values
 

@llvmbot
Copy link
Collaborator

llvmbot commented Apr 19, 2024

@llvm/pr-subscribers-lld

Author: Fangrui Song (MaskRay)

Changes

In ELF, relocatable files generated for x86-32 and some code models of
x86-64 (medium, large) may reference the special symbol
_GLOBAL_OFFSET_TABLE_ that is not used in the IR. In an LTO link, if
there is no regular relocatable file referencing the special symbol, the
linker may not define the symbol and lead to a spurious "undefined
symbol" error.

Fix #61101: record that _GLOBAL_OFFSET_TABLE_ is used in the IR symbol
table. The PreservedSymbols mechanism
(https://reviews.llvm.org/D112595) that just sets FB_used is not
applicable.


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

8 Files Affected:

  • (added) lld/test/ELF/lto/i386-global-offset-table.ll (+22)
  • (added) lld/test/ELF/lto/x86-64-global-offset-table.ll (+57)
  • (modified) llvm/lib/Object/ModuleSymbolTable.cpp (+14)
  • (modified) llvm/test/LTO/X86/codemodel-2.ll (+1-1)
  • (modified) llvm/test/LTO/X86/codemodel-3.ll (+2-1)
  • (modified) llvm/test/LTO/X86/largedatathreshold-1.ll (+1-1)
  • (modified) llvm/test/LTO/X86/largedatathreshold-2.ll (+1-1)
  • (modified) llvm/test/LTO/X86/largedatathreshold-3.ll (+2-1)
diff --git a/lld/test/ELF/lto/i386-global-offset-table.ll b/lld/test/ELF/lto/i386-global-offset-table.ll
new file mode 100644
index 00000000000000..e470763a3eb2ec
--- /dev/null
+++ b/lld/test/ELF/lto/i386-global-offset-table.ll
@@ -0,0 +1,22 @@
+; REQUIRES: x86
+; RUN: llvm-as %s -o %t.bc
+; RUN: ld.lld %t.bc -o %t
+
+target datalayout = "e-m:e-p:32:32-p270:32:32-p271:32:32-p272:64:64-i128:128-f64:32:64-f80:32-n8:16:32-S128"
+target triple = "i386-pc-linux-gnu"
+
+define dso_local void @f() {
+entry:
+  ret void
+}
+
+define dso_local void @_start() {
+entry:
+  call void @f()
+  ret void
+}
+
+!llvm.module.flags = !{!0, !1}
+
+!0 = !{i32 8, !"PIC Level", i32 2}
+!1 = !{i32 7, !"PIE Level", i32 2}
diff --git a/lld/test/ELF/lto/x86-64-global-offset-table.ll b/lld/test/ELF/lto/x86-64-global-offset-table.ll
new file mode 100644
index 00000000000000..1db1027e7a21c5
--- /dev/null
+++ b/lld/test/ELF/lto/x86-64-global-offset-table.ll
@@ -0,0 +1,57 @@
+; REQUIRES: x86
+; RUN: rm -rf %t && split-file %s %t && cd %t
+; RUN: llvm-mc -filetype=obj -triple=x86_64 b.s -o b.o
+
+; RUN: cat a.ll medium.ll | llvm-as - -o medium.bc
+; RUN: ld.lld -pie --no-relax medium.bc b.o -o medium
+; RUN: llvm-objdump -d medium | FileCheck %s
+
+; RUN: cat a.ll large.ll | llvm-as - -o large.bc
+; RUN: ld.lld -pie large.bc b.o -o large
+; RUN: llvm-objdump -d large | FileCheck %s
+
+; RUN: cat a.ll medium.ll ref.ll | llvm-as - -o ref.bc
+; RUN: ld.lld -pie --no-relax -u ref ref.bc b.o -o ref
+; RUN: llvm-objdump -d ref | FileCheck %s
+
+; CHECK: movabsq
+
+;--- a.ll
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-i128:128-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+@i = external global i32
+
+define dso_local void @_start() {
+entry:
+  %0 = load i32, ptr @i
+  %inc = add nsw i32 %0, 1
+  store i32 %inc, ptr @i
+  ret void
+}
+
+!llvm.module.flags = !{!0, !1, !2, !3}
+
+!0 = !{i32 8, !"PIC Level", i32 2}
+!1 = !{i32 7, !"PIE Level", i32 2}
+!2 = !{i32 1, !"Large Data Threshold", i64 0}
+
+;--- medium.ll
+!3 = !{i32 1, !"Code Model", i32 3}
+
+;--- large.ll
+!3 = !{i32 1, !"Code Model", i32 4}
+
+;--- ref.ll
+@_GLOBAL_OFFSET_TABLE_ = external global [0 x i8]
+
+define dso_local ptr @ref() {
+entry:
+  ret ptr @_GLOBAL_OFFSET_TABLE_
+}
+
+;--- b.s
+.data
+.globl i
+i:
+.long 0
diff --git a/llvm/lib/Object/ModuleSymbolTable.cpp b/llvm/lib/Object/ModuleSymbolTable.cpp
index 07f76688fa43e7..d8f520ad02c2f2 100644
--- a/llvm/lib/Object/ModuleSymbolTable.cpp
+++ b/llvm/lib/Object/ModuleSymbolTable.cpp
@@ -175,6 +175,20 @@ void ModuleSymbolTable::CollectAsmSymbols(
       AsmSymbol(Key, BasicSymbolRef::Flags(Res));
     }
   });
+
+  // In ELF, object code generated for x86-32 and some code models of x86-64 may
+  // reference the special symbol _GLOBAL_OFFSET_TABLE_ that is not used in the
+  // IR. Record it like inline asm symbols.
+  Triple TT(M.getTargetTriple());
+  if (!TT.isOSBinFormatELF() || !TT.isX86())
+    return;
+  auto CM = M.getCodeModel();
+  if (TT.getArch() == Triple::x86 || CM == CodeModel::Medium ||
+      CM == CodeModel::Large) {
+    AsmSymbol("_GLOBAL_OFFSET_TABLE_",
+              BasicSymbolRef::Flags(BasicSymbolRef::SF_Undefined |
+                                    BasicSymbolRef::SF_Global));
+  }
 }
 
 void ModuleSymbolTable::CollectAsmSymvers(
diff --git a/llvm/test/LTO/X86/codemodel-2.ll b/llvm/test/LTO/X86/codemodel-2.ll
index 5cd9731606f2bd..fc1074bcf2235c 100644
--- a/llvm/test/LTO/X86/codemodel-2.ll
+++ b/llvm/test/LTO/X86/codemodel-2.ll
@@ -1,5 +1,5 @@
 ; RUN: llvm-as %s -o %t.o
-; RUN: llvm-lto2 run -r %t.o,_start,px %t.o -o %t.s
+; RUN: llvm-lto2 run -r %t.o,_start,px -r %t.o,_GLOBAL_OFFSET_TABLE_, %t.o -o %t.s
 ; RUN: llvm-objdump --no-print-imm-hex -d %t.s.0 | FileCheck %s --check-prefix=CHECK-LARGE
 
 target triple = "x86_64-unknown-linux-gnu"
diff --git a/llvm/test/LTO/X86/codemodel-3.ll b/llvm/test/LTO/X86/codemodel-3.ll
index 947221e9f36dc5..13702dfbca2da4 100644
--- a/llvm/test/LTO/X86/codemodel-3.ll
+++ b/llvm/test/LTO/X86/codemodel-3.ll
@@ -1,6 +1,7 @@
 ; RUN: llvm-as %s -o %t0.o
 ; RUN: llvm-as < %p/Inputs/codemodel-3.ll > %t1.o
-; RUN: not llvm-lto2 run -r %t0.o,_start,px -r %t1.o,bar,px %t0.o %t1.o -o %t2.s 2>&1 | FileCheck %s 
+; RUN: not llvm-lto2 run -r %t0.o,_start,px -r %t1.o,bar,px -r %t0.o,_GLOBAL_OFFSET_TABLE_, \
+; RUN:   -r %t1.o,_GLOBAL_OFFSET_TABLE_, %t0.o %t1.o -o %t2.s 2>&1 | FileCheck %s 
 
 target triple = "x86_64-unknown-linux-gnu"
 target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
diff --git a/llvm/test/LTO/X86/largedatathreshold-1.ll b/llvm/test/LTO/X86/largedatathreshold-1.ll
index e3be5c11baaac2..dfd8319511b617 100644
--- a/llvm/test/LTO/X86/largedatathreshold-1.ll
+++ b/llvm/test/LTO/X86/largedatathreshold-1.ll
@@ -1,5 +1,5 @@
 ; RUN: llvm-as %s -o %t.o
-; RUN: llvm-lto2 run -r %t.o,_start,px %t.o -o %t.s
+; RUN: llvm-lto2 run -r %t.o,_start,px -r %t.o,_GLOBAL_OFFSET_TABLE_, %t.o -o %t.s
 ; RUN: llvm-objdump -d %t.s.0 | FileCheck %s
 
 target triple = "x86_64-unknown-linux-gnu"
diff --git a/llvm/test/LTO/X86/largedatathreshold-2.ll b/llvm/test/LTO/X86/largedatathreshold-2.ll
index 103c066b744d0f..59438bbdb5027f 100644
--- a/llvm/test/LTO/X86/largedatathreshold-2.ll
+++ b/llvm/test/LTO/X86/largedatathreshold-2.ll
@@ -1,5 +1,5 @@
 ; RUN: llvm-as %s -o %t.o
-; RUN: llvm-lto2 run -r %t.o,_start,px %t.o -o %t.s
+; RUN: llvm-lto2 run -r %t.o,_start,px -r %t.o,_GLOBAL_OFFSET_TABLE_, %t.o -o %t.s
 ; RUN: llvm-objdump -d %t.s.0 | FileCheck %s
 
 target triple = "x86_64-unknown-linux-gnu"
diff --git a/llvm/test/LTO/X86/largedatathreshold-3.ll b/llvm/test/LTO/X86/largedatathreshold-3.ll
index 3c0653db334d85..fea7987ff15566 100644
--- a/llvm/test/LTO/X86/largedatathreshold-3.ll
+++ b/llvm/test/LTO/X86/largedatathreshold-3.ll
@@ -1,6 +1,7 @@
 ; RUN: llvm-as %s -o %t0.o
 ; RUN: llvm-as < %p/Inputs/largedatathreshold.ll > %t1.o
-; RUN: not llvm-lto2 run -r %t0.o,_start,px -r %t1.o,bar,px %t0.o %t1.o -o %t2.s 2>&1 | FileCheck %s
+; RUN: not llvm-lto2 run -r %t0.o,_start,px -r %t1.o,bar,px -r %t0.o,_GLOBAL_OFFSET_TABLE_, \
+; RUN:   -r %t1.o,_GLOBAL_OFFSET_TABLE_, %t0.o %t1.o -o %t2.s 2>&1 | FileCheck %s
 
 ; CHECK: 'Large Data Threshold': IDs have conflicting values
 

@llvmbot
Copy link
Collaborator

llvmbot commented Apr 19, 2024

@llvm/pr-subscribers-lto

Author: Fangrui Song (MaskRay)

Changes

In ELF, relocatable files generated for x86-32 and some code models of
x86-64 (medium, large) may reference the special symbol
_GLOBAL_OFFSET_TABLE_ that is not used in the IR. In an LTO link, if
there is no regular relocatable file referencing the special symbol, the
linker may not define the symbol and lead to a spurious "undefined
symbol" error.

Fix #61101: record that _GLOBAL_OFFSET_TABLE_ is used in the IR symbol
table. The PreservedSymbols mechanism
(https://reviews.llvm.org/D112595) that just sets FB_used is not
applicable.


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

8 Files Affected:

  • (added) lld/test/ELF/lto/i386-global-offset-table.ll (+22)
  • (added) lld/test/ELF/lto/x86-64-global-offset-table.ll (+57)
  • (modified) llvm/lib/Object/ModuleSymbolTable.cpp (+14)
  • (modified) llvm/test/LTO/X86/codemodel-2.ll (+1-1)
  • (modified) llvm/test/LTO/X86/codemodel-3.ll (+2-1)
  • (modified) llvm/test/LTO/X86/largedatathreshold-1.ll (+1-1)
  • (modified) llvm/test/LTO/X86/largedatathreshold-2.ll (+1-1)
  • (modified) llvm/test/LTO/X86/largedatathreshold-3.ll (+2-1)
diff --git a/lld/test/ELF/lto/i386-global-offset-table.ll b/lld/test/ELF/lto/i386-global-offset-table.ll
new file mode 100644
index 00000000000000..e470763a3eb2ec
--- /dev/null
+++ b/lld/test/ELF/lto/i386-global-offset-table.ll
@@ -0,0 +1,22 @@
+; REQUIRES: x86
+; RUN: llvm-as %s -o %t.bc
+; RUN: ld.lld %t.bc -o %t
+
+target datalayout = "e-m:e-p:32:32-p270:32:32-p271:32:32-p272:64:64-i128:128-f64:32:64-f80:32-n8:16:32-S128"
+target triple = "i386-pc-linux-gnu"
+
+define dso_local void @f() {
+entry:
+  ret void
+}
+
+define dso_local void @_start() {
+entry:
+  call void @f()
+  ret void
+}
+
+!llvm.module.flags = !{!0, !1}
+
+!0 = !{i32 8, !"PIC Level", i32 2}
+!1 = !{i32 7, !"PIE Level", i32 2}
diff --git a/lld/test/ELF/lto/x86-64-global-offset-table.ll b/lld/test/ELF/lto/x86-64-global-offset-table.ll
new file mode 100644
index 00000000000000..1db1027e7a21c5
--- /dev/null
+++ b/lld/test/ELF/lto/x86-64-global-offset-table.ll
@@ -0,0 +1,57 @@
+; REQUIRES: x86
+; RUN: rm -rf %t && split-file %s %t && cd %t
+; RUN: llvm-mc -filetype=obj -triple=x86_64 b.s -o b.o
+
+; RUN: cat a.ll medium.ll | llvm-as - -o medium.bc
+; RUN: ld.lld -pie --no-relax medium.bc b.o -o medium
+; RUN: llvm-objdump -d medium | FileCheck %s
+
+; RUN: cat a.ll large.ll | llvm-as - -o large.bc
+; RUN: ld.lld -pie large.bc b.o -o large
+; RUN: llvm-objdump -d large | FileCheck %s
+
+; RUN: cat a.ll medium.ll ref.ll | llvm-as - -o ref.bc
+; RUN: ld.lld -pie --no-relax -u ref ref.bc b.o -o ref
+; RUN: llvm-objdump -d ref | FileCheck %s
+
+; CHECK: movabsq
+
+;--- a.ll
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-i128:128-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+@i = external global i32
+
+define dso_local void @_start() {
+entry:
+  %0 = load i32, ptr @i
+  %inc = add nsw i32 %0, 1
+  store i32 %inc, ptr @i
+  ret void
+}
+
+!llvm.module.flags = !{!0, !1, !2, !3}
+
+!0 = !{i32 8, !"PIC Level", i32 2}
+!1 = !{i32 7, !"PIE Level", i32 2}
+!2 = !{i32 1, !"Large Data Threshold", i64 0}
+
+;--- medium.ll
+!3 = !{i32 1, !"Code Model", i32 3}
+
+;--- large.ll
+!3 = !{i32 1, !"Code Model", i32 4}
+
+;--- ref.ll
+@_GLOBAL_OFFSET_TABLE_ = external global [0 x i8]
+
+define dso_local ptr @ref() {
+entry:
+  ret ptr @_GLOBAL_OFFSET_TABLE_
+}
+
+;--- b.s
+.data
+.globl i
+i:
+.long 0
diff --git a/llvm/lib/Object/ModuleSymbolTable.cpp b/llvm/lib/Object/ModuleSymbolTable.cpp
index 07f76688fa43e7..d8f520ad02c2f2 100644
--- a/llvm/lib/Object/ModuleSymbolTable.cpp
+++ b/llvm/lib/Object/ModuleSymbolTable.cpp
@@ -175,6 +175,20 @@ void ModuleSymbolTable::CollectAsmSymbols(
       AsmSymbol(Key, BasicSymbolRef::Flags(Res));
     }
   });
+
+  // In ELF, object code generated for x86-32 and some code models of x86-64 may
+  // reference the special symbol _GLOBAL_OFFSET_TABLE_ that is not used in the
+  // IR. Record it like inline asm symbols.
+  Triple TT(M.getTargetTriple());
+  if (!TT.isOSBinFormatELF() || !TT.isX86())
+    return;
+  auto CM = M.getCodeModel();
+  if (TT.getArch() == Triple::x86 || CM == CodeModel::Medium ||
+      CM == CodeModel::Large) {
+    AsmSymbol("_GLOBAL_OFFSET_TABLE_",
+              BasicSymbolRef::Flags(BasicSymbolRef::SF_Undefined |
+                                    BasicSymbolRef::SF_Global));
+  }
 }
 
 void ModuleSymbolTable::CollectAsmSymvers(
diff --git a/llvm/test/LTO/X86/codemodel-2.ll b/llvm/test/LTO/X86/codemodel-2.ll
index 5cd9731606f2bd..fc1074bcf2235c 100644
--- a/llvm/test/LTO/X86/codemodel-2.ll
+++ b/llvm/test/LTO/X86/codemodel-2.ll
@@ -1,5 +1,5 @@
 ; RUN: llvm-as %s -o %t.o
-; RUN: llvm-lto2 run -r %t.o,_start,px %t.o -o %t.s
+; RUN: llvm-lto2 run -r %t.o,_start,px -r %t.o,_GLOBAL_OFFSET_TABLE_, %t.o -o %t.s
 ; RUN: llvm-objdump --no-print-imm-hex -d %t.s.0 | FileCheck %s --check-prefix=CHECK-LARGE
 
 target triple = "x86_64-unknown-linux-gnu"
diff --git a/llvm/test/LTO/X86/codemodel-3.ll b/llvm/test/LTO/X86/codemodel-3.ll
index 947221e9f36dc5..13702dfbca2da4 100644
--- a/llvm/test/LTO/X86/codemodel-3.ll
+++ b/llvm/test/LTO/X86/codemodel-3.ll
@@ -1,6 +1,7 @@
 ; RUN: llvm-as %s -o %t0.o
 ; RUN: llvm-as < %p/Inputs/codemodel-3.ll > %t1.o
-; RUN: not llvm-lto2 run -r %t0.o,_start,px -r %t1.o,bar,px %t0.o %t1.o -o %t2.s 2>&1 | FileCheck %s 
+; RUN: not llvm-lto2 run -r %t0.o,_start,px -r %t1.o,bar,px -r %t0.o,_GLOBAL_OFFSET_TABLE_, \
+; RUN:   -r %t1.o,_GLOBAL_OFFSET_TABLE_, %t0.o %t1.o -o %t2.s 2>&1 | FileCheck %s 
 
 target triple = "x86_64-unknown-linux-gnu"
 target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
diff --git a/llvm/test/LTO/X86/largedatathreshold-1.ll b/llvm/test/LTO/X86/largedatathreshold-1.ll
index e3be5c11baaac2..dfd8319511b617 100644
--- a/llvm/test/LTO/X86/largedatathreshold-1.ll
+++ b/llvm/test/LTO/X86/largedatathreshold-1.ll
@@ -1,5 +1,5 @@
 ; RUN: llvm-as %s -o %t.o
-; RUN: llvm-lto2 run -r %t.o,_start,px %t.o -o %t.s
+; RUN: llvm-lto2 run -r %t.o,_start,px -r %t.o,_GLOBAL_OFFSET_TABLE_, %t.o -o %t.s
 ; RUN: llvm-objdump -d %t.s.0 | FileCheck %s
 
 target triple = "x86_64-unknown-linux-gnu"
diff --git a/llvm/test/LTO/X86/largedatathreshold-2.ll b/llvm/test/LTO/X86/largedatathreshold-2.ll
index 103c066b744d0f..59438bbdb5027f 100644
--- a/llvm/test/LTO/X86/largedatathreshold-2.ll
+++ b/llvm/test/LTO/X86/largedatathreshold-2.ll
@@ -1,5 +1,5 @@
 ; RUN: llvm-as %s -o %t.o
-; RUN: llvm-lto2 run -r %t.o,_start,px %t.o -o %t.s
+; RUN: llvm-lto2 run -r %t.o,_start,px -r %t.o,_GLOBAL_OFFSET_TABLE_, %t.o -o %t.s
 ; RUN: llvm-objdump -d %t.s.0 | FileCheck %s
 
 target triple = "x86_64-unknown-linux-gnu"
diff --git a/llvm/test/LTO/X86/largedatathreshold-3.ll b/llvm/test/LTO/X86/largedatathreshold-3.ll
index 3c0653db334d85..fea7987ff15566 100644
--- a/llvm/test/LTO/X86/largedatathreshold-3.ll
+++ b/llvm/test/LTO/X86/largedatathreshold-3.ll
@@ -1,6 +1,7 @@
 ; RUN: llvm-as %s -o %t0.o
 ; RUN: llvm-as < %p/Inputs/largedatathreshold.ll > %t1.o
-; RUN: not llvm-lto2 run -r %t0.o,_start,px -r %t1.o,bar,px %t0.o %t1.o -o %t2.s 2>&1 | FileCheck %s
+; RUN: not llvm-lto2 run -r %t0.o,_start,px -r %t1.o,bar,px -r %t0.o,_GLOBAL_OFFSET_TABLE_, \
+; RUN:   -r %t1.o,_GLOBAL_OFFSET_TABLE_, %t0.o %t1.o -o %t2.s 2>&1 | FileCheck %s
 
 ; CHECK: 'Large Data Threshold': IDs have conflicting values
 

Copy link
Contributor

@teresajohnson teresajohnson left a comment

Choose a reason for hiding this comment

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

A few questions about tests below. Also, ModuleSymbolTable::CollectAsmSymbols is called when building the ModuleSummaryIndex for ThinLTO - I don't see any ThinLTO tests modified or added here - do we need one to make sure nothing surprising happens in that mode?

@@ -0,0 +1,22 @@
; REQUIRES: x86
Copy link
Contributor

Choose a reason for hiding this comment

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

What is this test testing?

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for the review. Added a file-level comment.

; RUN: ld.lld -pie --no-relax -u ref ref.bc b.o -o ref
; RUN: llvm-objdump -d ref | FileCheck %s

; CHECK: movabsq
Copy link
Contributor

Choose a reason for hiding this comment

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

What is this checking for related to the GLOBAL_OFFSET_TABLE? Can you add a comment?

Copy link
Member Author

Choose a reason for hiding this comment

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

Added The IR symbol table references _GLOBAL_OFFSET_TABLE_, which causes lld to define the symbol.

@@ -1,5 +1,5 @@
; RUN: llvm-as %s -o %t.o
; RUN: llvm-lto2 run -r %t.o,_start,px %t.o -o %t.s
; RUN: llvm-lto2 run -r %t.o,_start,px -r %t.o,_GLOBAL_OFFSET_TABLE_, %t.o -o %t.s
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is it needed here and in other tests now?

Copy link
Member Author

Choose a reason for hiding this comment

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

For x86-64 medium/large code model, the IR symbol table now references _GLOBAL_OFFSET_TABLE_, which requires llvm-lto2 to provide it to avoid a missing symbol resolution for error

@aeubanks
Copy link
Contributor

I was trying to understand why we couldn't resolve _GLOBAL_OFFSET_TABLE_ after LTO in lld. Ideally we wouldn't special case linker-synthesized symbols in LLVM?

Created using spr 1.3.5-bogner
@MaskRay
Copy link
Member Author

MaskRay commented Apr 19, 2024

I was trying to understand why we couldn't resolve _GLOBAL_OFFSET_TABLE_ after LTO in lld. Ideally we wouldn't special case linker-synthesized symbols in LLVM?

lld only defines _GLOBAL_OFFSET_TABLE_ when it is referenced. The symbol table should be finalized before LTO.

Codegen passes may reference a runtime library function or linker-defined symbols _GLOBAL_OFFSET_TABLE_ that is not in the referencer's symbol table. In such a case, if we don't "workaround" it, we will get an "undefined symbol" error.
This behavior is not lld specific and applies to other linkers). I feel that LTO is the right place to implement the special logic. If we have other symbols that are not ELF-specific, a linker-oriented solution would require to patch every linker.

@aeubanks
Copy link
Contributor

Linker-defined symbols seem different from runtime library symbols.

Runtime library symbols have to be provided by some input object file. We can either do something similar to this PR and mark all LTO bitcode files as requiring any potential runtime library symbol (there are a lot, so it might take up noticeable space in the bitcode files), or have the linker special case them (which is what I believe lld does).

But for linker-defined symbols, it seems like it's a fixable phase ordering issue by defining them after parsing object files created by LTO. Then we don't have to start special casing any of these types of symbols. Perhaps I'm missing something though.

@MaskRay
Copy link
Member Author

MaskRay commented Apr 22, 2024

I think this patch is the right solution.

We want the symbol table to be complete before compileBitcodeFiles<ELFT>(skipLinkedOutput);. In the false positive cases, _GLOBAL_OFFSET_TABLE_ was not in the symbol table, so lld did not know the symbol would later be used by generated relocatable files. This cannot be fixed by reordering some passes in the linker.

This could be solved by adding a pass: making the linker try probing _GLOBAL_OFFSET_TABLE_ after LTO compilation, but it doesn't feel right to me...

For runtime library symbols, ideally we the solution should be in LLVMLTO instead of linkers. However, given that we have to be very conservative, that'd cause a lot of overhead in bitcode files, so we accepted to workaround in linkers.

@aeubanks
Copy link
Contributor

ah, so you're saying that LTO bitcode files should be more like a normal object file and declare everything in their symbol table up front before actually creating object files. this is so that we can follow ELF linking logic to determine which subset of LTO bitcode files we want to link (after LTO-ing them and creating an object file)? I was missing the part where we performed normal ELF linking logic on bitcode files as well. although linker-created symbols don't really participate in this the same way normal symbols do...

it still seems a weird to me that we can't move the call to addReservedSymbols() after compileBitcodeFiles<ELFT>(skipLinkedOutput) and that we have to force all input bitcode to declare dependencies on any potentially created linker-defined symbols. this forces LTO (which is supposed to be middle-end-y) to know about codegen details which isn't super nice.

I think this is a similar but reversed argument to your argument about the linker needing to know about runtime library calls, which I agree ideally would be in LTO bitcode files. the difference is that for runtime library calls, adding all potentially created runtime calls to the bitcode symbol table is expensive. but changing the order in which we call addReservedSymbols() and compileBitcodeFiles seems doable(?)

thoughts?

@aeubanks
Copy link
Contributor

(fwiw I think this change is an acceptable alternative, especially if the number of linker-defined symbols across all architectures that we need to special case is small)

@pcc
Copy link
Contributor

pcc commented Apr 23, 2024

The intent was always that LTO would tell the linker which symbols may be used by the combined object file, and that it wouldn't be handled by the individual bitcode files. See LTO::getRuntimeLibcallSymbols() which is where the list currently comes from (it currently refers to "library functions" but I don't see why it should be limited to that). So I am not sure that _GLOBAL_OFFSET_TABLE_ should be a special case here.

@pcc
Copy link
Contributor

pcc commented Apr 23, 2024

the difference is that for runtime library calls, adding all potentially created runtime calls to the bitcode symbol table is expensive.

More importantly, there would be no purpose in embedding the list in every object file. The bitcode symbol table is like a cache for the information in the rest of the bitcode file, and the behavior of the LLVM backend linked into lld is what ultimately matters for LTO because that's what generates library calls. Since the list is the same for every bitcode file, and it would go out of date if producer version != consumer version, the "cache" in the individual bitcode files would serve no purpose because LTO::getRuntimeLibcallSymbols() would always provide you with the correct symbol list.

@MaskRay
Copy link
Member Author

MaskRay commented Apr 23, 2024

The intent was always that LTO would tell the linker which symbols may be used by the combined object file, and that it wouldn't be handled by the individual bitcode files. See LTO::getRuntimeLibcallSymbols() which is where the list currently comes from (it currently refers to "library functions" but I don't see why it should be limited to that). So I am not sure that _GLOBAL_OFFSET_TABLE_ should be a special case here.

The commit message states:

The PreservedSymbols mechanism
(reviews.llvm.org/D112595) that just sets FB_used is not
applicable.

That mechanism and LTO::getRuntimeLibcallSymbols() handle lazy symbols that appear in the symbol table before LTO compilation. _GLOBAL_OFFSET_TABLE_ is a special case that it may not be in the symbol table when LTO compilation occurs.

Technically linkers can add the special case to handleLibcall by ensuring _GLOBAL_OFFSET_TABLE_ is always present (thigh might only be needed for x86). Personally I feel that LLVMLTO, which is equipped with more information, is the best place to implement this to benefit all linkers (GNU ld might always define _GLOBAL_OFFSET_TABLE_ when dynamic sections are needed).

@pcc
Copy link
Contributor

pcc commented Apr 23, 2024

That mechanism and LTO::getRuntimeLibcallSymbols() handle lazy symbols that appear in the symbol table before LTO compilation. _GLOBAL_OFFSET_TABLE_ is a special case that it may not be in the symbol table when LTO compilation occurs.

Isn't that just an implementation detail of lld? Basically, the behavior of handleLibcall should be as if we loaded an object file with an undefined symbol with that name. Currently we only implement the archive loading aspect of that behavior and only if the target is a bitcode file as a workaround for the libgcc issue, but maybe what we should really be doing is actually creating the undefined symbols via addSymbol(Undefined{...}) (unless there's a non-bitcode archive member to keep working around the libgcc issue)?

Technically linkers can add the special case to handleLibcall by ensuring _GLOBAL_OFFSET_TABLE_ is always present (thigh might only be needed for x86). Personally I feel that LLVMLTO, which is equipped with more information, is the best place to implement this to benefit all linkers (GNU ld might always define _GLOBAL_OFFSET_TABLE_ when dynamic sections are needed).

Right, LTO (on behalf of the LLVM backend) should be the source of truth of this information, and it can be accessed via LTO::getRuntimeLibcallSymbols(). It doesn't follow that this information needs to be in every bitcode symbol table.

@MaskRay
Copy link
Member Author

MaskRay commented Apr 23, 2024

That mechanism and LTO::getRuntimeLibcallSymbols() handle lazy symbols that appear in the symbol table before LTO compilation. _GLOBAL_OFFSET_TABLE_ is a special case that it may not be in the symbol table when LTO compilation occurs.

Isn't that just an implementation detail of lld? Basically, the behavior of handleLibcall should be as if we loaded an object file with an undefined symbol with that name. Currently we only implement the archive loading aspect of that behavior and only if the target is a bitcode file as a workaround for the libgcc issue, but maybe what we should really be doing is actually creating the undefined symbols via addSymbol(Undefined{...}) (unless there's a non-bitcode archive member to keep working around the libgcc issue)?

I think it's related to an implementation detail of lld and other linkers, but more related to the LTO symbol resolution scheme. The IR symbol table should strive to provide a full picture. In this case, it knows the architecture and the code model and can make a best guess of whether _GLOBAL_OFFSET_TABLE_ will be needed.

Technically linkers can add the special case to handleLibcall by ensuring _GLOBAL_OFFSET_TABLE_ is always present (thigh might only be needed for x86). Personally I feel that LLVMLTO, which is equipped with more information, is the best place to implement this to benefit all linkers (GNU ld might always define _GLOBAL_OFFSET_TABLE_ when dynamic sections are needed).

Right, LTO (on behalf of the LLVM backend) should be the source of truth of this information, and it can be accessed via LTO::getRuntimeLibcallSymbols(). It doesn't follow that this information needs to be in every bitcode symbol table.

I hope that the linker only defines _GLOBAL_OFFSET_TABLE_ when it is really used.
At the LTO::getRuntimeLibcallSymbols() call site there is no information about code models to make a good decision.

The IRSymTab code will likely be extended to be smarter on function-local inline asm symbols (beside current model-level inline asm symbols).
Adding _GLOBAL_OFFSET_TABLE_ there feel natural to me.

@pcc
Copy link
Contributor

pcc commented Apr 23, 2024

Okay, that makes sense. LTO::getRuntimeLibcallSymbols() should be used for symbols that are "always" potentially used. We could extend that notion to "always potentially used on a given architecture" by passing in an architecture (which the linker knows), but the linker doesn't know the code model. So adding it to the bitcode symbol table seems fine to me.

@@ -0,0 +1,71 @@
; REQUIRES: x86
;; LTO-generated felocatable files may reference _GLOBAL_OFFSET_TABLE_ while
Copy link
Collaborator

Choose a reason for hiding this comment

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

s/felocat/relocat/

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks. Fixed

Created using spr 1.3.5-bogner
Copy link
Contributor

@teresajohnson teresajohnson left a comment

Choose a reason for hiding this comment

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

lgtm with one comment fix below.

@@ -1,18 +1,25 @@
; REQUIRES: x86
; RUN: llvm-as %s -o %t.bc
; RUN: ld.lld %t.bc -o %t
;; LTO-generated felocatable files may reference _GLOBAL_OFFSET_TABLE_ while
Copy link
Contributor

Choose a reason for hiding this comment

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

relocatable

Created using spr 1.3.5-bogner
@MaskRay MaskRay merged commit 99e7350 into main Apr 24, 2024
3 of 4 checks passed
@MaskRay MaskRay deleted the users/MaskRay/spr/irsymtab-record-_global_offset_table_-for-elf-x86 branch April 24, 2024 00:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lld:ELF lld llvm:binary-utilities LTO Link time optimization (regular/full LTO or ThinLTO)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

lld error "undefined symbol: _GLOBAL_OFFSET_TABLE_ " with LTO
6 participants