Error instead of producing broken binary

[• espindola]

Summary

This "fixes" PR36678 by just producing an error when we find a case where we would produce an plt entry that used ebx but ebx would not be set.

[grimar]

ELF/Arch/X86.cpp

68 ↗

(On Diff #138128)

a library

70 ↗

(On Diff #138128)

Comment saying "for PIE", but Config->Pic also covers DSO, we have Config->Pie for PIE. Is this if correct?
We will report "cannot be used against symbol" for -shared case, so I think it can be if (Config->Pie).

[• rafael]

George Rimar via Phabricator via llvm-commits
<[e-mail]> writes:

grimar added inline comments.

Comment at: ELF/Arch/X86.cpp:68
+ compiled without -fPIE/-fPIC and doesn't maintain ebx.
+ * If a libray definition gets preempted to the executable, it will have the

+ // wrong ebx value.

a library

Thanks.

Comment at: ELF/Arch/X86.cpp:70
+ // wrong ebx value.
+ if (Config->Pic)

+ SupportsPltPreemption = false;

Comment saying "for PIE", but Config->Pic also covers DSO, we have Config->Pie for PIE. Is this if correct?
We will report "cannot be used against symbol" for -shared case, so I think it can be if (Config->Pie).

They would be equivalent as the canonical plt hack never works for
shared libraries, but using Pie is definitely better.

I will upload a new version.

Thanks,
Rafael

[ruiu]

ELF/Relocations.cpp

878

I wouldn't generalize this as a Target member because I believe you need it only for i386. Even if there are two or more targets that need this, you could generalize when you add another arch.

879–880

Please add a hint to the error message about how to fix the problem. You could fix this by recompiling with -fPIC, right?

[• espindola]

Simplify.

[ruiu]

LGTM

[• espindola]

r327542

ELF/Relocations.cpp

if (Sym.isFunc()) {
     // need the help of the dynamic linker for that. We let it know that we have
     // a direct reference to a so symbol by creating an undefined symbol with a
     // non zero st_value. Seeing that, the dynamic linker resolves the symbol to
     // the value of the symbol we created. This is true even for got entries, so
     // pointer equality is maintained. To avoid an infinite loop, the only entry
     // that points to the real function is a dedicated got entry used by the
     // plt. That is identified by special relocation types (R_X86_64_JUMP_SLOT,
     // R_386_JMP_SLOT, etc).
+
+    // For position independent executable on i386, the plt entry requires ebx
+    // to be set. This causes two problems:
+    // * If some code has a direct reference to a function, it was probably
+    //   compiled without -fPIE/-fPIC and doesn't maintain ebx.
+    // * If a library definition gets preempted to the executable, it will have
+    //   the wrong ebx value.
+    if (Config->Pie && Config->EMachine == EM_386)
+      errorOrWarn("symbol '" + toString(Sym) +
+                  "' cannot be preempted; recompile with -fPIE" +
+                  getLocation(Sec, Sym, Offset));
     Sym.NeedsPltAddr = true;
     Expr = toPlt(Expr);
     Sec.Relocations.push_back({Expr, Type, Offset, Addend, &Sym});
     return Expr;
   }
 
   errorOrWarn("symbol '" + toString(Sym) + "' has no type" +
               getLocation(Sec, Sym, Offset));

test/ELF/Inputs/i386-pic-plt.s

+        .global foo
+        .type foo, @function
+foo:
+        nop

test/ELF/i386-pic-plt.s

+// REQUIRES: x86
+// RUN: llvm-mc -filetype=obj -triple=i386-pc-linux %s -o %t.o
+// RUN: llvm-mc -filetype=obj -triple=i386-pc-linux %p/Inputs/i386-pic-plt.s -o %t2.o
+// RUN: ld.lld -shared %t2.o -o %t2.so
+// RUN: ld.lld %t.o %t2.so -o %t
+// RUN: not ld.lld %t.o %t2.so -o %t -pie 2>&1 | FileCheck %s
+
+// CHECK: error: symbol 'foo' cannot be preempted; recompile with -fPIE
+
+.global _start
+_start:
+  call foo