Skip to content

[X86][LLD] Handle R_X86_64_CODE_6_GOTTPOFF relocation type #117675

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

Merged
merged 15 commits into from
Dec 5, 2024
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 42 additions & 3 deletions lld/ELF/Arch/X86_64.cpp
Copy link
Contributor

Choose a reason for hiding this comment

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

BTW, there is a problem with the newly added relocation optimization. Unlike the relocation w/ REX prefix, we don't check the bits strictly. For example, we don't check if the W, X, pp and zero bits in EVEX payload2. This brings an potential issue that the linker hides the bug of assembler.

Copy link
Contributor

@KanRobert KanRobert Nov 27, 2024

Choose a reason for hiding this comment

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

We can easily edit the binary in obj file to produce an invalid encoding with R_X86_64_CODE_6_GOTTPOFF relocation, e.g. set the zero bits in payload2 to ones. And then feed the modified the obj to linker, it would error nothing with this implementation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Did you mean it's better to check the bits strictly as for REX2 prefix? Since we may not use EGPR in instructions with NDD/NF, so it's not necessary to check X bits. I've added check for W bits. Seemed it's complicated to check pp bits. I'll take a look and will add later.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated. Please help review. Thanks.

Original file line number Diff line number Diff line change
Expand Up @@ -401,6 +401,7 @@ RelExpr X86_64::getRelExpr(RelType type, const Symbol &s,
case R_X86_64_CODE_4_GOTPCRELX:
case R_X86_64_GOTTPOFF:
case R_X86_64_CODE_4_GOTTPOFF:
case R_X86_64_CODE_6_GOTTPOFF:
return R_GOT_PC;
case R_X86_64_GOTOFF64:
return R_GOTPLTREL;
Expand Down Expand Up @@ -562,8 +563,9 @@ void X86_64::relaxTlsGdToIe(uint8_t *loc, const Relocation &rel,
}
}

