[llvm] [BOLT] Prevent missing ExternalBranch validation (PR #221927)
Jinjie Huang via llvm-commits
llvm-commits at lists.llvm.org
Tue Sep 8 02:07:07 PDT 2026
https://github.com/Jinjie-Huang created https://github.com/llvm/llvm-project/pull/221927
#165406 introduced validation to check whether a branch target is a valid instruction to detect corrupted CFGs. However, as described in [165406#discussion](https://github.com/llvm/llvm-project/pull/165406#discussion_r2601459105), it can miss validations when a function contains multiple invalid ExternalBranches.
Therefore, this patch introduces a branch-level set to prevent duplicate validations while ensuring no branches are missed.
>From 49d070567ad11b0f89f55f47a04d24d4aeaaa13f Mon Sep 17 00:00:00 2001
From: huangjinjie <huangjinjie at bytedance.com>
Date: Tue, 8 Sep 2026 16:39:50 +0800
Subject: [PATCH] Prevent missing ExternalBranch validation
---
bolt/include/bolt/Core/BinaryContext.h | 10 +++-
bolt/include/bolt/Core/BinaryFunction.h | 4 --
bolt/lib/Core/BinaryContext.cpp | 60 +++++++++++++---------
bolt/lib/Core/BinaryFunction.cpp | 12 ++++-
bolt/test/AArch64/validate-branch-target.s | 11 ++++
bolt/test/X86/validate-branch-target.s | 8 +++
6 files changed, 75 insertions(+), 30 deletions(-)
diff --git a/bolt/include/bolt/Core/BinaryContext.h b/bolt/include/bolt/Core/BinaryContext.h
index 2ccccde342e7f..214e7d872569f 100644
--- a/bolt/include/bolt/Core/BinaryContext.h
+++ b/bolt/include/bolt/Core/BinaryContext.h
@@ -24,6 +24,7 @@
#include "llvm/ADT/AddressRanges.h"
#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/DenseSet.h"
#include "llvm/ADT/EquivalenceClasses.h"
#include "llvm/ADT/StringMap.h"
#include "llvm/ADT/StringRef.h"
@@ -949,6 +950,10 @@ class BinaryContext {
/// and are referenced from BinaryFunction.
std::list<std::pair<BinaryFunction *, uint64_t>> InterproceduralReferences;
+ /// Invalid external branch targets discovered during validation.
+ DenseSet<std::pair<BinaryFunction *, uint64_t>>
+ InvalidInterproceduralReferences;
+
/// DWARF encoding. Available encoding types defined in BinaryFormat/Dwarf.h
/// enum Constants, e.g. DW_EH_PE_omit.
unsigned LSDAEncoding = dwarf::DW_EH_PE_omit;
@@ -1061,8 +1066,9 @@ class BinaryContext {
/// point.
///
/// This function also performs validations: If \p Address points to an
- /// invalid instruction or lies within a constant island, return nullptr and
- /// mark both \p Source and \p Target as ignored.
+ /// invalid instruction or lies within a constant island, return nullptr. The
+ /// caller is responsible for marking \p Source and \p Target as ignored
+ /// after all relevant references have been validated.
MCSymbol *handleExternalBranchTarget(uint64_t Address, BinaryFunction &Source,
BinaryFunction &Target);
diff --git a/bolt/include/bolt/Core/BinaryFunction.h b/bolt/include/bolt/Core/BinaryFunction.h
index 14d7f9b5b5359..ea4de245bc16c 100644
--- a/bolt/include/bolt/Core/BinaryFunction.h
+++ b/bolt/include/bolt/Core/BinaryFunction.h
@@ -402,10 +402,6 @@ class BinaryFunction {
/// True if the function should not have an associated symbol table entry.
bool IsAnonymous{false};
- /// Indicates whether branch validation has already been performed,
- /// to avoid redundant processing.
- bool NeedBranchValidation{true};
-
/// Name for the section this function code should reside in. When unset, the
/// default name is derived on demand from the function's name (see
/// getMainSectionName()). Deferring this avoids eagerly storing a copy of the
diff --git a/bolt/lib/Core/BinaryContext.cpp b/bolt/lib/Core/BinaryContext.cpp
index c40851d9aaa98..928969d5cb7b0 100644
--- a/bolt/lib/Core/BinaryContext.cpp
+++ b/bolt/lib/Core/BinaryContext.cpp
@@ -16,6 +16,7 @@
#include "bolt/Utils/CommandLineOpts.h"
#include "bolt/Utils/Utils.h"
#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/SmallPtrSet.h"
#include "llvm/ADT/Twine.h"
#include "llvm/DebugInfo/DWARF/DWARFCompileUnit.h"
#include "llvm/DebugInfo/DWARF/DWARFContext.h"
@@ -557,34 +558,35 @@ BinaryContext::handleAddressRef(uint64_t Address, BinaryFunction &BF,
MCSymbol *BinaryContext::handleExternalBranchTarget(uint64_t Address,
BinaryFunction &Source,
BinaryFunction &Target) {
+ const auto Reference = std::make_pair(&Source, Address);
+ // Avoid processing a known-invalid reference again when an ignored source
+ // is rescanned.
+ if (InvalidInterproceduralReferences.contains(Reference))
+ return nullptr;
+
const uint64_t Offset = Address - Target.getAddress();
assert(Offset < Target.getSize() &&
"Address should be inside the referenced function");
bool IsValid = true;
- if (Source.NeedBranchValidation) {
- if (Target.CurrentState == BinaryFunction::State::Disassembled &&
- !Target.getInstructionAtOffset(Offset)) {
- this->errs()
- << "BOLT-WARNING: corrupted control flow detected in function "
- << Source
- << ": an external branch/call targets an invalid instruction "
- << "in function " << Target << " at address 0x"
- << Twine::utohexstr(Address) << "; ignoring both functions\n";
- IsValid = false;
- }
- if (Target.isInConstantIsland(Address)) {
- this->errs() << "BOLT-WARNING: ignoring entry point at address 0x"
- << Twine::utohexstr(Address)
- << " in constant island of function " << Target << '\n';
- IsValid = false;
- }
+ if (Target.CurrentState == BinaryFunction::State::Disassembled &&
+ !Target.getInstructionAtOffset(Offset)) {
+ this->errs() << "BOLT-WARNING: corrupted control flow detected in function "
+ << Source
+ << ": an external branch/call targets an invalid instruction "
+ << "in function " << Target << " at address 0x"
+ << Twine::utohexstr(Address) << "; ignoring both functions\n";
+ IsValid = false;
+ }
+ if (Target.isInConstantIsland(Address)) {
+ this->errs() << "BOLT-WARNING: ignoring entry point at address 0x"
+ << Twine::utohexstr(Address)
+ << " in constant island of function " << Target << '\n';
+ IsValid = false;
}
if (!IsValid) {
- Source.NeedBranchValidation = false;
- Source.setIgnored();
- Target.setIgnored();
+ InvalidInterproceduralReferences.insert(Reference);
return nullptr;
}
@@ -1466,6 +1468,8 @@ bool BinaryContext::handleAArch64Veneer(uint64_t Address, bool MatchOnly) {
}
void BinaryContext::processInterproceduralReferences() {
+ SmallPtrSet<BinaryFunction *, 4> InvalidFunctions;
+
for (const std::pair<BinaryFunction *, uint64_t> &It :
InterproceduralReferences) {
BinaryFunction &Function = *It.first;
@@ -1490,9 +1494,12 @@ void BinaryContext::processInterproceduralReferences() {
<< TargetFunction->getPrintName() << '\n';
}
- // Create an extra entry point if needed. Can also render the target
- // function ignored if the reference is invalid.
- handleExternalBranchTarget(Address, Function, *TargetFunction);
+ // Create an extra entry point if needed. Defer state changes for invalid
+ // references until all references have been validated.
+ if (!handleExternalBranchTarget(Address, Function, *TargetFunction)) {
+ InvalidFunctions.insert(&Function);
+ InvalidFunctions.insert(TargetFunction);
+ }
continue;
}
@@ -1536,6 +1543,13 @@ void BinaryContext::processInterproceduralReferences() {
}
}
+ // Defer applying state changes until the entire validation scan is complete.
+ // Ignoring a function during the scan may lead to missed references or
+ // targets.
+ for (BinaryFunction *Function : InvalidFunctions)
+ if (!Function->isIgnored())
+ Function->setIgnored();
+
InterproceduralReferences.clear();
}
diff --git a/bolt/lib/Core/BinaryFunction.cpp b/bolt/lib/Core/BinaryFunction.cpp
index 99d6d27264fbe..22a365d1c6ab6 100644
--- a/bolt/lib/Core/BinaryFunction.cpp
+++ b/bolt/lib/Core/BinaryFunction.cpp
@@ -21,6 +21,7 @@
#include "bolt/Utils/NameShortener.h"
#include "bolt/Utils/Utils.h"
#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/SmallPtrSet.h"
#include "llvm/ADT/SmallSet.h"
#include "llvm/ADT/StringExtras.h"
#include "llvm/ADT/StringRef.h"
@@ -1575,6 +1576,7 @@ void BinaryFunction::analyzeInstructionForFuncReference(const MCInst &Inst) {
bool BinaryFunction::scanExternalRefs() {
bool Success = true;
bool DisassemblyFailed = false;
+ SmallPtrSet<BinaryFunction *, 4> InvalidTargets;
// Ignore pseudo functions.
if (isPseudo())
@@ -1683,8 +1685,10 @@ bool BinaryFunction::scanExternalRefs() {
// reference.
BranchTargetSymbol =
BC.handleExternalBranchTarget(TargetAddress, *this, *TargetFunction);
- if (!BranchTargetSymbol)
+ if (!BranchTargetSymbol) {
+ InvalidTargets.insert(TargetFunction);
continue;
+ }
}
// Can't find more references. Not creating relocations since we are not
@@ -1887,6 +1891,12 @@ bool BinaryFunction::scanExternalRefs() {
if (opts::Verbosity >= 1 && !Success)
BC.outs() << "BOLT-INFO: failed to scan refs for " << *this << '\n';
+ // Apply target state only after the complete source has been scanned. The
+ // source is either already ignored or is marked ignored by the caller.
+ for (BinaryFunction *Target : InvalidTargets)
+ if (!Target->isIgnored())
+ Target->setIgnored();
+
return Success;
}
diff --git a/bolt/test/AArch64/validate-branch-target.s b/bolt/test/AArch64/validate-branch-target.s
index b187fe762dea2..3855384b8d191 100644
--- a/bolt/test/AArch64/validate-branch-target.s
+++ b/bolt/test/AArch64/validate-branch-target.s
@@ -8,6 +8,7 @@
# RUN: llvm-bolt %t/main.exe -o %t/main.exe.bolt -lite=0 2>&1 | FileCheck %s --check-prefix=CHECK-TARGETS
# CHECK-TARGETS: BOLT-WARNING: corrupted control flow detected in function external_corrupt: an external branch/call targets an invalid instruction in function external_func at address 0x{{[0-9a-f]+}}; ignoring both functions
+# CHECK-TARGETS: BOLT-WARNING: corrupted control flow detected in function external_corrupt: an external branch/call targets an invalid instruction in function external_func_2 at address 0x{{[0-9a-f]+}}; ignoring both functions
# CHECK-TARGETS: BOLT-WARNING: corrupted control flow detected in function internal_corrupt: an internal branch/call targets an invalid instruction at address 0x{{[0-9a-f]+}}; ignoring this function
@@ -24,6 +25,7 @@ constant_island_0:
.type external_corrupt, at function
external_corrupt:
b constant_island_1 // targeting the data in code externally
+ b constant_island_2 // targeting another invalid target from the same source
.size external_corrupt,.-external_corrupt
.globl external_func
@@ -34,3 +36,12 @@ constant_island_1:
.word 0xffffffff // data in code
ret
.size external_func,.-external_func
+
+.globl external_func_2
+.type external_func_2, at function
+external_func_2:
+ sub x0, x0, x1
+constant_island_2:
+ .word 0xffffffff // data in code
+ ret
+.size external_func_2,.-external_func_2
diff --git a/bolt/test/X86/validate-branch-target.s b/bolt/test/X86/validate-branch-target.s
index 3cc5b9154e12a..527bf01979e12 100644
--- a/bolt/test/X86/validate-branch-target.s
+++ b/bolt/test/X86/validate-branch-target.s
@@ -8,6 +8,7 @@
# RUN: llvm-bolt %t/main.exe -o %t/main.exe.bolt -lite=0 2>&1 | FileCheck %s --check-prefix=CHECK-TARGETS
# CHECK-TARGETS: BOLT-WARNING: corrupted control flow detected in function external_corrupt: an external branch/call targets an invalid instruction in function external_func at address 0x{{[0-9a-f]+}}; ignoring both functions
+# CHECK-TARGETS: BOLT-WARNING: corrupted control flow detected in function external_corrupt: an external branch/call targets an invalid instruction in function external_func_2 at address 0x{{[0-9a-f]+}}; ignoring both functions
# CHECK-TARGETS: BOLT-WARNING: corrupted control flow detected in function internal_corrupt: an internal branch/call targets an invalid instruction at address 0x{{[0-9a-f]+}}; ignoring this function
@@ -24,6 +25,7 @@ data_in_code:
.type external_corrupt, at function
external_corrupt:
jb external_func + 1 # targeting the middle of normal instruction externally
+ jb external_func_2 + 1 # targeting another invalid instruction from the same source
.size external_corrupt,.-external_corrupt
.globl external_func
@@ -31,3 +33,9 @@ external_corrupt:
external_func:
addq $1, %rax # normal instruction
.size external_func,.-external_func
+
+.globl external_func_2
+.type external_func_2, at function
+external_func_2:
+ subq $1, %rax # another normal instruction
+.size external_func_2,.-external_func_2
More information about the llvm-commits
mailing list