This is an archive of the discontinued LLVM Phabricator instance.

Differential D23157

[COFF][ARM] Error out if 24 bit thumb branches are out of range
ClosedPublic

Authored by mstorsjo on Aug 4 2016, 2:12 AM.

Details

Reviewers
zatrazz
ruiu
davide
peter.smith
Summary

In the ELF linker, the same situation already errors out with "relocation R_ARM_THM_CALL out of range".

Diff Detail

Event Timeline

mstorsjo updated this revision to Diff 66773.Aug 4 2016, 2:12 AM
mstorsjo retitled this revision from to [COFF][ARM] Error out if 24 bit thumb branches are out of range.
mstorsjo updated this object.
mstorsjo added reviewers: peter.smith, zatrazz.
mstorsjo added a subscriber: llvm-commits.
Herald added subscribers: samparker, rengolin, aemerson. · View Herald TranscriptAug 4 2016, 2:12 AM
mstorsjo added a reviewer: ruiu.Aug 4 2016, 11:10 AM
ruiu edited edge metadata.Aug 4 2016, 2:03 PM

LGTM

COFF/Chunks.cpp
119–120

This does not depend on none of S, J1 nor J2, so please move this to the beginning of this function.

ruiu accepted this revision.Aug 4 2016, 2:03 PM
ruiu edited edge metadata.
This revision is now accepted and ready to land.Aug 4 2016, 2:03 PM
davide added a subscriber: davide.Aug 4 2016, 5:26 PM

ehm, do you have a testcase?

mstorsjo added inline comments.Aug 4 2016, 10:58 PM
COFF/Chunks.cpp
119–120

Ok, moving it up

mstorsjo updated this revision to Diff 66910.Aug 4 2016, 11:01 PM
mstorsjo edited edge metadata.

Moved the check up in the function as requested, added a test based on ELF/arm-thumb-branch-error.s.

davide accepted this revision.Aug 5 2016, 1:21 AM
davide added a reviewer: davide.

LGTM

mstorsjo added a comment.Aug 5 2016, 2:59 AM

Can someone commit this one, now that it has no objections and a bunch of approvals? I don't have commit access.

(The diff is generated on top of some of the other patches touching code close by, but they shouldn't directly conflict.)

compnerd closed this revision.Aug 5 2016, 10:41 AM
compnerd added a subscriber: compnerd.

SVN r277838

Revision Contents

PathSize
COFF/
2 lines
test/
COFF/
Inputs/
2 lines
10 lines

Diff 66910

COFF/Chunks.cpp

Show First 20 Lines • Show All 105 Lines • ▼ Show 20 Linesstatic void applyBranch20T(uint8_t *Off, int32_t V) {
uint32_t S = V < 0 ? 1 : 0; uint32_t S = V < 0 ? 1 : 0;
uint32_t J1 = (V >> 19) & 1; uint32_t J1 = (V >> 19) & 1;
uint32_t J2 = (V >> 18) & 1; uint32_t J2 = (V >> 18) & 1;
or16(Off, (S << 10) | ((V >> 12) & 0x3f)); or16(Off, (S << 10) | ((V >> 12) & 0x3f));
or16(Off + 2, (J1 << 13) | (J2 << 11) | ((V >> 1) & 0x7ff)); or16(Off + 2, (J1 << 13) | (J2 << 11) | ((V >> 1) & 0x7ff));
} }
static void applyBranch24T(uint8_t *Off, int32_t V) { static void applyBranch24T(uint8_t *Off, int32_t V) {
if (!isInt<25>(V))
fatal("relocation out of range");
uint32_t S = V < 0 ? 1 : 0; uint32_t S = V < 0 ? 1 : 0;
uint32_t J1 = ((~V >> 23) & 1) ^ S; uint32_t J1 = ((~V >> 23) & 1) ^ S;
uint32_t J2 = ((~V >> 22) & 1) ^ S; uint32_t J2 = ((~V >> 22) & 1) ^ S;
or16(Off, (S << 10) | ((V >> 12) & 0x3ff)); or16(Off, (S << 10) | ((V >> 12) & 0x3ff));
// Clear out the J1 and J2 bits which may be set. // Clear out the J1 and J2 bits which may be set.
ruiuUnsubmitted
Not Done
ReplyInline Actions

This does not depend on none of S, J1 nor J2, so please move this to the beginning of this function.

ruiu: This does not depend on none of S, J1 nor J2, so please move this to the beginning of this…
mstorsjoAuthorUnsubmitted
Not Done
ReplyInline Actions

Ok, moving it up

mstorsjo: Ok, moving it up
write16le(Off + 2, (read16le(Off + 2) & 0xd000) | (J1 << 13) | (J2 << 11) | ((V >> 1) & 0x7ff)); write16le(Off + 2, (read16le(Off + 2) & 0xd000) | (J1 << 13) | (J2 << 11) | ((V >> 1) & 0x7ff));
} }
void SectionChunk::applyRelARM(uint8_t *Off, uint16_t Type, Defined *Sym, void SectionChunk::applyRelARM(uint8_t *Off, uint16_t Type, Defined *Sym,
uint64_t P) const { uint64_t P) const {
uint64_t S = Sym->getRVA(); uint64_t S = Sym->getRVA();
// Pointer to thumb code must have the LSB set. // Pointer to thumb code must have the LSB set.
if (Sym->isExecutable()) if (Sym->isExecutable())
▲ Show 20 Lines • Show All 224 Lines • Show Last 20 Lines

test/COFF/Inputs/far-arm-thumb-abs.s

  • This file was added.
.global too_far1
too_far1 = 0x1401004

test/COFF/arm-thumb-branch-error.s

  • This file was added.
// RUN: llvm-mc -filetype=obj -triple=thumbv7a-windows-gnu %s -o %t
// RUN: llvm-mc -filetype=obj -triple=thumbv7a-windows-gnu %S/Inputs/far-arm-thumb-abs.s -o %tfar
// RUN: not lld-link -entry:_start -subsystem:console %t %tfar -out:%t2 2>&1 | FileCheck %s
// REQUIRES: arm
.syntax unified
.globl _start
_start:
bl too_far1
// CHECK: relocation out of range