This is an archive of the discontinued LLVM Phabricator instance.

Differential D78898

[llvm-objcopy][MachO] Avoid accidental invalid relocations in tests
ClosedPublic

Authored by alexander-shaposhnikov on Apr 26 2020, 11:58 PM.

Details

Reviewers
seiya
MaskRay
smeenai
jhenderson
rupprecht
Commits
rGcf7edb6ea507: [llvm-objcopy][MachO] Avoid accidental invalid relocations in tests
Summary

Until recently yaml2obj didn't properly support relocations for MachO and whenever it saw nreloc != 0 and reloff != 0 it was populating those bytes with 0s. This behavior results in binaries having invalid relocations, e.g. if r_extern == 0 then r_symbolnum should be >= 1 (because in this case r_symbolnum is supposed to be an index of a section and for MachO section indices start from 1), but it's equal to 0.
In this diff we adjust the existing tests (for the tests which don't actually look at any relocations at all - we remove them
(this causes the binary to shrink a little bit), for the tests which essentially depend on relocations - we fix them).
This change is a preparation to unblock https://reviews.llvm.org/D71647 .

Test plan: make check-all

Diff Detail

Event Timeline

alexander-shaposhnikov created this revision.Apr 26 2020, 11:58 PM
Herald added a reviewer: rupprecht. · View Herald TranscriptApr 26 2020, 11:58 PM
Herald added a project: Restricted Project. · View Herald Transcript
Herald added a subscriber: abrachet. · View Herald Transcript
alexander-shaposhnikov edited the summary of this revision. (Show Details)Apr 26 2020, 11:59 PM
alexander-shaposhnikov edited the summary of this revision. (Show Details)Apr 27 2020, 12:06 AM
Harbormaster completed remote builds in B54743: Diff 260220.Apr 27 2020, 1:01 AM
jhenderson added a comment.Apr 27 2020, 1:03 AM

This seems reasonable to me, although without a fuller Mach-O understanding, I can't verify what you're describing makes sense, so I'll leave somebody else to review.

smeenai accepted this revision.Apr 27 2020, 2:23 PM

LGTM

llvm/test/tools/llvm-objcopy/MachO/Inputs/strip-all-with-dwarf.yaml
46

I don't think it makes much difference for this test, but would it be better/more realistic to have some of these pointing to different symbols?

This revision is now accepted and ready to land.Apr 27 2020, 2:23 PM
alexander-shaposhnikov marked an inline comment as done.Apr 27 2020, 2:26 PM
alexander-shaposhnikov added inline comments.
llvm/test/tools/llvm-objcopy/MachO/Inputs/strip-all-with-dwarf.yaml
46

sure, we can do that

alexander-shaposhnikov updated this revision to Diff 260471.Apr 27 2020, 3:16 PM

Address comments

Closed by commit rGcf7edb6ea507: [llvm-objcopy][MachO] Avoid accidental invalid relocations in tests (authored by alexander-shaposhnikov). · Explain WhyApr 27 2020, 3:40 PM
This revision was automatically updated to reflect the committed changes.
Harbormaster failed remote builds in B54883: Diff 260471!Apr 27 2020, 4:12 PM

Revision Contents

Diff 260220

llvm/test/tools/llvm-objcopy/MachO/Inputs/strip-all-with-dwarf.yaml

Show All 27 Lines Sections:
offset: 0x000004F8 offset: 0x000004F8
align: 4 align: 4
reloff: 0x00000818 reloff: 0x00000818
nreloc: 2 nreloc: 2
flags: 0x80000400 flags: 0x80000400
reserved1: 0x00000000 reserved1: 0x00000000
reserved2: 0x00000000 reserved2: 0x00000000
reserved3: 0x00000000 reserved3: 0x00000000
relocations:
- address: 0x0
symbolnum: 0
pcrel: false
length: 0
extern: true
type: 0
scattered: false
value: 0
- address: 0x0
symbolnum: 0
smeenaiUnsubmitted
Not Done
ReplyInline Actions

I don't think it makes much difference for this test, but would it be better/more realistic to have some of these pointing to different symbols?

