This is an archive of the discontinued LLVM Phabricator instance.

Differential D108769

[lld][Hexagon] Add checks for instructions that can have TLS relocations
ClosedPublic

Authored by sidneym on Aug 26 2021, 8:30 AM.

Details

Reviewers
bcain
MaskRay
Summary

Several instructions with potential TLS relocations were missing. This issue was found when building the Canadian LLVM toolchain.

Diff Detail

Event Timeline

sidneym created this revision.Aug 26 2021, 8:30 AM
Herald added a reviewer: MaskRay. · View Herald TranscriptAug 26 2021, 8:30 AM
Herald added subscribers: arichardson, emaste. · View Herald Transcript
sidneym requested review of this revision.Aug 26 2021, 8:30 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 26 2021, 8:30 AM
Herald added a subscriber: llvm-commits. · View Herald Transcript
Harbormaster completed remote builds in B121354: Diff 368892.Aug 26 2021, 9:07 AM
bcain added a comment.Aug 26 2021, 9:48 AM

The change LGTM, we may want to fix the error message.

lld/ELF/Arch/Hexagon.cpp
238

This error message is inaccurate, right? In this case it's not that relocation mode. If this error case applies to multiple relocations we should probably describe those here instead.

I'm not sure which ones apply but R_HEX_{16_X,TPREL_16} could work. Are there lld idioms about the format of these error messages that we should conform to?

sidneym added inline comments.Aug 26 2021, 3:08 PM
lld/ELF/Arch/Hexagon.cpp
238

It could be changed to 16_X relocation type.

bcain accepted this revision.Aug 26 2021, 8:13 PM
bcain added inline comments.
lld/ELF/Arch/Hexagon.cpp
238

unrecognized instruction for 16_X relocation -- like that? If so, that makes sense.

This revision is now accepted and ready to land.Aug 26 2021, 8:13 PM
sidneym updated this revision to Diff 369967.Sep 1 2021, 10:06 AM
sidneym retitled this revision from [lld][Hexagon] Add check for missing predicated TLS reloc to [lld][Hexagon] Add checks for instructions that can have TLS relocations.
sidneym edited the summary of this revision. (Show Details)
MaskRay accepted this revision.Sep 1 2021, 10:14 AM
Harbormaster completed remote builds in B122107: Diff 369967.Sep 1 2021, 11:19 AM
sidneym closed this revision.Sep 1 2021, 1:18 PM

Revision Contents

PathSize
lld/
ELF/
Arch/
2 lines
test/
ELF/
6 lines

Diff 368892

lld/ELF/Arch/Hexagon.cpp

Show First 20 Lines • Show All 222 Lines • ▼ Show 20 Linesstatic uint32_t findMaskR16(uint32_t insn) {
if ((0xff000000 & insn) == 0x48000000) if ((0xff000000 & insn) == 0x48000000)
return 0x061f20ff; return 0x061f20ff;
if ((0xff000000 & insn) == 0x49000000) if ((0xff000000 & insn) == 0x49000000)
return 0x061f3fe0; return 0x061f3fe0;
if ((0xff000000 & insn) == 0x78000000) if ((0xff000000 & insn) == 0x78000000)
return 0x00df3fe0; return 0x00df3fe0;
if ((0xff000000 & insn) == 0xb0000000) if ((0xff000000 & insn) == 0xb0000000)
return 0x0fe03fe0; return 0x0fe03fe0;
if ((0xf0000000 & insn) == 0x40000000)
return 0x000020f8;
if (isDuplex(insn)) if (isDuplex(insn))
return 0x03f00000; return 0x03f00000;
error("unrecognized instruction for R_HEX_16_X relocation: 0x" + error("unrecognized instruction for R_HEX_16_X relocation: 0x" +
utohexstr(insn)); utohexstr(insn));
bcainUnsubmitted
Not Done
ReplyInline Actions

This error message is inaccurate, right? In this case it's not that relocation mode. If this error case applies to multiple relocations we should probably describe those here instead.

I'm not sure which ones apply but R_HEX_{16_X,TPREL_16} could work. Are there lld idioms about the format of these error messages that we should conform to?

bcain: This error message is inaccurate, right? In this case it's not that relocation mode. If this…
sidneymAuthorUnsubmitted
Done
ReplyInline Actions

It could be changed to 16_X relocation type.

sidneym: It could be changed to 16_X relocation type.
bcainUnsubmitted
Not Done
ReplyInline Actions

unrecognized instruction for 16_X relocation -- like that? If so, that makes sense.

bcain: `unrecognized instruction for 16_X relocation` -- like that? If so, that makes sense.
return 0; return 0;
} }
static void or32le(uint8_t *p, int32_t v) { write32le(p, read32le(p) | v); } static void or32le(uint8_t *p, int32_t v) { write32le(p, read32le(p) | v); }
void Hexagon::relocate(uint8_t *loc, const Relocation &rel, void Hexagon::relocate(uint8_t *loc, const Relocation &rel,
uint64_t val) const { uint64_t val) const {
switch (rel.type) { switch (rel.type) {
▲ Show 20 Lines • Show All 147 Lines • Show Last 20 Lines

lld/test/ELF/hexagon-tls-le.s

Show All 29 Lines
# R_HEX_TPREL_LO16 # R_HEX_TPREL_LO16
# CHECK: { r3.l = #0xfff0 } # CHECK: { r3.l = #0xfff0 }
r3.l = #a@TPREL r3.l = #a@TPREL
# RELOC-NEXT: 0x1C R_HEX_TPREL_16 a 0x0 # RELOC-NEXT: 0x1C R_HEX_TPREL_16 a 0x0
# CHECK: { r4 = #-0x10 } # CHECK: { r4 = #-0x10 }
r4 = #a@TPREL r4 = #a@TPREL
# RELOC-NEXT: 0x20 R_HEX_TPREL_32_6_X a 0x0
# RELOC-NEXT: 0x24 R_HEX_TPREL_16_X a 0x0
# CHECK: { immext(#0xffffffc0)
# CHECK-NEXT: if (p0) memw(r0+##0xfffffff0) = r1
if (p0) memw(r0+##a@TPREL)=r1
.section .tdata,"awT",@progbits .section .tdata,"awT",@progbits
.globl a .globl a
.p2align 2 .p2align 2
a: a:
.word 1 .word 1
.size a, 4 .size a, 4
.globl b .globl b
Show All 16 Lines