Skip to content

Handle empty .got in linker-diff#831

Merged
davidlattimore merged 3 commits into
wild-linker:mainfrom
Noratrieb:linker-diff-empty-got
Jun 20, 2025
Merged

Handle empty .got in linker-diff#831
davidlattimore merged 3 commits into
wild-linker:mainfrom
Noratrieb:linker-diff-empty-got

Conversation

@Noratrieb
Copy link
Copy Markdown
Contributor

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.

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 in GotIndex.

fixes #830

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`.
@davidlattimore
Copy link
Copy Markdown
Member

Thanks for the PR! as_chunks looks like a neat API. There's still 3 weeks until 1.88. Any interest in merging this sooner? Two alternatives come to mind. Either we could allow unaligned bytes only for the empty case:

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 {

@davidlattimore
Copy link
Copy Markdown
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 as_chunks later once it's stable. Thanks again for the PR!

@davidlattimore davidlattimore merged commit e592208 into wild-linker:main Jun 20, 2025
26 of 27 checks passed
@Noratrieb Noratrieb deleted the linker-diff-empty-got branch June 20, 2025 07:01
@Noratrieb
Copy link
Copy Markdown
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

linker-diff can't handle empty .got

2 participants