This is an archive of the discontinued LLVM Phabricator instance.

Differential D153382

[lld-macho][test] Make reloc-subtractor.s robust
ClosedPublic

Authored by MaskRay on Jun 20 2023, 3:24 PM.

Details

Reviewers
int3
smeenai
nickdesaulniers
Group Reviewers
Restricted Project
Commits
rG8653db950377: [lld-macho][test] Make reloc-subtractor.s robust
Summary

The test requires that LLVM integreated assembler generates
SUBTRACTOR/UNSIGNED relocations for .long _minuend_1 - _subtrahend_1.
This currently works by luck because:

  • _minuend_1 and _subtrahend_1 are in different fragments (separated by a MCFillFragment)
  • and the result is known to be negative at parsing time.

D153096 will change the assembler to fold .long _minuend_1 - _subtrahend_1 to
a constant, giving ld -order_file no chance to change the result.

To fix the test, move the referenced labels after the label differences to block
constant folding.

Note: you may think the model is somewhat broken and it is. The
.subsections_via_symbols mechanism does not block such constant folding. In
reality SUBTRACTOR/UNSIGNED is for references to another section, which does not
have the problem.

Diff Detail

Event Timeline

MaskRay created this revision.Jun 20 2023, 3:24 PM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptJun 20 2023, 3:24 PM
MaskRay requested review of this revision.Jun 20 2023, 3:24 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 20 2023, 3:24 PM
Herald added a subscriber: llvm-commits. · View Herald Transcript
Harbormaster completed remote builds in B240106: Diff 533063.Jun 20 2023, 3:39 PM
smeenai accepted this revision.Jun 20 2023, 4:16 PM

LGTM

lld/test/MachO/reloc-subtractor.s
52

Is it important to repeat the .section directive?

This revision is now accepted and ready to land.Jun 20 2023, 4:16 PM
MaskRay added inline comments.Jun 20 2023, 8:48 PM
lld/test/MachO/reloc-subtractor.s
52

Yes, the above check lines test this...
# CHECK: {{0*}}[[#%x, SUB1ADDR:]] l {{.*}} __DATA,bar _sub1

Closed by commit rG8653db950377: [lld-macho][test] Make reloc-subtractor.s robust (authored by MaskRay). · Explain WhyJun 20 2023, 8:50 PM
This revision was automatically updated to reflect the committed changes.
MaskRay added a commit: rG8653db950377: [lld-macho][test] Make reloc-subtractor.s robust.
smeenai added inline comments.Jun 21 2023, 11:05 AM
lld/test/MachO/reloc-subtractor.s
52

I meant that we already have a .section __DATA,bar above _sub1, so do we need to issue that directive again here?

MaskRay added inline comments.Jun 21 2023, 11:09 AM
lld/test/MachO/reloc-subtractor.s
52

Sorry, I missed this. I think this .section __DATA,bar directive can be removed.

smeenai mentioned this in rGb1dadab3d9e3: [MachO] Remove redundant `.section` directive.Jun 21 2023, 4:17 PM
smeenai added inline comments.Jun 21 2023, 4:17 PM
lld/test/MachO/reloc-subtractor.s
52

Thanks, I pushed rGb1dadab3d9e3

Revision Contents

PathSize
lld/
test/
MachO/
19 lines

Diff 533118

lld/test/MachO/reloc-subtractor.s

Show All 27 Lines
.globl _main, _subtrahend_1, _subtrahend_2, _minuend1, _minuend2 .globl _main, _subtrahend_1, _subtrahend_2, _minuend1, _minuend2
.section __DATA,foo .section __DATA,foo
.space 16 .space 16
L_.minuend: L_.minuend:
.space 16 .space 16
.section __DATA,bar .section __DATA,bar
_minuend_1:
.space 16
_minuend_2:
.space 16
_subtrahend_1:
.space 16
_subtrahend_2:
.space 16
_sub1: _sub1:
.long _minuend_1 - _subtrahend_1 .long _minuend_1 - _subtrahend_1
.space 12 .space 12
_sub2: _sub2:
.long _minuend_2 - _subtrahend_2 + 2 .long _minuend_2 - _subtrahend_2 + 2
.space 12 .space 12
_sub3: _sub3:
.quad _minuend_1 - _subtrahend_1 + 4 .quad _minuend_1 - _subtrahend_1 + 4
.space 8 .space 8
_sub4: _sub4:
.quad _minuend_2 - _subtrahend_2 + 6 .quad _minuend_2 - _subtrahend_2 + 6
.space 8 .space 8
_sub5: _sub5:
.quad L_.minuend - _subtrahend_1 + 1 .quad L_.minuend - _subtrahend_1 + 1
.space 8 .space 8
.section __DATA,bar
smeenaiUnsubmitted
Not Done
ReplyInline Actions

Is it important to repeat the .section directive?

smeenai: Is it important to repeat the `.section` directive?
MaskRayAuthorUnsubmitted
Done
ReplyInline Actions

Yes, the above check lines test this...
# CHECK: {{0*}}[[#%x, SUB1ADDR:]] l {{.*}} __DATA,bar _sub1

MaskRay: Yes, the above check lines test this... `# CHECK: {{0*}}[[#%x, SUB1ADDR:]] l {{.*}}…
smeenaiUnsubmitted
Not Done
ReplyInline Actions

I meant that we already have a .section __DATA,bar above _sub1, so do we need to issue that directive again here?

smeenai: I meant that we already have a `.section __DATA,bar` above `_sub1`, so do we need to issue that…
MaskRayAuthorUnsubmitted
Done
ReplyInline Actions

Sorry, I missed this. I think this .section __DATA,bar directive can be removed.

MaskRay: Sorry, I missed this. I think this `.section __DATA,bar` directive can be removed.
smeenaiUnsubmitted
Not Done
ReplyInline Actions

Thanks, I pushed rGb1dadab3d9e3

smeenai: Thanks, I pushed rGb1dadab3d9e3
_minuend_1:
.space 16
_minuend_2:
.space 16
_subtrahend_1:
.space 16
_subtrahend_2:
.space 16
.text .text
.p2align 2 .p2align 2
_main: _main:
ret ret
.subsections_via_symbols .subsections_via_symbols
#--- order-file #--- order-file
## Reorder the symbols to make sure that the addends are being associated with ## Reorder the symbols to make sure that the addends are being associated with
## the minuend (and not the subtrahend) relocation. ## the minuend (and not the subtrahend) relocation.
_subtrahend_1 _subtrahend_1
_minuend_1 _minuend_1
_minuend_2 _minuend_2
_subtrahend_2 _subtrahend_2