Skip to content

Commit

Permalink
[ELF] Create canonical PLTs only when needed
Browse files Browse the repository at this point in the history
Previously, we make all PLT entries canonical if we are creating
a position dependent executable, because I was thinking that promoting
usual PLT entries to canonical ones is harmless; symbol equality still
holds.

However, it looks like Qt depends on the usual linker's behavior not
to make PLT canonical if not necessary. I believe they are maintaining
some hidden symbol as aliases for exported symbols and compare their
addresses at runtime.

Of course this doesn't work if your program is not compiled with -fPIC,
but qt5/QtCore/qglobal.h has a macro to abort compilation if PIC is
disabled. (They check for __PIC__ and __PIE__ macros.) So, all object
files are guaranteed to be compiled with -fPIC if they are using QT
functions, and they assume that the linker doesn't create a canonical
PLT for Qt functions.

This commit makes mold to create canonical PLTs only when needed.
That is, if an address of a function is directly taken (i.e. not via
GOT), then mold makes its PLT canonical.

Fixes #352
  • Loading branch information
rui314 committed Apr 5, 2022
1 parent 3768ca6 commit e0bc74a
Show file tree
Hide file tree
Showing 10 changed files with 82 additions and 26 deletions.
2 changes: 1 addition & 1 deletion elf/arch-arm32.cc
Original file line number Diff line number Diff line change
Expand Up @@ -389,7 +389,7 @@ void InputSection<E>::scan_relocations(Context<E> &ctx) {
// Absolute Local Imported data Imported code
{ NONE, BASEREL, DYNREL, DYNREL }, // DSO
{ NONE, BASEREL, DYNREL, DYNREL }, // PIE
{ NONE, NONE, COPYREL, PLT }, // PDE
{ NONE, NONE, COPYREL, CPLT }, // PDE
};
dispatch(ctx, table, i, rel, sym);
break;
Expand Down
6 changes: 3 additions & 3 deletions elf/arch-arm64.cc
Original file line number Diff line number Diff line change
Expand Up @@ -416,7 +416,7 @@ void InputSection<E>::scan_relocations(Context<E> &ctx) {
// Absolute Local Imported data Imported code
{ NONE, BASEREL, DYNREL, DYNREL }, // DSO
{ NONE, BASEREL, DYNREL, DYNREL }, // PIE
{ NONE, NONE, COPYREL, PLT }, // PDE
{ NONE, NONE, COPYREL, CPLT }, // PDE
};
dispatch(ctx, table, i, rel, sym);
break;
Expand All @@ -439,8 +439,8 @@ void InputSection<E>::scan_relocations(Context<E> &ctx) {
Action table[][4] = {
// Absolute Local Imported data Imported code
{ ERROR, NONE, ERROR, ERROR }, // DSO
{ ERROR, NONE, COPYREL, PLT }, // PIE
{ NONE, NONE, COPYREL, PLT }, // PDE
{ ERROR, NONE, COPYREL, CPLT }, // PIE
{ NONE, NONE, COPYREL, CPLT }, // PDE
};
dispatch(ctx, table, i, rel, sym);
break;
Expand Down
4 changes: 2 additions & 2 deletions elf/arch-i386.cc
Original file line number Diff line number Diff line change
Expand Up @@ -449,7 +449,7 @@ void InputSection<E>::scan_relocations(Context<E> &ctx) {
// Absolute Local Imported data Imported code
{ NONE, ERROR, ERROR, ERROR }, // DSO
{ NONE, ERROR, ERROR, ERROR }, // PIE
{ NONE, NONE, COPYREL, PLT }, // PDE
{ NONE, NONE, COPYREL, CPLT }, // PDE
};
dispatch(ctx, table, i, rel, sym);
break;
Expand All @@ -459,7 +459,7 @@ void InputSection<E>::scan_relocations(Context<E> &ctx) {
// Absolute Local Imported data Imported code
{ NONE, BASEREL, DYNREL, DYNREL }, // DSO
{ NONE, BASEREL, DYNREL, DYNREL }, // PIE
{ NONE, NONE, COPYREL, PLT }, // PDE
{ NONE, NONE, COPYREL, CPLT }, // PDE
};
dispatch(ctx, table, i, rel, sym);
break;
Expand Down
4 changes: 2 additions & 2 deletions elf/arch-riscv64.cc
Original file line number Diff line number Diff line change
Expand Up @@ -556,7 +556,7 @@ void InputSection<E>::scan_relocations(Context<E> &ctx) {
// Absolute Local Imported data Imported code
{ NONE, ERROR, ERROR, ERROR }, // DSO
{ NONE, ERROR, ERROR, ERROR }, // PIE
{ NONE, NONE, COPYREL, PLT }, // PDE
{ NONE, NONE, COPYREL, CPLT }, // PDE
};
dispatch(ctx, table, i, rel, sym);
break;
Expand All @@ -566,7 +566,7 @@ void InputSection<E>::scan_relocations(Context<E> &ctx) {
// Absolute Local Imported data Imported code
{ NONE, BASEREL, DYNREL, DYNREL }, // DSO
{ NONE, BASEREL, DYNREL, DYNREL }, // PIE
{ NONE, NONE, COPYREL, PLT }, // PDE
{ NONE, NONE, COPYREL, CPLT }, // PDE
};
dispatch(ctx, table, i, rel, sym);
break;
Expand Down
4 changes: 2 additions & 2 deletions elf/arch-x86-64.cc
Original file line number Diff line number Diff line change
Expand Up @@ -654,7 +654,7 @@ void InputSection<E>::scan_relocations(Context<E> &ctx) {
// Absolute Local Imported data Imported code
{ NONE, ERROR, ERROR, ERROR }, // DSO
{ NONE, ERROR, ERROR, ERROR }, // PIE
{ NONE, NONE, COPYREL, PLT }, // PDE
{ NONE, NONE, COPYREL, CPLT }, // PDE
};
dispatch(ctx, table, i, rel, sym);
break;
Expand All @@ -666,7 +666,7 @@ void InputSection<E>::scan_relocations(Context<E> &ctx) {
// Absolute Local Imported data Imported code
{ NONE, BASEREL, DYNREL, DYNREL }, // DSO
{ NONE, BASEREL, DYNREL, DYNREL }, // PIE
{ NONE, NONE, COPYREL, PLT }, // PDE
{ NONE, NONE, COPYREL, CPLT }, // PDE
};
dispatch(ctx, table, i, rel, sym);
break;
Expand Down
6 changes: 6 additions & 0 deletions elf/input-sections.cc
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,12 @@ void InputSection<E>::dispatch(Context<E> &ctx, Action table[3][4], i64 i,
case PLT:
sym.flags |= NEEDS_PLT;
return;
case CPLT: {
std::scoped_lock lock(sym.mu);
sym.flags |= NEEDS_PLT;
sym.is_canonical = true;
return;
}
case DYNREL:
if (!is_writable) {
if (ctx.arg.z_text) {
Expand Down
3 changes: 2 additions & 1 deletion elf/mold.h
Original file line number Diff line number Diff line change
Expand Up @@ -312,7 +312,7 @@ class InputSection {
u8 p2align = 0;

private:
typedef enum : u8 { NONE, ERROR, COPYREL, PLT, DYNREL, BASEREL } Action;
typedef enum : u8 { NONE, ERROR, COPYREL, PLT, CPLT, DYNREL, BASEREL } Action;

void dispatch(Context<E> &ctx, Action table[3][4], i64 i,
const ElfRel<E> &rel, Symbol<E> &sym);
Expand Down Expand Up @@ -1820,6 +1820,7 @@ class Symbol {
u8 wrap : 1 = false;
u8 has_copyrel : 1 = false;
u8 copyrel_readonly : 1 = false;
u8 is_canonical : 1 = false;

// If a symbol can be interposed at runtime, `is_imported` is true.
// If a symbol is a dynamic symbol and can be used by other ELF
Expand Down
5 changes: 1 addition & 4 deletions elf/output-chunks.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1309,10 +1309,7 @@ void DynsymSection<E>::copy_buf(Context<E> &ctx) {
} else if (sym.file->is_dso || sym.esym().is_undef()) {
esym.st_shndx = SHN_UNDEF;
esym.st_size = 0;
if (sym.has_plt(ctx) && !ctx.arg.pic && sym.is_imported) {
// Emit an address for a canonical PLT
esym.st_value = sym.get_plt_addr(ctx);
}
esym.st_value = sym.is_canonical ? sym.get_plt_addr(ctx) : 0;
} else {
InputSection<E> *isec = sym.get_input_section();
if (!isec) {
Expand Down
21 changes: 10 additions & 11 deletions elf/passes.cc
Original file line number Diff line number Diff line change
Expand Up @@ -886,20 +886,19 @@ void scan_rels(Context<E> &ctx) {
ctx.got->add_got_symbol(ctx, sym);

if (sym->flags & NEEDS_PLT) {
bool is_canonical = (!ctx.arg.pic && sym->is_imported);

// If a symbol needs a canonical PLT, it is considered both
// imported and exported.
if (is_canonical)
if (sym->is_canonical) {
// A canonical PLT needs to be visible from DSOs.
sym->is_exported = true;

if ((sym->flags & NEEDS_GOT) && !is_canonical) {
ctx.pltgot->add_symbol(ctx, sym);
} else{
// If we need to create a canonical PLT, we can't use .plt.got
// because otherwise .plt.got and .got would refer each other,
// resulting in an infinite loop at runtime.
// We can't use .plt.got for a canonical PLT because otherwise
// .plt.got and .got would refer each other, resulting in an
// infinite loop at runtime.
ctx.plt->add_symbol(ctx, sym);
} else {
if (sym->flags & NEEDS_GOT)
ctx.pltgot->add_symbol(ctx, sym);
else
ctx.plt->add_symbol(ctx, sym);
}
}

Expand Down
53 changes: 53 additions & 0 deletions test/elf/non-canonical-plt.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
#!/bin/bash
export LC_ALL=C
set -e
CC="${CC:-cc}"
CXX="${CXX:-c++}"
GCC="${GCC:-gcc}"
GXX="${GXX:-g++}"
OBJDUMP="${OBJDUMP:-objdump}"
MACHINE="${MACHINE:-$(uname -m)}"
testname=$(basename "$0" .sh)
echo -n "Testing $testname ... "
cd "$(dirname "$0")"/../..
mold="$(pwd)/mold"
t=out/test/elf/$testname
mkdir -p $t

cat <<EOF | $CC -o $t/a.so -fPIC -shared -xc -
void *foo() {
return foo;
}
void *bar() {
return bar;
}
EOF

cat <<EOF | $CC -o $t/b.o -c -xc - -fPIC
void *bar();
void *baz() {
return bar;
}
EOF

cat <<EOF | $CC -o $t/c.o -c -xc - -fPIC
#include <stdio.h>
void *foo();
void *bar();
void *baz();
int main() {
printf("%d %d %d\n", foo == foo(), bar == bar(), bar == baz());
}
EOF

$CC -B. -no-pie -o $t/exe $t/a.so $t/b.o $t/c.o
$QEMU $t/exe | grep -q '^1 1 1$'

readelf --dyn-syms $t/exe | grep -q '00000000 .* foo'
readelf --dyn-syms $t/exe | grep -q '00000000 .* bar'

echo OK

0 comments on commit e0bc74a

Please sign in to comment.