Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 28 additions & 4 deletions bolt/lib/Target/AArch64/AArch64MCSymbolizer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -125,15 +125,39 @@ AArch64MCSymbolizer::adjustRelocation(const Relocation &Rel,
// instruction pairs and will perform necessary adjustments.
ErrorOr<uint64_t> SymbolValue = BC.getSymbolValue(*Rel.Symbol);
assert(SymbolValue && "Symbol value should be set");
(void)SymbolValue;

AdjustedRel.Symbol = BC.registerNameAtAddress("__BOLT_got_zero", 0, 0, 0);
AdjustedRel.Addend = Rel.Value;
const uint64_t SymbolPageAddr = *SymbolValue & ~0xfffULL;

// Check if defined symbol and GOT are on the same page. If they are not,
// disambiguate the operand.
if (BC.MIB->isADRP(Inst) && Rel.Addend == 0 &&
SymbolPageAddr == Rel.Value &&
!isPageAddressValidForGOT(SymbolPageAddr)) {
AdjustedRel.Type = ELF::R_AARCH64_ADR_PREL_PG_HI21;
} else {
AdjustedRel.Symbol = BC.registerNameAtAddress("__BOLT_got_zero", 0, 0, 0);
AdjustedRel.Addend = Rel.Value;
}
}

return AdjustedRel;
}

bool AArch64MCSymbolizer::isPageAddressValidForGOT(uint64_t PageAddress) const {
assert(!(PageAddress & 0xfffULL) && "Page address not aligned at 4KB");

ErrorOr<BinarySection &> GOT =
Function.getBinaryContext().getUniqueSectionByName(".got");
if (!GOT || !GOT->getSize())
return false;

const uint64_t GOTFirstPageAddress = GOT->getAddress() & ~0xfffULL;
const uint64_t GOTLastPageAddress =
(GOT->getAddress() + GOT->getSize() - 1) & ~0xfffULL;

return PageAddress >= GOTFirstPageAddress &&
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if we can do this, the GOT doesn't have to be page-aligned. And in some rare cases ADRP might refer to a page with GOT, but not to GOT it self..

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, the symbolizer-level conversion is only triggered when the address is guaranteed to not be in the GOT. There will be cases when the object will be on the same page as GOT in which case the symbolization will preserve the GOT reference and delegate the proper resolution to the FixRelaxationPass.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yota9, any further comments? If not, I'll merge.

PageAddress <= GOTLastPageAddress;
}

void AArch64MCSymbolizer::tryAddingPcLoadReferenceComment(raw_ostream &CStream,
int64_t Value,
uint64_t Address) {}
Expand Down
3 changes: 3 additions & 0 deletions bolt/lib/Target/AArch64/AArch64MCSymbolizer.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,9 @@ class AArch64MCSymbolizer : public MCSymbolizer {
std::optional<Relocation> adjustRelocation(const Relocation &Rel,
const MCInst &Inst) const;

/// Return true if \p PageAddress is a valid page address for .got section.
bool isPageAddressValidForGOT(uint64_t PageAddress) const;

public:
AArch64MCSymbolizer(BinaryFunction &Function, bool CreateNewSymbols = true)
: MCSymbolizer(*Function.getBinaryContext().Ctx.get(), nullptr),
Expand Down
100 changes: 100 additions & 0 deletions bolt/test/AArch64/got-load-symbolization.s
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
## Check that BOLT symbolizer properly handles loads from GOT, including
## instruction sequences changed/relaxed by the linker.

# RUN: split-file %s %t

# RUN: llvm-mc -filetype=obj -triple aarch64-unknown-unknown %t/main.s \
# RUN: -o %t/main.o
# RUN: llvm-mc -filetype=obj -triple aarch64-unknown-unknown %t/near.s \
# RUN: -o %t/near.o
# RUN: llvm-mc -filetype=obj -triple aarch64-unknown-unknown %t/far.s \
# RUN: -o %t/far.o
# RUN: %clang %cflags %t/main.o %t/near.o %t/far.o -o %t/main.exe -Wl,-q -static
# RUN: llvm-bolt %t/main.exe -o %t/main.bolt --keep-nops --print-disasm \
# RUN: --print-only=_start | FileCheck %s

#--- main.s

.text
.globl _start
.p2align 2
.type _start, @function
# CHECK-LABEL: _start
_start:

## Function address load relaxable into nop+adr.
# CHECK: nop
# CHECK-NEXT: adr x0, near
adrp x0, :got:near
ldr x0, [x0, :got_lo12:near]

## Function address load relaxable into adrp+add.
# CHECK-NEXT: adrp x1, far
# CHECK-NEXT: add x1, x1, :lo12:far
adrp x1, :got:far
ldr x1, [x1, :got_lo12:far]

## Non-relaxable due to the instruction in-between.
# CHECK-NEXT: adrp x2, __BOLT_got_zero
# CHECK-NEXT: nop
# CHECK-NEXT: ldr x2, [x2, :lo12:__BOLT_got_zero{{.*}}]
adrp x2, :got:near
nop
ldr x2, [x2, :got_lo12:near]

## Load data object with local visibility. Relaxable into adrp+add.
# CHECK-NEXT: adrp x3, "local_far_data/1"
# CHECK-NEXT: add x3, x3, :lo12:"local_far_data/1"
adrp x3, :got:local_far_data
ldr x3, [x3, :got_lo12:local_far_data]

## Global data reference relaxable into adrp+add.
# CHECK-NEXT: adrp x4, far_data
# CHECK-NEXT: add x4, x4, :lo12:far_data
adrp x4, :got:far_data
ldr x4, [x4, :got_lo12:far_data]

ret
.size _start, .-_start

.weak near
.weak far
.weak far_data

## Data object separated by more than 1MB from _start.
.data
.type local_far_data, @object
local_far_data:
.xword 0
.size local_far_data, .-local_far_data

#--- near.s

.text
.globl near
.type near, @function
near:
ret
.size near, .-near

#--- far.s

.text

## Insert 1MB of empty space to make objects after it unreachable by adr
## instructions in _start.
.space 0x100000

.globl far
.type far, @function
far:
ret
.size far, .-far

.data
.globl far_data
.type far_data, @object
far_data:
.xword 0
.size far_data, .-far_data