This is an archive of the discontinued LLVM Phabricator instance.

Differential D58510

[yaml2obj]Allow explicit symbol indexes in relocations and emit error for bad names
ClosedPublic

Authored by jhenderson on Feb 21 2019, 7:44 AM.

Details

Reviewers
grimar
ruiu
jakehehrlich
• espindola
alexander-shaposhnikov
rupprecht
Commits
rG9bc817a0ae7d: [yaml2obj]Allow explicit symbol indexes in relocations and emit error for bad…
rL355938: [yaml2obj]Allow explicit symbol indexes in relocations and emit error for bad…
Summary

Prior to this change, the "Symbol" field of a relocation would always be assumed to be a symbol name, and if no such symbol existed, the relocation would reference index 0. This confused me when I tried to use a literal symbol index in the field: since "0x1" was not a known symbol name, the symbol index was set as 0. This change fallsback to treating unknown symbol names as integers, and emits an error if the name is not found and the string is not an integer.

Note that the Symbol field is optional, so if a relocation doesn't reference a symbol, it shouldn't be specified. The new error required a number of test updates.

This change relies on D58508 landing first with LLD test changes (I haven't got a monorepo checkout sorted out yet).

Diff Detail

Repository
rL LLVM

Event Timeline

jhenderson created this revision.Feb 21 2019, 7:44 AM
Herald added a reviewer: • espindola. · View Herald TranscriptFeb 21 2019, 7:44 AM
Herald added a reviewer: alexander-shaposhnikov. · View Herald Transcript
Herald added a reviewer: rupprecht. · View Herald Transcript
Herald added a project: Restricted Project. · View Herald Transcript
Herald added subscribers: jdoerfert, rupprecht, arphaman and 4 others. · View Herald Transcript
jhenderson added a parent revision: D58508: [ELF][test]Remove unnecessary empty symbol references in yaml/add missing symbols for relocs.Feb 21 2019, 7:45 AM
ruiu accepted this revision.Feb 21 2019, 10:32 AM

LGTM

This revision is now accepted and ready to land.Feb 21 2019, 10:32 AM
grimar added inline comments.Feb 22 2019, 12:23 AM
test/tools/yaml2obj/relocation-explicit-symbol-index.yaml
2 ↗(On Diff #187796)

Do you need --docnum?

10 ↗(On Diff #187796)

I would suggest doing something like we have in LLD tests here to show the values
you want to pay attention to explicitly. e.g.:

# CHECK-NEXT:     0000: 011B033B 1C000000 02000000 A80E0000
##                               ^        ^-- FDE(1) PC
##                               ^-- Number of FDEs
# CHECK-NEXT:     0010: 38000000 AA0E0000 50000000
##                               ^-- FDE(2) PC

Up to you though.

grimar added inline comments.Feb 22 2019, 12:27 AM
tools/yaml2obj/yaml2elf.cpp
538 ↗(On Diff #187796)

I suggest merging ifs

if (Rel.Symbol && SymN2I.lookup(*Rel.Symbol, SymIdx) &&
    !to_integer(*Rel.Symbol, SymIdx)) {
jhenderson marked 5 inline comments as done.Feb 22 2019, 2:50 AM
jhenderson added inline comments.
test/tools/yaml2obj/relocation-explicit-symbol-index.yaml
2 ↗(On Diff #187796)

No, that was left over from a previous version of this test.

10 ↗(On Diff #187796)

I like that idea, thanks.

jhenderson updated this revision to Diff 187914.Feb 22 2019, 2:51 AM
jhenderson marked 2 inline comments as done.

Address review comments.

grimar accepted this revision.Feb 22 2019, 3:08 AM

LGTM

jhenderson added a comment.Feb 22 2019, 3:51 AM

I just ran into https://bugs.llvm.org/show_bug.cgi?id=40818, which is preventing me landing this change as is. I could modify the llvm-objcopy compress-debug-sections.yaml file to not reference a symbol at all, but I think that loses some of the intention of that test (plus it would just hide the bug).

jhenderson added a comment.Mar 1 2019, 1:13 AM
In D58510#1406984, @jhenderson wrote:

I just ran into https://bugs.llvm.org/show_bug.cgi?id=40818, which is preventing me landing this change as is. I could modify the llvm-objcopy compress-debug-sections.yaml file to not reference a symbol at all, but I think that loses some of the intention of that test (plus it would just hide the bug).

This bug has been fixed, but it revealed https://bugs.llvm.org/show_bug.cgi?id=40885, which also blocks this change unless I workaround it (I may do so for expediency, but I have other things to look at in the meantime).

Herald added a subscriber: MaskRay. · View Herald TranscriptMar 1 2019, 1:13 AM
jhenderson mentioned this in D58960: [llvm-objcopy] - Fix --compress-debug-sections when there are relocations..Mar 5 2019, 4:43 AM
Closed by commit rL355938: [yaml2obj]Allow explicit symbol indexes in relocations and emit error for bad… (authored by jhenderson). · Explain WhyMar 12 2019, 10:01 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents

PathSize
llvm/
trunk/
test/
ExecutionEngine/
RuntimeDyld/
X86/
1 line
Object/
AMDGPU/
13 lines
Lanai/
6 lines
X86/
3 lines
1 line
tools/
llvm-objdump/
X86/
3 lines
sanstats/
17 lines
yaml2obj/
36 lines
24 lines
tools/
yaml2obj/
13 lines

Diff 190288

llvm/trunk/test/ExecutionEngine/RuntimeDyld/X86/ELF_x86-64_none.yaml

Show All 14 Lines - Name: .text
Type: SHT_PROGBITS Type: SHT_PROGBITS
Flags: [ SHF_ALLOC ] Flags: [ SHF_ALLOC ]
- Name: .rela.text - Name: .rela.text
Type: SHT_RELA Type: SHT_RELA
Link: .symtab Link: .symtab
Info: .text Info: .text
Relocations: Relocations:
- Offset: 0x0000000000000000 - Offset: 0x0000000000000000
Symbol: ''
Type: R_X86_64_NONE Type: R_X86_64_NONE
Symbols: Symbols:
Global: Global:
- Name: _main - Name: _main
Section: .text Section: .text
Value: 0 Value: 0
Size: 4 Size: 4

llvm/trunk/test/Object/AMDGPU/elf64-relocs.yaml

Show All 32 Lines - Type: SHT_PROGBITS
Content: 0000000000000000 Content: 0000000000000000
- Type: SHT_RELA - Type: SHT_RELA
Name: .rela.text Name: .rela.text
Link: .symtab Link: .symtab
Info: .text Info: .text
AddressAlign: 0x08 AddressAlign: 0x08
Relocations: Relocations:
- Offset: 0x0 - Offset: 0x0
Symbol: s0
Type: R_AMDGPU_NONE Type: R_AMDGPU_NONE
- Offset: 0x2 - Offset: 0x2
Symbol: s1
Type: R_AMDGPU_ABS32_LO Type: R_AMDGPU_ABS32_LO
- Offset: 0x4 - Offset: 0x4
Symbol: s2
Type: R_AMDGPU_ABS32_HI Type: R_AMDGPU_ABS32_HI
- Offset: 0x6 - Offset: 0x6
Symbol: s3
Type: R_AMDGPU_ABS64 Type: R_AMDGPU_ABS64
- Offset: 0x8 - Offset: 0x8
Symbol: s4
Type: R_AMDGPU_REL32 Type: R_AMDGPU_REL32
- Offset: 0x10 - Offset: 0x10
Symbol: s5
Type: R_AMDGPU_REL64 Type: R_AMDGPU_REL64
- Offset: 0x12 - Offset: 0x12
Symbol: s6
Type: R_AMDGPU_ABS32 Type: R_AMDGPU_ABS32
- Offset: 0x14 - Offset: 0x14
Symbol: s7
Type: R_AMDGPU_GOTPCREL Type: R_AMDGPU_GOTPCREL
- Offset: 0x16 - Offset: 0x16
Symbol: s8
Type: R_AMDGPU_GOTPCREL32_LO Type: R_AMDGPU_GOTPCREL32_LO
- Offset: 0x18 - Offset: 0x18
Symbol: s9
Type: R_AMDGPU_GOTPCREL32_HI Type: R_AMDGPU_GOTPCREL32_HI
- Offset: 0x20 - Offset: 0x20
Symbol: s10
Type: R_AMDGPU_REL32_LO Type: R_AMDGPU_REL32_LO
- Offset: 0x22 - Offset: 0x22
Symbol: s11
Type: R_AMDGPU_REL32_HI Type: R_AMDGPU_REL32_HI
- Offset: 0x24 - Offset: 0x24
Symbol: s12
Type: R_AMDGPU_RELATIVE64 Type: R_AMDGPU_RELATIVE64
Symbols: Symbols:
Local: Local:
- Name: .text - Name: .text
Type: STT_SECTION Type: STT_SECTION
Section: .text Section: .text
Global: Global:
- Name: main - Name: main
Type: STT_FUNC Type: STT_FUNC
Section: .text Section: .text
Size: 0x08 Size: 0x08

llvm/trunk/test/Object/Lanai/yaml2obj-elf-lanai-rel.yaml

Show All 29 Lines - Type: SHT_REL
Link: .symtab Link: .symtab
Info: .text Info: .text
AddressAlign: 0x04 AddressAlign: 0x04
Relocations: Relocations:
- Offset: 0x0 - Offset: 0x0
Symbol: main Symbol: main
Type: R_LANAI_32 Type: R_LANAI_32
- Offset: 0x4 - Offset: 0x4
Symbol: a
Type: R_LANAI_NONE Type: R_LANAI_NONE
- Offset: 0x8 - Offset: 0x8
Symbol: b
Type: R_LANAI_21 Type: R_LANAI_21
- Offset: 0xC - Offset: 0xC
Symbol: c
Type: R_LANAI_21_F Type: R_LANAI_21_F
- Offset: 0x10 - Offset: 0x10
Symbol: d
Type: R_LANAI_25 Type: R_LANAI_25
- Offset: 0x14 - Offset: 0x14
Symbol: e
Type: R_LANAI_HI16 Type: R_LANAI_HI16
- Offset: 0x18 - Offset: 0x18
Symbol: f
Type: R_LANAI_LO16 Type: R_LANAI_LO16
Symbols: Symbols:
Local: Local:
- Name: .text - Name: .text
Type: STT_SECTION Type: STT_SECTION
Section: .text Section: .text
Global: Global:
- Name: main - Name: main
Type: STT_FUNC Type: STT_FUNC
Section: .text Section: .text
Size: 0x08 Size: 0x08

llvm/trunk/test/Object/X86/yaml-elf-x86-rel-broken.yaml

Show All 21 Lines - Type: SHT_REL
Name: .rel.text Name: .rel.text
Link: .symtab Link: .symtab
Info: .text Info: .text
AddressAlign: 0x04 AddressAlign: 0x04
Relocations: Relocations:
- Offset: 0 - Offset: 0
Symbol: main Symbol: main
Type: 0xFF Type: 0xFF
Symbols:
Global:
- Name: main

llvm/trunk/test/Object/yaml2obj-elf-rel-noref.yaml

Show All 15 LinesSections:
- Name: .rel.text - Name: .rel.text
Type: SHT_REL Type: SHT_REL
Flags: [ SHF_INFO_LINK ] Flags: [ SHF_INFO_LINK ]
Link: .symtab Link: .symtab
AddressAlign: 0x0000000000000004 AddressAlign: 0x0000000000000004
Info: .text Info: .text
Relocations: Relocations:
- Offset: 0x000000000000000C - Offset: 0x000000000000000C
Symbol: ''
Type: R_ARM_V4BX Type: R_ARM_V4BX
- Name: .data - Name: .data
Type: SHT_PROGBITS Type: SHT_PROGBITS
Flags: [ SHF_WRITE, SHF_ALLOC ] Flags: [ SHF_WRITE, SHF_ALLOC ]
AddressAlign: 0x0000000000000001 AddressAlign: 0x0000000000000001
Content: '' Content: ''
- Name: .bss - Name: .bss
Type: SHT_NOBITS Type: SHT_NOBITS
▲ Show 20 Lines • Show All 45 Lines • Show Last 20 Lines

llvm/trunk/test/tools/llvm-objdump/X86/disasm-zeroes-relocations.test

Show First 20 Lines • Show All 44 Lines • ▼ Show 20 Lines - Name: .rela.rodata
Info: .rodata Info: .rodata
Relocations: Relocations:
- Offset: 0x0000000000000000 - Offset: 0x0000000000000000
Symbol: x Symbol: x
Type: R_X86_64_64 Type: R_X86_64_64
- Offset: 0x0000000000000008 - Offset: 0x0000000000000008
Symbol: x Symbol: x
Type: R_X86_64_64 Type: R_X86_64_64
Symbols:
Global:
- Name: x

llvm/trunk/test/tools/sanstats/elf.test

Show First 20 Lines • Show All 79 Lines • ▼ Show 20 Lines - Name: .debug_info
Content: 660000000400000000000801000000000C0000000000000000000000000000000000000000002600000002000000000000000006000000015600000000010102000000000000000006000000015600000000010202000000000000000006000000015600000000010300 Content: 660000000400000000000801000000000C0000000000000000000000000000000000000000002600000002000000000000000006000000015600000000010102000000000000000006000000015600000000010202000000000000000006000000015600000000010300
- Name: .rela.debug_info - Name: .rela.debug_info
Type: SHT_RELA Type: SHT_RELA
Link: .symtab Link: .symtab
AddressAlign: 0x0000000000000008 AddressAlign: 0x0000000000000008
Info: .debug_info Info: .debug_info
Relocations: Relocations:
- Offset: 0x0000000000000006 - Offset: 0x0000000000000006
Symbol: ''
Type: R_X86_64_32 Type: R_X86_64_32
- Offset: 0x000000000000000C - Offset: 0x000000000000000C
Symbol: ''
Type: R_X86_64_32 Type: R_X86_64_32
- Offset: 0x0000000000000012 - Offset: 0x0000000000000012
Symbol: ''
Type: R_X86_64_32 Type: R_X86_64_32
Addend: 55 Addend: 55
- Offset: 0x0000000000000016 - Offset: 0x0000000000000016
Symbol: ''
Type: R_X86_64_32 Type: R_X86_64_32
- Offset: 0x000000000000001A - Offset: 0x000000000000001A
Symbol: ''
Type: R_X86_64_32 Type: R_X86_64_32
Addend: 59 Addend: 59
- Offset: 0x000000000000001E - Offset: 0x000000000000001E
Symbol: ''
Type: R_X86_64_64 Type: R_X86_64_64
- Offset: 0x000000000000002B - Offset: 0x000000000000002B
Symbol: ''
Type: R_X86_64_64 Type: R_X86_64_64
- Offset: 0x0000000000000039 - Offset: 0x0000000000000039
Symbol: ''
Type: R_X86_64_32 Type: R_X86_64_32
Addend: 64 Addend: 64
- Offset: 0x0000000000000040 - Offset: 0x0000000000000040
Symbol: ''
Type: R_X86_64_64 Type: R_X86_64_64
Addend: 16 Addend: 16
- Offset: 0x000000000000004E - Offset: 0x000000000000004E
Symbol: ''
Type: R_X86_64_32 Type: R_X86_64_32
Addend: 67 Addend: 67
- Offset: 0x0000000000000055 - Offset: 0x0000000000000055
Symbol: ''
Type: R_X86_64_64 Type: R_X86_64_64
Addend: 32 Addend: 32
- Offset: 0x0000000000000063 - Offset: 0x0000000000000063
Symbol: ''
Type: R_X86_64_32 Type: R_X86_64_32
Addend: 70 Addend: 70
- Name: .debug_ranges - Name: .debug_ranges
Type: SHT_PROGBITS Type: SHT_PROGBITS
AddressAlign: 0x0000000000000001 AddressAlign: 0x0000000000000001
Content: '' Content: ''
- Name: .debug_pubnames - Name: .debug_pubnames
Type: SHT_PROGBITS Type: SHT_PROGBITS
AddressAlign: 0x0000000000000001 AddressAlign: 0x0000000000000001
Content: 230000000200000000006A0000002A0000006631003F0000006632005400000066330000000000 Content: 230000000200000000006A0000002A0000006631003F0000006632005400000066330000000000
- Name: .rela.debug_pubnames - Name: .rela.debug_pubnames
Type: SHT_RELA Type: SHT_RELA
Link: .symtab Link: .symtab
AddressAlign: 0x0000000000000008 AddressAlign: 0x0000000000000008
Info: .debug_pubnames Info: .debug_pubnames
Relocations: Relocations:
- Offset: 0x0000000000000006 - Offset: 0x0000000000000006
Symbol: ''
Type: R_X86_64_32 Type: R_X86_64_32
- Name: .comment - Name: .comment
Type: SHT_PROGBITS Type: SHT_PROGBITS
Flags: [ SHF_MERGE, SHF_STRINGS ] Flags: [ SHF_MERGE, SHF_STRINGS ]
AddressAlign: 0x0000000000000001 AddressAlign: 0x0000000000000001
Content: 00636C616E672076657273696F6E20332E382E3020287472756E6B203235353339332920286C6C766D2F7472756E6B203235353734352900 Content: 00636C616E672076657273696F6E20332E382E3020287472756E6B203235353339332920286C6C766D2F7472756E6B203235353734352900
- Name: .note.GNU-stack - Name: .note.GNU-stack
Type: SHT_PROGBITS Type: SHT_PROGBITS
AddressAlign: 0x0000000000000001 AddressAlign: 0x0000000000000001
Content: '' Content: ''
- Name: .eh_frame - Name: .eh_frame
Type: SHT_X86_64_UNWIND Type: SHT_X86_64_UNWIND
Flags: [ SHF_ALLOC ] Flags: [ SHF_ALLOC ]
AddressAlign: 0x0000000000000008 AddressAlign: 0x0000000000000008
Content: 1400000000000000017A5200017810011B0C070890010000180000001C000000000000000600000000410E108602430D060000001800000038000000000000000600000000410E108602430D060000001C00000054000000000000000600000000410E108602430D0600000000000000 Content: 1400000000000000017A5200017810011B0C070890010000180000001C000000000000000600000000410E108602430D060000001800000038000000000000000600000000410E108602430D060000001C00000054000000000000000600000000410E108602430D0600000000000000
- Name: .rela.eh_frame - Name: .rela.eh_frame
Type: SHT_RELA Type: SHT_RELA
Link: .symtab Link: .symtab
AddressAlign: 0x0000000000000008 AddressAlign: 0x0000000000000008
Info: .eh_frame Info: .eh_frame
Relocations: Relocations:
- Offset: 0x0000000000000020 - Offset: 0x0000000000000020
Symbol: ''
Type: R_X86_64_PC32 Type: R_X86_64_PC32
- Offset: 0x000000000000003C - Offset: 0x000000000000003C
Symbol: ''
Type: R_X86_64_PC32 Type: R_X86_64_PC32
Addend: 16 Addend: 16
- Offset: 0x0000000000000058 - Offset: 0x0000000000000058
Symbol: ''
Type: R_X86_64_PC32 Type: R_X86_64_PC32
Addend: 32 Addend: 32
- Name: .debug_line - Name: .debug_line
Type: SHT_PROGBITS Type: SHT_PROGBITS
AddressAlign: 0x0000000000000001 AddressAlign: 0x0000000000000001
Content: 4300000002001A0000000101FB0E0D00010101010000000100000100662E630000000000000902000000000000000001050C0A4A0500BB050C0A4A0500BB050C0A4A0202000101 Content: 4300000002001A0000000101FB0E0D00010101010000000100000100662E630000000000000902000000000000000001050C0A4A0500BB050C0A4A0500BB050C0A4A0202000101
- Name: .rela.debug_line - Name: .rela.debug_line
Type: SHT_RELA Type: SHT_RELA
Link: .symtab Link: .symtab
AddressAlign: 0x0000000000000008 AddressAlign: 0x0000000000000008
Info: .debug_line Info: .debug_line
Relocations: Relocations:
- Offset: 0x0000000000000027 - Offset: 0x0000000000000027
Symbol: ''
Type: R_X86_64_64 Type: R_X86_64_64
Symbols: Symbols:
Local: Local:
- Name: f.c - Name: f.c
Type: STT_FILE Type: STT_FILE
- Type: STT_SECTION - Type: STT_SECTION
Section: .text Section: .text
- Type: STT_SECTION - Type: STT_SECTION
Show All 23 Lines

llvm/trunk/test/tools/yaml2obj/relocation-explicit-symbol-index.yaml

# Test that a relocation's symbol can be an integer.
# RUN: yaml2obj %s > %t
# LLVM tools (both llvm-readobj and llvm-objdump) reject relocations with
# invalid symbol indexes, so inspect the hex contents instead.
# RUN: llvm-readobj -x .rela.text %t | FileCheck %s
# CHECK: Hex dump of section '.rela.text':
# CHECK-NEXT: 0x00000000 00000000 00000000 02000000 42000000
# Symbol index 0x42 --^
# CHECK-NEXT: 0x00000010 00000000 00000000 00000000 00000000
# CHECK-NEXT: 0x00000020 02000000 01000000 00000000 00000000
# Symbol index 0x01 --^
--- !ELF
FileHeader:
Class: ELFCLASS64
Data: ELFDATA2LSB
Type: ET_REL
Machine: EM_X86_64
Sections:
- Name: .text
Type: SHT_PROGBITS
- Name: .rela.text
Type: SHT_RELA
Info: .text
Link: .symtab
Relocations:
- Type: R_X86_64_PC32
Offset: 0
Symbol: 0x42
- Type: R_X86_64_PC32
Offset: 0
Symbol: 0x1
Symbols:
Global:
- Name: foo

llvm/trunk/test/tools/yaml2obj/relocation-missing-symbol.yaml

# Show that yaml2obj rejects a symbol reference from a relocation if the symbol
# does not exist.
# RUN: not yaml2obj %s -o %t 2>&1 | FileCheck %s
# CHECK: Unknown symbol referenced: 'does_not_exist' at YAML section '.rela.text'
--- !ELF
FileHeader:
Class: ELFCLASS64
Data: ELFDATA2LSB
Type: ET_REL
Machine: EM_X86_64
Sections:
- Name: .text
Type: SHT_PROGBITS
- Name: .rela.text
Type: SHT_RELA
Info: .text
Link: .symtab
Relocations:
- Type: R_X86_64_PC32
Offset: 0
Symbol: does_not_exist

llvm/trunk/tools/yaml2obj/yaml2elf.cpp

Show First 20 Lines • Show All 524 Lines • ▼ Show 20 LinesELFState<ELFT>::writeSectionContent(Elf_Shdr &SHeader,
bool IsRela = Section.Type == llvm::ELF::SHT_RELA; bool IsRela = Section.Type == llvm::ELF::SHT_RELA;
SHeader.sh_entsize = IsRela ? sizeof(Elf_Rela) : sizeof(Elf_Rel); SHeader.sh_entsize = IsRela ? sizeof(Elf_Rela) : sizeof(Elf_Rel);
SHeader.sh_size = SHeader.sh_entsize * Section.Relocations.size(); SHeader.sh_size = SHeader.sh_entsize * Section.Relocations.size();
auto &OS = CBA.getOSAndAlignedOffset(SHeader.sh_offset, SHeader.sh_addralign); auto &OS = CBA.getOSAndAlignedOffset(SHeader.sh_offset, SHeader.sh_addralign);
for (const auto &Rel : Section.Relocations) { for (const auto &Rel : Section.Relocations) {
unsigned SymIdx = 0; unsigned SymIdx = 0;
// Some special relocation, R_ARM_v4BX for instance, does not have // If a relocation references a symbol, try to look one up in the symbol
// an external reference. So it ignores the return value of lookup() // table. If it is not there, treat the value as a symbol index.
// here. if (Rel.Symbol && SymN2I.lookup(*Rel.Symbol, SymIdx) &&
if (Rel.Symbol) !to_integer(*Rel.Symbol, SymIdx)) {
SymN2I.lookup(*Rel.Symbol, SymIdx); WithColor::error() << "Unknown symbol referenced: '" << *Rel.Symbol
<< "' at YAML section '" << Section.Name << "'.\n";
return false;
}
if (IsRela) { if (IsRela) {
Elf_Rela REntry; Elf_Rela REntry;
zero(REntry); zero(REntry);
REntry.r_offset = Rel.Offset; REntry.r_offset = Rel.Offset;
REntry.r_addend = Rel.Addend; REntry.r_addend = Rel.Addend;
REntry.setSymbolAndType(SymIdx, Rel.Type, isMips64EL(Doc)); REntry.setSymbolAndType(SymIdx, Rel.Type, isMips64EL(Doc));
OS.write((const char *)&REntry, sizeof(REntry)); OS.write((const char *)&REntry, sizeof(REntry));
▲ Show 20 Lines • Show All 392 Lines • Show Last 20 Lines