smeenai: I don't think it makes much difference for this test, but would it be better/more realistic to…
alexander-shaposhnikovAuthorUnsubmitted
Done
ReplyInline Actions

sure, we can do that

alexander-shaposhnikov: sure, we can do that
pcrel: false
length: 0
extern: true
type: 0
scattered: false
value: 0
- sectname: __data - sectname: __data
segname: __DATA segname: __DATA
addr: 0x0000000000000024 addr: 0x0000000000000024
size: 4 size: 4
offset: 0x0000051C offset: 0x0000051C
align: 2 align: 2
reloff: 0x00000000 reloff: 0x00000000
nreloc: 0 nreloc: 0
Show All 32 Lines Sections:
offset: 0x000005E9 offset: 0x000005E9
align: 0 align: 0
reloff: 0x00000828 reloff: 0x00000828
nreloc: 4 nreloc: 4
flags: 0x02000000 flags: 0x02000000
reserved1: 0x00000000 reserved1: 0x00000000
reserved2: 0x00000000 reserved2: 0x00000000
reserved3: 0x00000000 reserved3: 0x00000000
relocations:
- address: 0x0
symbolnum: 0
pcrel: false
length: 0
extern: true
type: 0
scattered: false
value: 0
- address: 0x0
symbolnum: 0
pcrel: false
length: 0
extern: true
type: 0
scattered: false
value: 0
- address: 0x0
symbolnum: 0
pcrel: false
length: 0
extern: true
type: 0
scattered: false
value: 0
- address: 0x0
symbolnum: 0
pcrel: false
length: 0
extern: true
type: 0
scattered: false
value: 0
- sectname: __debug_macinfo - sectname: __debug_macinfo
segname: __DWARF segname: __DWARF
addr: 0x0000000000000166 addr: 0x0000000000000166
size: 1 size: 1
offset: 0x0000065E offset: 0x0000065E
align: 0 align: 0
reloff: 0x00000000 reloff: 0x00000000
nreloc: 0 nreloc: 0
▲ Show 20 Lines • Show All 56 Lines • ▼ Show 20 Lines Sections:
offset: 0x00000768 offset: 0x00000768
align: 3 align: 3
reloff: 0x00000848 reloff: 0x00000848
nreloc: 1 nreloc: 1
flags: 0x02000000 flags: 0x02000000
reserved1: 0x00000000 reserved1: 0x00000000
reserved2: 0x00000000 reserved2: 0x00000000
reserved3: 0x00000000 reserved3: 0x00000000
relocations:
- address: 0x0
symbolnum: 0
pcrel: false
length: 0
extern: true
type: 0
scattered: false
value: 0
- sectname: __eh_frame - sectname: __eh_frame
segname: __TEXT segname: __TEXT
addr: 0x0000000000000290 addr: 0x0000000000000290
size: 64 size: 64
offset: 0x00000788 offset: 0x00000788
align: 3 align: 3
reloff: 0x00000000 reloff: 0x00000000
nreloc: 0 nreloc: 0
flags: 0x6800000B flags: 0x6800000B
reserved1: 0x00000000 reserved1: 0x00000000
reserved2: 0x00000000 reserved2: 0x00000000
reserved3: 0x00000000 reserved3: 0x00000000
- sectname: __debug_line - sectname: __debug_line
segname: __DWARF segname: __DWARF
addr: 0x00000000000002D0 addr: 0x00000000000002D0
size: 78 size: 78
offset: 0x000007C8 offset: 0x000007C8
align: 0 align: 0
reloff: 0x00000850 reloff: 0x00000850
nreloc: 1 nreloc: 1
flags: 0x02000000 flags: 0x02000000
reserved1: 0x00000000 reserved1: 0x00000000
reserved2: 0x00000000 reserved2: 0x00000000
reserved3: 0x00000000 reserved3: 0x00000000
relocations:
- address: 0x0
symbolnum: 0
pcrel: false
length: 0
extern: true
type: 0
scattered: false
value: 0
- cmd: LC_BUILD_VERSION - cmd: LC_BUILD_VERSION
cmdsize: 24 cmdsize: 24
platform: 1 platform: 1
minos: 658944 minos: 658944
sdk: 0 sdk: 0
ntools: 0 ntools: 0
- cmd: LC_SYMTAB - cmd: LC_SYMTAB
cmdsize: 24 cmdsize: 24
▲ Show 20 Lines • Show All 261 Lines • Show Last 20 Lines

