[llvm-branch-commits] [llvm] [BOLT] Skip out-of-range pending relocations (PR #116964)
https://github.com/paschalis-mpeis converted_to_draft https://github.com/llvm/llvm-project/pull/116964 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [BOLT] Skip out-of-range pending relocations (PR #116964)
paschalis-mpeis wrote: I've put up a separate patch that makes Relocation type 32b and adds an `Optional` field: - #130792 https://github.com/llvm/llvm-project/pull/116964 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [BOLT] Skip out-of-range pending relocations (PR #116964)
paschalis-mpeis wrote: Hey Maksim, Extending Relocations is even better. Thanks for the suggestion and the review. Before proceeding, and regarding the size overheads, I want to highlight an inconsistency with LLVM’s ObjectFile, where the type is 64 bits ([see here](https://github.com/llvm/llvm-project/blob/16cd5cdf4d6387e34d2bb723bc26c331c8d89d75/llvm/include/llvm/Object/ObjectFile.h#L628)). We only have 3 inlined sites of this in `RewriteInstance` (eg one is [here](https://github.com/llvm/llvm-project/blob/3c357a49d61e4c81a1ac016502ee504521bc8dda/bolt/lib/Rewrite/RewriteInstance.cpp#L2408)). If you agree, I'll proceed with an NFCI change, adding assertion overflow checks at these sites. https://github.com/llvm/llvm-project/pull/116964 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [BOLT] Skip out-of-range pending relocations (PR #116964)
https://github.com/paschalis-mpeis edited https://github.com/llvm/llvm-project/pull/116964 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [BOLT] Skip out-of-range pending relocations (PR #116964)
paschalis-mpeis wrote: - force-push to stack this PR on top of #127812. - add code & test to ensure that we skip pending relocations only when `-force-patch` was set https://github.com/llvm/llvm-project/pull/116964 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [BOLT] Skip out-of-range pending relocations (PR #116964)
https://github.com/paschalis-mpeis edited https://github.com/llvm/llvm-project/pull/116964 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [BOLT] Skip out-of-range pending relocations (PR #116964)
https://github.com/paschalis-mpeis edited https://github.com/llvm/llvm-project/pull/116964 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [BOLT] Skip out-of-range pending relocations (PR #116964)
paschalis-mpeis wrote: Forced-pushed to add the missing code. Also, this PR on now stacked top of #123635. Thanks for the comments @maks. I am not sure if your [concern](https://github.com/llvm/llvm-project/issues/116817#issuecomment-2602866672) on the issue still stands or not. https://github.com/llvm/llvm-project/pull/116964 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [BOLT] Skip out-of-range pending relocations (PR #116964)
@@ -165,11 +165,17 @@ void
BinarySection::flushPendingRelocations(raw_pwrite_stream &OS,
OS.pwrite(Patch.Bytes.data(), Patch.Bytes.size(),
SectionFileOffset + Patch.Offset);
+ uint64_t SkippedPendingRelocations = 0;
for (Relocation &Reloc : PendingRelocations) {
uint64_t Value = Reloc.Addend;
if (Reloc.Symbol)
Value += Resolver(Reloc.Symbol);
+if (!Relocation::canEncodeValue(Reloc.Type, Value,
paschalis-mpeis wrote:
`addPendingRelocation` now becomes the only way now to add a pending relocation
(Parent PR #123635). If it comes from the `scanExternalRefs` optimization, then
it is marked as optional. Finally, those relocations can be safely skipped when
it's time to flush them.
https://github.com/llvm/llvm-project/pull/116964
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [BOLT] Skip out-of-range pending relocations (PR #116964)
llvmbot wrote:
@llvm/pr-subscribers-bolt
Author: Paschalis Mpeis (paschalis-mpeis)
Changes
When a pending relocation is created it is also marked whether it is
optional or not. It can be optional when such relocation is added as
part of an optimization (i.e., `scanExternalRefs`).
When bolt tries to `flushPendingRelocations`, it safely skips any
optional relocations that cannot be encoded.
Background:
BOLT, as part of scanExternalRefs, identifies external references from
calls and creates some pending relocations for them. Those when flushed
will update references to point to the optimized functions. This
optimization can be disabled using `--no-scan`.
BOLT can assert if any of these pending relocations cannot be encoded.
This patch does not disable this optimization but instead selectively
applies it given that a pending relocation is optional.
---
# Stacked PR on top of:
- #123635
---
Full diff: https://github.com/llvm/llvm-project/pull/116964.diff
6 Files Affected:
- (modified) bolt/include/bolt/Core/BinarySection.h (+8-5)
- (modified) bolt/include/bolt/Core/Relocation.h (+4-1)
- (modified) bolt/lib/Core/BinaryFunction.cpp (+1-1)
- (modified) bolt/lib/Core/BinarySection.cpp (+13-1)
- (modified) bolt/lib/Core/Relocation.cpp (+33)
- (modified) bolt/unittests/Core/BinaryContext.cpp (+32)
``diff
diff --git a/bolt/include/bolt/Core/BinarySection.h
b/bolt/include/bolt/Core/BinarySection.h
index dedee361882497..37d0932a5c142e 100644
--- a/bolt/include/bolt/Core/BinarySection.h
+++ b/bolt/include/bolt/Core/BinarySection.h
@@ -66,8 +66,10 @@ class BinarySection {
// from the original section address.
RelocationSetType DynamicRelocations;
- // Pending relocations for this section.
- std::vector PendingRelocations;
+ /// Pending relocations for this section and whether they are optional, i.e.,
+ /// added as part of an optimization. In that case they can be safely omitted
+ /// if flushPendingRelocations discovers they cannot be encoded.
+ std::vector> PendingRelocations;
struct BinaryPatch {
uint64_t Offset;
@@ -374,9 +376,10 @@ class BinarySection {
DynamicRelocations.emplace(Reloc);
}
- /// Add relocation against the original contents of this section.
- void addPendingRelocation(const Relocation &Rel) {
-PendingRelocations.push_back(Rel);
+ /// Add relocation against the original contents of this section. When added
+ /// as part of an optimization it is marked as \p Optional.
+ void addPendingRelocation(const Relocation &Rel, bool Optional = false) {
+PendingRelocations.push_back({Rel, Optional});
}
/// Add patch to the input contents of this section.
diff --git a/bolt/include/bolt/Core/Relocation.h
b/bolt/include/bolt/Core/Relocation.h
index 933f62a31f8fd7..177cc0c70431f4 100644
--- a/bolt/include/bolt/Core/Relocation.h
+++ b/bolt/include/bolt/Core/Relocation.h
@@ -64,9 +64,12 @@ struct Relocation {
/// and \P Type mismatch occurred.
static bool skipRelocationProcess(uint64_t &Type, uint64_t Contents);
- // Adjust value depending on relocation type (make it PC relative or not)
+ /// Adjust value depending on relocation type (make it PC relative or not).
static uint64_t encodeValue(uint64_t Type, uint64_t Value, uint64_t PC);
+ /// Return true if there are enough bits to encode the relocation value.
+ static bool canEncodeValue(uint64_t Type, uint64_t Value, uint64_t PC);
+
/// Extract current relocated value from binary contents. This is used for
/// RISC architectures where values are encoded in specific bits depending
/// on the relocation value. For X86, we limit to sign extending the value
diff --git a/bolt/lib/Core/BinaryFunction.cpp b/bolt/lib/Core/BinaryFunction.cpp
index 5da777411ba7a1..5d1e5ca92ca131 100644
--- a/bolt/lib/Core/BinaryFunction.cpp
+++ b/bolt/lib/Core/BinaryFunction.cpp
@@ -1672,7 +1672,7 @@ bool BinaryFunction::scanExternalRefs() {
// Add relocations unless disassembly failed for this function.
if (!DisassemblyFailed)
for (Relocation &Rel : FunctionRelocations)
- getOriginSection()->addPendingRelocation(Rel);
+ getOriginSection()->addPendingRelocation(Rel, /*Optional*/ true);
// Inform BinaryContext that this function symbols will not be defined and
// relocations should not be created against them.
diff --git a/bolt/lib/Core/BinarySection.cpp b/bolt/lib/Core/BinarySection.cpp
index 9ad49ca1b3a038..a37cc7603df285 100644
--- a/bolt/lib/Core/BinarySection.cpp
+++ b/bolt/lib/Core/BinarySection.cpp
@@ -165,11 +165,19 @@ void
BinarySection::flushPendingRelocations(raw_pwrite_stream &OS,
OS.pwrite(Patch.Bytes.data(), Patch.Bytes.size(),
SectionFileOffset + Patch.Offset);
- for (Relocation &Reloc : PendingRelocations) {
+ uint64_t SkippedPendingRelocations = 0;
+ for (auto &[Reloc, Optional] : PendingRelocations) {
uint64_t Value = Reloc.Addend;
if (Reloc.Symbol)
Value += Resolver(Reloc.Symbol);
+//
[llvm-branch-commits] [llvm] [BOLT] Skip out-of-range pending relocations (PR #116964)
https://github.com/paschalis-mpeis ready_for_review https://github.com/llvm/llvm-project/pull/116964 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [BOLT] Skip out-of-range pending relocations (PR #116964)
https://github.com/paschalis-mpeis edited https://github.com/llvm/llvm-project/pull/116964 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [BOLT] Skip out-of-range pending relocations (PR #116964)
https://github.com/paschalis-mpeis edited https://github.com/llvm/llvm-project/pull/116964 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
