Handle empty .got in linker-diff#831
Merged
davidlattimore merged 3 commits intoJun 20, 2025
Merged
Conversation
If `.got` is empty, the slice may not be aligned since `object` will just return a `&[]` for empty sections. Avoid depending on its alignment by just using the newly stabilized `as_chunks`.
Member
|
Thanks for the PR! diff --git a/linker-diff/src/asm_diff.rs b/linker-diff/src/asm_diff.rs
index 291d000051..edc0c4361e 100644
--- a/linker-diff/src/asm_diff.rs
+++ b/linker-diff/src/asm_diff.rs
@@ -3140,13 +3140,15 @@
};
let data = got_section.data()?;
- // Ensure we don't try to cast it to `[u64]`, as it may not be aligned.
- let (raw_entries, tail) = data.as_chunks::<{ size_of::<u64>() }>();
-
- ensure!(
- tail.is_empty(),
- "GOT table `{table_name}` does not have an aligned length"
- );
+ let raw_entries: &[u64] = if data.is_empty() {
+ // An empty .got may not be aligned, so we avoid calling object::slice_from_bytes.
+ &[]
+ } else {
+ let entry_size = size_of::<u64>();
+ object::slice_from_bytes(data, data.len() / entry_size)
+ .unwrap()
+ .0
+ };
let base = got_section.address();
Ok(Some(GotIndex {
@@ -3270,7 +3272,7 @@
/// The addresses covered by this table.
address_range: Range<u64>,
- entries: &'data [[u8; size_of::<u64>()]],
+ entries: &'data [u64],
}
impl<'data> GotIndex<'data> {
@@ -3363,11 +3365,10 @@
}
} else {
// No dynamic relocation, just read from the original file data.
- let raw_value = self
+ let raw_value = *self
.entries
.get((offset / entry_size) as usize)
.context("got_address past end of index range")?;
- let raw_value = u64::from_le_bytes(*raw_value);
match relocation_kind {
RelocationKind::GotTpOff
GOT entries should always be aligned, so an unaligned non-empty GOT likely means a linker bug. But perhaps we want Wild to handle it gracefully anyway, in that case we could keep the got as bytes until we go to read an entry: diff --git a/linker-diff/src/asm_diff.rs b/linker-diff/src/asm_diff.rs
index 291d000051..feb41a2d70 100644
--- a/linker-diff/src/asm_diff.rs
+++ b/linker-diff/src/asm_diff.rs
@@ -3140,18 +3140,11 @@
};
let data = got_section.data()?;
- // Ensure we don't try to cast it to `[u64]`, as it may not be aligned.
- let (raw_entries, tail) = data.as_chunks::<{ size_of::<u64>() }>();
-
- ensure!(
- tail.is_empty(),
- "GOT table `{table_name}` does not have an aligned length"
- );
let base = got_section.address();
Ok(Some(GotIndex {
address_range: base..base + data.len() as u64,
- entries: raw_entries,
+ entries: data,
}))
}
@@ -3270,7 +3263,7 @@
/// The addresses covered by this table.
address_range: Range<u64>,
- entries: &'data [[u8; size_of::<u64>()]],
+ entries: &'data [u8],
}
impl<'data> GotIndex<'data> {
@@ -3365,8 +3358,10 @@
// No dynamic relocation, just read from the original file data.
let raw_value = self
.entries
- .get((offset / entry_size) as usize)
+ .get(offset as usize..)
+ .and_then(|bytes| bytes.first_chunk::<{ size_of::<u64>() }>())
.context("got_address past end of index range")?;
+
let raw_value = u64::from_le_bytes(*raw_value);
match relocation_kind {
|
Member
|
Another PR (just merged) conflicted with this PR. I fixed the merge conflict and then applied the first of the two backport-to-stable options. I'll merge this now. We can always switch back to using |
Contributor
Author
|
thanks, I didn't get to it earlier :) |
RossSmyth
added a commit
to RossSmyth/wild
that referenced
this pull request
Jun 20, 2025
AadiWaghray
pushed a commit
to AadiWaghray/wild
that referenced
this pull request
Aug 10, 2025
If `.got` is empty, the slice may not be aligned since `object` will just return a `&[]` for empty sections. Co-authored-by: David Lattimore <dvdlttmr@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
If
.gotis empty, the slice may not be aligned sinceobjectwill just return a&[]for empty sections.Avoid depending on its alignment by just using the newly stabilized
as_chunks.This requires Rust 1.88 which isn't released yet. But I think we can wait a few weeks with merging this PR 🙃. We could also rewrite it to use
chunks_exact, which would require using a vec instead inGotIndex.fixes #830