llvm/test/tools/llvm-objcopy/MachO/basic-big-endian-32-copy.test

Show All 25 Lines - cmd: LC_SEGMENT
flags: 0 flags: 0
Sections: Sections:
- sectname: __text - sectname: __text
segname: __TEXT segname: __TEXT
addr: 0x0000000000000000 addr: 0x0000000000000000
size: 61 size: 61
offset: 0x00000198 offset: 0x00000198
align: 4 align: 4
reloff: 0x00000250 reloff: 0x0
nreloc: 1 nreloc: 0
flags: 0x80000400 flags: 0x80000400
reserved1: 0x00000000 reserved1: 0x00000000
reserved2: 0x00000000 reserved2: 0x00000000
reserved3: 0x00000000 reserved3: 0x00000000
- sectname: __compact_unwind - sectname: __compact_unwind
segname: __LD segname: __LD
addr: 0x0000000000000040 addr: 0x0000000000000040
size: 40 size: 40
offset: 0x000001D8 offset: 0x000001D8
align: 2 align: 2
reloff: 0x00000258 reloff: 0x00000
nreloc: 2 nreloc: 0
flags: 0x02000000 flags: 0x02000000
reserved1: 0x00000000 reserved1: 0x00000000
reserved2: 0x00000000 reserved2: 0x00000000
reserved3: 0x00000000 reserved3: 0x00000000
- sectname: __eh_frame - sectname: __eh_frame
segname: __TEXT segname: __TEXT
addr: 0x0000000000000068 addr: 0x0000000000000068
size: 80 size: 80
offset: 0x00000200 offset: 0x00000200
align: 2 align: 2
reloff: 0x00000000 reloff: 0x00000000
nreloc: 0 nreloc: 0
flags: 0x6800000B flags: 0x6800000B
reserved1: 0x00000000 reserved1: 0x00000000
reserved2: 0x00000000 reserved2: 0x00000000
reserved3: 0x00000000 reserved3: 0x00000000
- cmd: LC_VERSION_MIN_MACOSX - cmd: LC_VERSION_MIN_MACOSX
cmdsize: 16 cmdsize: 16
version: 658688 version: 658688
sdk: 0 sdk: 0
- cmd: LC_SYMTAB - cmd: LC_SYMTAB
cmdsize: 24 cmdsize: 24
symoff: 616 symoff: 592
nsyms: 2 nsyms: 2
stroff: 640 stroff: 616
strsize: 16 strsize: 16
- cmd: LC_DYSYMTAB - cmd: LC_DYSYMTAB
cmdsize: 80 cmdsize: 80
ilocalsym: 0 ilocalsym: 0
nlocalsym: 0 nlocalsym: 0
iextdefsym: 0 iextdefsym: 0
nextdefsym: 2 nextdefsym: 2
iundefsym: 2 iundefsym: 2
Show All 32 Lines

llvm/test/tools/llvm-objcopy/MachO/basic-big-endian-64-copy.test

