Skip to content

[BOLT] Validate secondary entry point #135731

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 15, 2025
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
9 changes: 9 additions & 0 deletions bolt/include/bolt/Core/BinaryFunction.h
Original file line number Diff line number Diff line change
Expand Up @@ -1174,6 +1174,11 @@ class BinaryFunction {
return getSecondaryEntryPointSymbol(BB.getLabel());
}

/// Remove a label from the secondary entry point map.
void removeSymbolFromSecondaryEntryPointMap(const MCSymbol *Label) {
SecondaryEntryPoints.erase(Label);
}

/// Return true if the basic block is an entry point into the function
/// (either primary or secondary).
bool isEntryPoint(const BinaryBasicBlock &BB) const {
Expand Down Expand Up @@ -2126,6 +2131,10 @@ class BinaryFunction {
return Islands && !Islands->DataOffsets.empty();
}

bool isStartOfConstantIsland(uint64_t Offset) const {
return hasConstantIsland() && Islands->DataOffsets.count(Offset);
}

/// Return true iff the symbol could be seen inside this function otherwise
/// it is probably another function.
bool isSymbolValidInScope(const SymbolRef &Symbol, uint64_t SymbolSize) const;
Expand Down
9 changes: 9 additions & 0 deletions bolt/lib/Core/BinaryFunction.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1896,6 +1896,15 @@ void BinaryFunction::postProcessEntryPoints() {
if (BC.isAArch64() && Offset == getSize())
continue;

// If we have grabbed a wrong code label which actually points to some
// constant island inside the function, ignore this label and remove it
// from the secondary entry point map.
if (isStartOfConstantIsland(Offset)) {
BC.SymbolToFunctionMap.erase(Label);
removeSymbolFromSecondaryEntryPointMap(Label);
continue;
}

BC.errs() << "BOLT-WARNING: reference in the middle of instruction "
"detected in function "
<< *this << " at offset 0x" << Twine::utohexstr(Offset) << '\n';
Expand Down
34 changes: 34 additions & 0 deletions bolt/test/AArch64/validate-secondary-entry-point.s
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
# This test is to verify that BOLT won't take a label pointing to constant
# island as a secondary entry point (function `_start` doesn't have ELF size
# set originally) and the function won't otherwise be mistaken as non-simple.

# RUN: %clang %cflags -pie %s -o %t.so -Wl,-q -Wl,--init=_foo -Wl,--fini=_foo
# RUN: llvm-bolt %t.so -o %t.bolt.so --print-cfg 2>&1 | FileCheck %s
# CHECK-NOT: BOLT-WARNING: reference in the middle of instruction detected \
# CHECK-NOT: function _start at offset 0x{{[0-9a-f]+}}
# CHECK: Binary Function "_start" after building cfg

.text

.global _foo
.type _foo, %function
_foo:
ret

.global _start
.type _start, %function
_start:
b _foo

.balign 16
_random_consts:
.long 0x12345678
.long 0x90abcdef

.global _bar
.type _bar, %function
_bar:
ret

# Dummy relocation to force relocation mode
.reloc 0, R_AARCH64_NONE
34 changes: 34 additions & 0 deletions bolt/test/RISCV/validate-secondary-entry-point.s
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
# This test is to verify that BOLT won't take a label pointing to constant
# island as a secondary entry point (function `_start` doesn't have ELF size
# set originally) and the function won't otherwise be mistaken as non-simple.

# RUN: %clang %cflags -pie %s -o %t.so -Wl,-q -Wl,--init=_foo -Wl,--fini=_foo
# RUN: llvm-bolt %t.so -o %t.bolt.so --print-cfg 2>&1 | FileCheck %s
# CHECK-NOT: BOLT-WARNING: reference in the middle of instruction detected \
# CHECK-NOT: function _start at offset 0x{{[0-9a-f]+}}
# CHECK: Binary Function "_start" after building cfg

.text

.global _foo
.type _foo, %function
_foo:
ret

.global _start
.type _start, %function
_start:
j _foo

.balign 16
_random_consts:
.long 0x12345678
.long 0x90abcdef

.global _bar
.type _bar, %function
_bar:
ret

# Dummy relocation to force relocation mode
.reloc 0, R_RISCV_NONE
Loading