// In some conditions, R_X86_64_GOTTPOFF/R_X86_64_CODE_4_GOTTPOFF relocation can
// be optimized to R_X86_64_TPOFF32 so that it does not use GOT.
// In some conditions,
// R_X86_64_GOTTPOFF/R_X86_64_CODE_4_GOTTPOFF/R_X86_64_CODE_6_GOTTPOFF
// relocation can be optimized to R_X86_64_TPOFF32 so that it does not use GOT.
void X86_64::relaxTlsIeToLe(uint8_t *loc, const Relocation &rel,
uint64_t val) const {
uint8_t *inst = loc - 3;
Expand Down Expand Up @@ -605,7 +607,7 @@ void X86_64::relaxTlsIeToLe(uint8_t *loc, const Relocation &rel,
} else if (rel.type == R_X86_64_CODE_4_GOTTPOFF) {
if (loc[-4] != 0xd5) {
Err(ctx) << getErrorLoc(ctx, loc - 4)
<< "Invalid prefix with R_X86_64_CODE_4_GOTTPOFF!";
<< "invalid prefix with R_X86_64_CODE_4_GOTTPOFF!";
return;
}
const uint8_t rex = loc[-3];
Expand All @@ -623,6 +625,41 @@ void X86_64::relaxTlsIeToLe(uint8_t *loc, const Relocation &rel,
<< "R_X86_64_CODE_4_GOTTPOFF must be used in MOVQ or ADDQ "
"instructions only";
}
} else if (rel.type == R_X86_64_CODE_6_GOTTPOFF) {
if (loc[-6] != 0x62) {
Err(ctx) << getErrorLoc(ctx, loc - 6)
<< "invalid prefix with R_X86_64_CODE_6_GOTTPOFF!";
return;
}
// Check bits are satisfied:
// loc[-5]: X==1 (inverted polarity), (loc[-5] & 0x7) == 0x4
// loc[-4]: W==1, X2==1 (inverted polarity), pp==0b00(NP)
// loc[-3]: NF==1 or ND==1
// loc[-2]: opcode==0x1 or opcode==0x3
// loc[-1]: Mod==0b00, RM==0b101
if (((loc[-5] & 0x47) == 0x44) && ((loc[-4] & 0x87) == 0x84) &&
((loc[-3] & 0x14) != 0) && (loc[-2] == 0x1 || loc[-2] == 0x3) &&
((loc[-1] & 0xc7) == 0x5)) {
// "addq %reg1, foo@GOTTPOFF(%rip), %reg2" -> "addq $foo, %reg1, %reg2"
// "addq foo@GOTTPOFF(%rip), %reg1, %reg2" -> "addq $foo, %reg1, %reg2"
// "{nf} addq %reg1, foo@GOTTPOFF(%rip), %reg2"
// -> "{nf} addq $foo, %reg1, %reg2"
// "{nf} addq name@GOTTPOFF(%rip), %reg1, %reg2"
// -> "{nf} addq $foo, %reg1, %reg2"
// "{nf} addq name@GOTTPOFF(%rip), %reg" -> "{nf} addq $foo, %reg"
loc[-2] = 0x81;
// Move R bits to B bits in EVEX payloads and ModRM byte.
const uint8_t evexPayload0 = loc[-5];
if ((evexPayload0 & (1 << 7)) == 0)
loc[-5] = (evexPayload0 | (1 << 7)) & ~(1 << 5);
if ((evexPayload0 & (1 << 4)) == 0)
loc[-5] = evexPayload0 | (1 << 4) | (1 << 3);
*regSlot = 0xc0 | reg;
} else {
Err(ctx) << getErrorLoc(ctx, loc - 6)
Copy link
Member

Choose a reason for hiding this comment

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

new error messages must be tested

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

<< "R_X86_64_CODE_6_GOTTPOFF must be used in ADDQ instructions "
"with NDD/NF/NDD+NF only";
}
} else {
llvm_unreachable("Unsupported relocation type!");
}
Expand Down Expand Up @@ -782,6 +819,7 @@ int64_t X86_64::getImplicitAddend(const uint8_t *buf, RelType type) const {
case R_X86_64_PC32:
case R_X86_64_GOTTPOFF:
case R_X86_64_CODE_4_GOTTPOFF:
case R_X86_64_CODE_6_GOTTPOFF:
case R_X86_64_PLT32:
case R_X86_64_TLSGD:
case R_X86_64_TLSLD:
Expand Down Expand Up @@ -893,6 +931,7 @@ void X86_64::relocate(uint8_t *loc, const Relocation &rel, uint64_t val) const {
break;
case R_X86_64_GOTTPOFF:
case R_X86_64_CODE_4_GOTTPOFF:
case R_X86_64_CODE_6_GOTTPOFF:
if (rel.expr == R_RELAX_TLS_IE_TO_LE) {
relaxTlsIeToLe(loc, rel, val);
} else {
Expand Down
140 changes: 134 additions & 6 deletions lld/test/ELF/invalid/broken-relaxation-x64.test
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
# REQUIRES: x86

# RUN: yaml2obj %s -o %t.o
# RUN: not ld.lld %t.o -o /dev/null 2>&1 | FileCheck --check-prefix=ERR %s
# ERR: R_X86_64_GOTTPOFF must be used in MOVQ or ADDQ instructions only
# ERR: R_X86_64_GOTTPOFF must be used in MOVQ or ADDQ instructions only
# RUN: yaml2obj --docnum=1 %s -o %t1.o
# RUN: not ld.lld %t1.o -o /dev/null 2>&1 | FileCheck --check-prefix=ERR %s
# ERR: error: {{.*}}: R_X86_64_GOTTPOFF must be used in MOVQ or ADDQ instructions only
# ERR: error: {{.*}}: R_X86_64_GOTTPOFF must be used in MOVQ or ADDQ instructions only

## YAML below contains 2 relocations of type R_X86_64_GOTTPOFF, and a .text
## with fake content filled by 0xFF. That means instructions for relaxation are
## "broken", so they does not match any known valid relaxations. We also generate
## .tls section because we need it for correct processing of STT_TLS symbol.
!ELF
--- !ELF
FileHeader:
Class: ELFCLASS64
Data: ELFDATA2LSB
Expand Down Expand Up @@ -44,4 +44,132 @@ Symbols:
Value: 0x12345
Size: 4
Binding: STB_GLOBAL



# RUN: yaml2obj --docnum=2 %s -o %t2.o
Copy link
Member

Choose a reason for hiding this comment

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

Thanks for adding negative tests but YAML is not very readable. You can use assembly tests with .reloc ., R_.... See x86-64-reloc-32.s

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added one more assembly test.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is the new assembly test okay?

# RUN: not ld.lld %t2.o -o /dev/null 2>&1 | FileCheck --check-prefix=ERR2 %s
# ERR2: error: {{.*}}: invalid prefix with R_X86_64_CODE_4_GOTTPOFF!
# ERR2: error: {{.*}}: invalid prefix with R_X86_64_CODE_6_GOTTPOFF!

## YAML below contains 2 relocations of
## R_X86_64_CODE_4_GOTTPOFF/R_X86_64_CODE_6_GOTTPOFF type, and a .text with
## fake content filled by 0xFF. It's expected to get "invalid prefix" error
## message as above.
--- !ELF
FileHeader:
Class: ELFCLASS64
Data: ELFDATA2LSB
OSABI: ELFOSABI_FREEBSD
Type: ET_REL
Machine: EM_X86_64
Sections:
- Type: SHT_PROGBITS
Name: .text
Flags: [ SHF_ALLOC, SHF_EXECINSTR ]
AddressAlign: 0x04
Content: "FFFFFFFFFFFFFFFFFFFF"
- Type: SHT_PROGBITS
Name: .tls
Flags: [ SHF_ALLOC, SHF_TLS ]
- Type: SHT_REL
Name: .rel.text
Link: .symtab
Info: .text
AddressAlign: 0x04
Relocations:
- Offset: 4
Symbol: foo
Type: R_X86_64_CODE_4_GOTTPOFF
- Offset: 6
Symbol: foo
Type: R_X86_64_CODE_6_GOTTPOFF
Symbols:
- Name: foo
Type: STT_TLS
Section: .text
Value: 0x12345
Size: 4
Binding: STB_GLOBAL


# RUN: yaml2obj --docnum=3 %s -o %t3.o
# RUN: not ld.lld %t3.o -o /dev/null 2>&1 | FileCheck --check-prefix=ERR3 %s
# ERR3: error: {{.*}}: R_X86_64_CODE_4_GOTTPOFF must be used in MOVQ or ADDQ instructions only

## YAML below contains R_X86_64_CODE_4_GOTTPOFF relocation type, and a .text
## with fake content filled by 0xd5, 0xFF, ... and 0xFF. It's expected to get
## the error message as above.
--- !ELF
FileHeader:
Class: ELFCLASS64
Data: ELFDATA2LSB
OSABI: ELFOSABI_FREEBSD
Type: ET_REL
Machine: EM_X86_64
Sections:
- Type: SHT_PROGBITS
Name: .text
Flags: [ SHF_ALLOC, SHF_EXECINSTR ]
AddressAlign: 0x04
Content: "d5FFFFFFFFFFFFFFFFFF"
- Type: SHT_PROGBITS
Name: .tls
Flags: [ SHF_ALLOC, SHF_TLS ]
- Type: SHT_REL
Name: .rel.text
Link: .symtab
Info: .text
AddressAlign: 0x04
Relocations:
- Offset: 4
Symbol: foo
Type: R_X86_64_CODE_4_GOTTPOFF
Symbols:
- Name: foo
Type: STT_TLS
Section: .text
Value: 0x12345
Size: 4
Binding: STB_GLOBAL


# RUN: yaml2obj --docnum=4 %s -o %t4.o
# RUN: not ld.lld %t4.o -o /dev/null 2>&1 | FileCheck --check-prefix=ERR4 %s
# ERR4: error: {{.*}}: R_X86_64_CODE_6_GOTTPOFF must be used in ADDQ instructions with NDD/NF/NDD+NF only

## YAML below contains R_X86_64_CODE_6_GOTTPOFF relocation type, and a .text
## with fake content filled by 0x62, 0xFF, ... and 0xFF. It's expected to get
## the error message as above.
--- !ELF
FileHeader:
Class: ELFCLASS64
Data: ELFDATA2LSB
OSABI: ELFOSABI_FREEBSD
Type: ET_REL
Machine: EM_X86_64
Sections:
- Type: SHT_PROGBITS
Name: .text
Flags: [ SHF_ALLOC, SHF_EXECINSTR ]
AddressAlign: 0x04
Content: "62FFFFFFFFFFFFFFFFFF"
- Type: SHT_PROGBITS
Name: .tls
Flags: [ SHF_ALLOC, SHF_TLS ]
- Type: SHT_REL
Name: .rel.text
Link: .symtab
Info: .text
AddressAlign: 0x04
Relocations:
- Offset: 6
Symbol: foo
Type: R_X86_64_CODE_6_GOTTPOFF
Symbols:
- Name: foo
Type: STT_TLS
Section: .text
Value: 0x12345
Size: 4
Binding: STB_GLOBAL

5 changes: 5 additions & 0 deletions lld/test/ELF/pack-dyn-relocs-tls-x86-64.s
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
foo:
movq tlsvar@GOTTPOFF(%rip), %rcx
movq tlsvar2@GOTTPOFF(%rip), %r31
addq tlsvar3@GOTTPOFF(%rip), %rcx, %r16


.section .tdata,"awT",@progbits
Expand All @@ -21,7 +22,11 @@ tlsvar:
.word 42
tlsvar2:
.word 42
tlsvar3:
.word 42

// CHECK: Section ({{.+}}) .rela.dyn {
// CHECK-NEXT: R_X86_64_TPOFF64 - 0x1234
// CHECK-NEXT: R_X86_64_TPOFF64 - 0x1236
// CHECK-NEXT: R_X86_64_TPOFF64 - 0x1238
// CHECK-NEXT: }
25 changes: 25 additions & 0 deletions lld/test/ELF/tls-opt.s
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,25 @@
// DISASM-NEXT: leaq -4(%r15), %r15
// DISASM-NEXT: addq $-4, %rsp
// DISASM-NEXT: addq $-4, %r12
# EGPR
// DISASM-NEXT: movq $-8, %r16
// DISASM-NEXT: movq $-8, %r20
// DISASM-NEXT: movq $-4, %r16
// DISASM-NEXT: addq $-8, %r16
// DISASM-NEXT: addq $-8, %r28
// DISASM-NEXT: addq $-4, %r16
# NDD
// DISASM-NEXT: addq $-10, %r16, %r16
// DISASM-NEXT: addq $-10, %r16, %r20
// DISASM-NEXT: addq $-10, %r16, %rax
// DISASM-NEXT: addq $-10, %rax, %r16
// DISASM-NEXT: addq $-10, %r8, %r16
// DISASM-NEXT: addq $-10, %rax, %r12
# NDD + NF
// DISASM-NEXT: {nf} addq $-10, %r8, %r16
// DISASM-NEXT: {nf} addq $-10, %rax, %r12
# NF
// DISASM-NEXT: {nf} addq $-10, %r12

// LD to LE:
// DISASM-NEXT: movq %fs:0, %rax
Expand Down Expand Up @@ -82,6 +95,18 @@ _start:
addq tls0@GOTTPOFF(%rip), %r16
addq tls0@GOTTPOFF(%rip), %r28
addq tls1@GOTTPOFF(%rip), %r16
# NDD
addq tls0@GOTTPOFF(%rip), %r16, %r16
addq tls0@GOTTPOFF(%rip), %r16, %r20
addq tls0@GOTTPOFF(%rip), %r16, %rax
addq tls0@GOTTPOFF(%rip), %rax, %r16
addq %r8, tls0@GOTTPOFF(%rip), %r16
addq tls0@GOTTPOFF(%rip), %rax, %r12
# NDD + NF
{nf} addq %r8, tls0@GOTTPOFF(%rip), %r16
{nf} addq tls0@GOTTPOFF(%rip), %rax, %r12
# NF
{nf} addq tls0@GOTTPOFF(%rip), %r12

// LD to LE
leaq tls0@tlsld(%rip), %rdi
Expand Down
36 changes: 36 additions & 0 deletions lld/test/ELF/x86-64-tls-ie-err.s
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
# REQUIRES: x86
# RUN: llvm-mc -filetype=obj -triple=x86_64 %s -o %t.o
# RUN: not ld.lld %t.o -o /dev/null 2>&1 | FileCheck -DFILE=%t.o %s

# CHECK: error: [[FILE]]:(.text+0x2): invalid prefix with R_X86_64_CODE_4_GOTTPOFF!
# CHECK-NEXT: error: [[FILE]]:(.text+0x8): invalid prefix with R_X86_64_CODE_6_GOTTPOFF!
# CHECK-NEXT: error: [[FILE]]:(.text+0x12): R_X86_64_CODE_4_GOTTPOFF must be used in MOVQ or ADDQ instructions only
# CHECK-NEXT: error: [[FILE]]:(.text+0x1a): R_X86_64_CODE_6_GOTTPOFF must be used in ADDQ instructions with NDD/NF/NDD+NF only

## These negative tests are to check if the invalid prefix and unsupported
## instructions for TLS relocation types with APX instructions are handled as
## errors.

.type tls0,@object
.section .tbss,"awT",@nobits
.globl tls0
.align 4
tls0:
.long 0
.size tls0, 4

.text
.globl _start
_start:
addq 0(%rip), %rax, %r16
.reloc .-4, R_X86_64_CODE_4_GOTTPOFF, tls0-4

movq 0(%rip), %r16
.reloc .-4, R_X86_64_CODE_6_GOTTPOFF, tls0-4

andq 0(%rip), %r16
.reloc .-4, R_X86_64_CODE_4_GOTTPOFF, tls0-4

andq 0(%rip), %rax, %r16
.reloc .-4, R_X86_64_CODE_6_GOTTPOFF, tls0-4

Copy link
Member

Choose a reason for hiding this comment

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

delete trailing blank line

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated.

Loading
Loading