Show All 26 Lines - cmd: LC_SEGMENT_64
flags: 0 flags: 0
Sections: Sections:
- sectname: __text - sectname: __text
segname: __TEXT segname: __TEXT
addr: 0x0000000000000000 addr: 0x0000000000000000
size: 93 size: 93
offset: 0x00000220 offset: 0x00000220
align: 4 align: 4
reloff: 0x00000310 reloff: 0x0
nreloc: 3 nreloc: 0
flags: 0x80000400 flags: 0x80000400
reserved1: 0x00000000 reserved1: 0x00000000
reserved2: 0x00000000 reserved2: 0x00000000
reserved3: 0x00000000 reserved3: 0x00000000
- sectname: __cstring - sectname: __cstring
segname: __TEXT segname: __TEXT
addr: 0x000000000000005D addr: 0x000000000000005D
size: 48 size: 48
offset: 0x0000027D offset: 0x0000027D
align: 0 align: 0
reloff: 0x00000000 reloff: 0x00000000
nreloc: 0 nreloc: 0
flags: 0x00000002 flags: 0x00000002
reserved1: 0x00000000 reserved1: 0x00000000
reserved2: 0x00000000 reserved2: 0x00000000
reserved3: 0x00000000 reserved3: 0x00000000
- sectname: __compact_unwind - sectname: __compact_unwind
segname: __LD segname: __LD
addr: 0x0000000000000090 addr: 0x0000000000000090
size: 32 size: 32
offset: 0x000002B0 offset: 0x000002B0
align: 3 align: 3
reloff: 0x00000328 reloff: 0x0
nreloc: 1 nreloc: 0
flags: 0x02000000 flags: 0x02000000
reserved1: 0x00000000 reserved1: 0x00000000
reserved2: 0x00000000 reserved2: 0x00000000
reserved3: 0x00000000 reserved3: 0x00000000
- sectname: __eh_frame - sectname: __eh_frame
segname: __TEXT segname: __TEXT
addr: 0x00000000000000B0 addr: 0x00000000000000B0
size: 64 size: 64
offset: 0x000002D0 offset: 0x000002D0
align: 3 align: 3
reloff: 0x00000000 reloff: 0x00000000
nreloc: 0 nreloc: 0
flags: 0x6800000B flags: 0x6800000B
reserved1: 0x00000000 reserved1: 0x00000000
reserved2: 0x00000000 reserved2: 0x00000000
reserved3: 0x00000000 reserved3: 0x00000000
- cmd: LC_VERSION_MIN_MACOSX - cmd: LC_VERSION_MIN_MACOSX
cmdsize: 16 cmdsize: 16
version: 658432 version: 658432
sdk: 0 sdk: 0
- cmd: LC_SYMTAB - cmd: LC_SYMTAB
cmdsize: 24 cmdsize: 24
symoff: 816 symoff: 784
nsyms: 2 nsyms: 2
stroff: 848 stroff: 816
strsize: 36 strsize: 36
- cmd: LC_DYSYMTAB - cmd: LC_DYSYMTAB
cmdsize: 80 cmdsize: 80
ilocalsym: 0 ilocalsym: 0
nlocalsym: 0 nlocalsym: 0
iextdefsym: 0 iextdefsym: 0
nextdefsym: 1 nextdefsym: 1
iundefsym: 1 iundefsym: 1
Show All 31 Lines

llvm/test/tools/llvm-objcopy/MachO/basic-little-endian-32-copy.test

Show All 25 Lines - cmd: LC_SEGMENT
flags: 0 flags: 0
Sections: Sections:
- sectname: __text - sectname: __text
segname: __TEXT segname: __TEXT
addr: 0x0000000000000000 addr: 0x0000000000000000
size: 61 size: 61
offset: 0x00000198 offset: 0x00000198
align: 4 align: 4
reloff: 0x00000250 reloff: 0x0
nreloc: 1 nreloc: 0
flags: 0x80000400 flags: 0x80000400
reserved1: 0x00000000 reserved1: 0x00000000
reserved2: 0x00000000 reserved2: 0x00000000
reserved3: 0x00000000 reserved3: 0x00000000
- sectname: __compact_unwind - sectname: __compact_unwind
segname: __LD segname: __LD
addr: 0x0000000000000040 addr: 0x0000000000000040
size: 40 size: 40
offset: 0x000001D8 offset: 0x000001D8
align: 2 align: 2
reloff: 0x00000258 reloff: 0x0
nreloc: 2 nreloc: 0
flags: 0x02000000 flags: 0x02000000
reserved1: 0x00000000 reserved1: 0x00000000
reserved2: 0x00000000 reserved2: 0x00000000
reserved3: 0x00000000 reserved3: 0x00000000
- sectname: __eh_frame - sectname: __eh_frame
segname: __TEXT segname: __TEXT
addr: 0x0000000000000068 addr: 0x0000000000000068
size: 80 size: 80
offset: 0x00000200 offset: 0x00000200
align: 2 align: 2
reloff: 0x00000000 reloff: 0x00000000
nreloc: 0 nreloc: 0
flags: 0x6800000B flags: 0x6800000B
reserved1: 0x00000000 reserved1: 0x00000000
reserved2: 0x00000000 reserved2: 0x00000000
reserved3: 0x00000000 reserved3: 0x00000000
- cmd: LC_VERSION_MIN_MACOSX - cmd: LC_VERSION_MIN_MACOSX
cmdsize: 16 cmdsize: 16
version: 658688 version: 658688
sdk: 0 sdk: 0
- cmd: LC_SYMTAB - cmd: LC_SYMTAB
cmdsize: 24 cmdsize: 24
symoff: 616 symoff: 592
nsyms: 2 nsyms: 2
stroff: 640 stroff: 616
strsize: 16 strsize: 16
- cmd: LC_DYSYMTAB - cmd: LC_DYSYMTAB
cmdsize: 80 cmdsize: 80
ilocalsym: 0 ilocalsym: 0
nlocalsym: 0 nlocalsym: 0
iextdefsym: 0 iextdefsym: 0
nextdefsym: 2 nextdefsym: 2
iundefsym: 2 iundefsym: 2
Show All 32 Lines

llvm/test/tools/llvm-objcopy/MachO/basic-little-endian-64-copy.test

Show All 26 Lines - cmd: LC_SEGMENT_64
flags: 0 flags: 0
Sections: Sections:
- sectname: __text - sectname: __text
segname: __TEXT segname: __TEXT
addr: 0x0000000000000000 addr: 0x0000000000000000
size: 93 size: 93
offset: 0x00000220 offset: 0x00000220
align: 4 align: 4
reloff: 0x00000310 reloff: 0x0
nreloc: 3 nreloc: 0
flags: 0x80000400 flags: 0x80000400
reserved1: 0x00000000 reserved1: 0x00000000
reserved2: 0x00000000 reserved2: 0x00000000
reserved3: 0x00000000 reserved3: 0x00000000
- sectname: __cstring - sectname: __cstring
segname: __TEXT segname: __TEXT
addr: 0x000000000000005D addr: 0x000000000000005D
size: 48 size: 48
offset: 0x0000027D offset: 0x0000027D
align: 0 align: 0
reloff: 0x00000000 reloff: 0x00000000
nreloc: 0 nreloc: 0
flags: 0x00000002 flags: 0x00000002
reserved1: 0x00000000 reserved1: 0x00000000
reserved2: 0x00000000 reserved2: 0x00000000
reserved3: 0x00000000 reserved3: 0x00000000
- sectname: __compact_unwind - sectname: __compact_unwind
segname: __LD segname: __LD
addr: 0x0000000000000090 addr: 0x0000000000000090
size: 32 size: 32
offset: 0x000002B0 offset: 0x000002B0
align: 3 align: 3
reloff: 0x00000328 reloff: 0x0
nreloc: 1 nreloc: 0
flags: 0x02000000 flags: 0x02000000
reserved1: 0x00000000 reserved1: 0x00000000
reserved2: 0x00000000 reserved2: 0x00000000
reserved3: 0x00000000 reserved3: 0x00000000
- sectname: __eh_frame - sectname: __eh_frame
segname: __TEXT segname: __TEXT
addr: 0x00000000000000B0 addr: 0x00000000000000B0
size: 64 size: 64
offset: 0x000002D0 offset: 0x000002D0
align: 3 align: 3
reloff: 0x00000000 reloff: 0x00000000
nreloc: 0 nreloc: 0
flags: 0x6800000B flags: 0x6800000B
reserved1: 0x00000000 reserved1: 0x00000000
reserved2: 0x00000000 reserved2: 0x00000000
reserved3: 0x00000000 reserved3: 0x00000000
- cmd: LC_VERSION_MIN_MACOSX - cmd: LC_VERSION_MIN_MACOSX
cmdsize: 16 cmdsize: 16
version: 658432 version: 658432
sdk: 0 sdk: 0
- cmd: LC_SYMTAB - cmd: LC_SYMTAB
cmdsize: 24 cmdsize: 24
symoff: 816 symoff: 784
nsyms: 2 nsyms: 2
stroff: 848 stroff: 816
strsize: 36 strsize: 36
- cmd: LC_DYSYMTAB - cmd: LC_DYSYMTAB
cmdsize: 80 cmdsize: 80
ilocalsym: 0 ilocalsym: 0
nlocalsym: 0 nlocalsym: 0
iextdefsym: 0 iextdefsym: 0
nextdefsym: 1 nextdefsym: 1
iundefsym: 1 iundefsym: 1
Show All 31 Lines

llvm/test/tools/llvm-objcopy/MachO/strip-debug.test

Show All 17 Lines
# RUN: llvm-readobj -r %t | FileCheck %s --check-prefixes=RELOC,DEBUG # RUN: llvm-readobj -r %t | FileCheck %s --check-prefixes=RELOC,DEBUG
# RUN: llvm-readobj -r %t.stripped | FileCheck %s --check-prefix=RELOC # RUN: llvm-readobj -r %t.stripped | FileCheck %s --check-prefix=RELOC
# RUN: llvm-strip --strip-debug %t -o %t.stripped2 # RUN: llvm-strip --strip-debug %t -o %t.stripped2
# RUN: cmp %t.stripped %t.stripped2 # RUN: cmp %t.stripped %t.stripped2
# RELOC: Relocations [ # RELOC: Relocations [
# RELOC-NEXT: Section __text { # RELOC-NEXT: Section __text {
# RELOC-NEXT: 0x0 0 0 0 X86_64_RELOC_UNSIGNED 0 - # RELOC-NEXT: 0x0 0 0 1 X86_64_RELOC_UNSIGNED 0 _bar
# RELOC-NEXT: 0x0 0 0 0 X86_64_RELOC_UNSIGNED 0 - # RELOC-NEXT: 0x0 0 0 1 X86_64_RELOC_UNSIGNED 0 _bar
# RELOC-NEXT: } # RELOC-NEXT: }
# DEBUG: Section __debug_info { # DEBUG: Section __debug_info {
# DEBUG-NEXT: 0x0 0 0 0 X86_64_RELOC_UNSIGNED 0 - # DEBUG-NEXT: 0x0 0 0 1 X86_64_RELOC_UNSIGNED 0 _bar
# DEBUG-NEXT: 0x0 0 0 0 X86_64_RELOC_UNSIGNED 0 - # DEBUG-NEXT: 0x0 0 0 1 X86_64_RELOC_UNSIGNED 0 _bar
# DEBUG-NEXT: 0x0 0 0 0 X86_64_RELOC_UNSIGNED 0 - # DEBUG-NEXT: 0x0 0 0 1 X86_64_RELOC_UNSIGNED 0 _bar
# DEBUG-NEXT: 0x0 0 0 0 X86_64_RELOC_UNSIGNED 0 - # DEBUG-NEXT: 0x0 0 0 1 X86_64_RELOC_UNSIGNED 0 _bar
# DEBUG-NEXT: } # DEBUG-NEXT: }
# RELOC-NEXT: Section __compact_unwind { # RELOC-NEXT: Section __compact_unwind {
# RELOC-NEXT: 0x0 0 0 0 X86_64_RELOC_UNSIGNED 0 - # RELOC-NEXT: 0x0 0 0 1 X86_64_RELOC_UNSIGNED 0 _bar
# RELOC-NEXT: } # RELOC-NEXT: }
# DEBUG-NEXT: Section __debug_line { # DEBUG-NEXT: Section __debug_line {
# DEBUG-NEXT: 0x0 0 0 0 X86_64_RELOC_UNSIGNED 0 - # DEBUG-NEXT: 0x0 0 0 1 X86_64_RELOC_UNSIGNED 0 _bar
# DEBUG-NEXT: } # DEBUG-NEXT: }
# RELOC-NEXT: ] # RELOC-NEXT: ]