write/macho: Reverse the order of emitting relocations (#702)

This prevents a crash of Apple's new linker.
gimli-rs · Jun 28, 2024 · 2a297a5 · 2a297a5
1 parent d892da2
commit 2a297a5
Show file tree

Hide file tree

Showing 3 changed files with 51 additions and 27 deletions.
diff --git a/src/write/macho.rs b/src/write/macho.rs
@@ -729,7 +729,8 @@ impl<'a> Object<'a> {
             if !section.relocations.is_empty() {
                 write_align(buffer, pointer_align);
                 debug_assert_eq!(section_offsets[index].reloc_offset, buffer.len());
-                for reloc in &section.relocations {
+
+                let mut write_reloc = |reloc: &Relocation| {
                     let (r_type, r_pcrel, r_length) = if let RelocationFlags::MachO {
                         r_type,
                         r_pcrel,
@@ -783,6 +784,29 @@ impl<'a> Object<'a> {
                         r_type,
                     };
                     buffer.write(&reloc_info.relocation(endian));
+                    Ok(())
+                };
+
+                // Relocations are emitted in descending order as otherwise Apple's
+                // new linker crashes. This matches LLVM's behavior too:
+                // https://github.com/llvm/llvm-project/blob/e9b8cd0c8/llvm/lib/MC/MachObjectWriter.cpp#L1001-L1002
+                let need_reverse = |relocs: &[Relocation]| {
+                    let Some(first) = relocs.first() else {
+                        return false;
+                    };
+                    let Some(last) = relocs.last() else {
+                        return false;
+                    };
+                    first.offset < last.offset
+                };
+                if need_reverse(&section.relocations) {
+                    for reloc in section.relocations.iter().rev() {
+                        write_reloc(reloc)?;
+                    }
+                } else {
+                    for reloc in &section.relocations {
+                        write_reloc(reloc)?;
+                    }
                 }
             }
         }

diff --git a/tests/round_trip/mod.rs b/tests/round_trip/mod.rs
@@ -454,27 +454,27 @@ fn macho_x86_64() {
 
     let (offset, relocation) = relocations.next().unwrap();
     println!("{:?}", relocation);
-    assert_eq!(offset, 8);
-    assert_eq!(relocation.kind(), RelocationKind::Absolute);
-    assert_eq!(relocation.encoding(), RelocationEncoding::Generic);
-    assert_eq!(relocation.size(), 64);
+    assert_eq!(offset, 16);
+    assert_eq!(relocation.kind(), RelocationKind::Relative);
+    assert_eq!(relocation.encoding(), RelocationEncoding::X86RipRelative);
+    assert_eq!(relocation.size(), 32);
     assert_eq!(
         relocation.target(),
         read::RelocationTarget::Symbol(func1_symbol)
     );
-    assert_eq!(relocation.addend(), 0);
+    assert_eq!(relocation.addend(), -4);
 
     let (offset, relocation) = relocations.next().unwrap();
     println!("{:?}", relocation);
-    assert_eq!(offset, 16);
-    assert_eq!(relocation.kind(), RelocationKind::Relative);
-    assert_eq!(relocation.encoding(), RelocationEncoding::X86RipRelative);
-    assert_eq!(relocation.size(), 32);
+    assert_eq!(offset, 8);
+    assert_eq!(relocation.kind(), RelocationKind::Absolute);
+    assert_eq!(relocation.encoding(), RelocationEncoding::Generic);
+    assert_eq!(relocation.size(), 64);
     assert_eq!(
         relocation.target(),
         read::RelocationTarget::Symbol(func1_symbol)
     );
-    assert_eq!(relocation.addend(), -4);
+    assert_eq!(relocation.addend(), 0);
 
     let map = object.symbol_map();
     let symbol = map.get(func1_offset + 1).unwrap();
@@ -564,14 +564,6 @@ fn macho_any() {
 
         let mut relocations = data.relocations();
 
-        let (offset, relocation) = relocations.next().unwrap();
-        println!("{:?}", relocation);
-        assert_eq!(offset, 8);
-        assert_eq!(relocation.kind(), RelocationKind::Absolute);
-        assert_eq!(relocation.encoding(), RelocationEncoding::Generic);
-        assert_eq!(relocation.size(), 32);
-        assert_eq!(relocation.addend(), 0);
-
         if arch.address_size().unwrap().bytes() >= 8 {
             let (offset, relocation) = relocations.next().unwrap();
             println!("{:?}", relocation);
@@ -581,6 +573,14 @@ fn macho_any() {
             assert_eq!(relocation.size(), 64);
             assert_eq!(relocation.addend(), 0);
         }
+
+        let (offset, relocation) = relocations.next().unwrap();
+        println!("{:?}", relocation);
+        assert_eq!(offset, 8);
+        assert_eq!(relocation.kind(), RelocationKind::Absolute);
+        assert_eq!(relocation.encoding(), RelocationEncoding::Generic);
+        assert_eq!(relocation.size(), 32);
+        assert_eq!(relocation.addend(), 0);
     }
 }
 

diff --git a/tests/round_trip/tls.rs b/tests/round_trip/tls.rs
@@ -260,49 +260,49 @@ fn macho_x86_64_tls() {
 
     let (offset, relocation) = relocations.next().unwrap();
     println!("{:?}", relocation);
-    assert_eq!(offset, 0);
+    assert_eq!(offset, 40);
     assert_eq!(relocation.kind(), RelocationKind::Absolute);
     assert_eq!(relocation.encoding(), RelocationEncoding::Generic);
     assert_eq!(relocation.size(), 64);
     assert_eq!(
         relocation.target(),
-        read::RelocationTarget::Symbol(tlv_bootstrap_symbol)
+        read::RelocationTarget::Symbol(tls2_init_symbol)
     );
     assert_eq!(relocation.addend(), 0);
 
     let (offset, relocation) = relocations.next().unwrap();
     println!("{:?}", relocation);
-    assert_eq!(offset, 16);
+    assert_eq!(offset, 24);
     assert_eq!(relocation.kind(), RelocationKind::Absolute);
     assert_eq!(relocation.encoding(), RelocationEncoding::Generic);
     assert_eq!(relocation.size(), 64);
     assert_eq!(
         relocation.target(),
-        read::RelocationTarget::Symbol(tls1_init_symbol)
+        read::RelocationTarget::Symbol(tlv_bootstrap_symbol)
     );
     assert_eq!(relocation.addend(), 0);
 
     let (offset, relocation) = relocations.next().unwrap();
     println!("{:?}", relocation);
-    assert_eq!(offset, 24);
+    assert_eq!(offset, 16);
     assert_eq!(relocation.kind(), RelocationKind::Absolute);
     assert_eq!(relocation.encoding(), RelocationEncoding::Generic);
     assert_eq!(relocation.size(), 64);
     assert_eq!(
         relocation.target(),
-        read::RelocationTarget::Symbol(tlv_bootstrap_symbol)
+        read::RelocationTarget::Symbol(tls1_init_symbol)
     );
     assert_eq!(relocation.addend(), 0);
 
     let (offset, relocation) = relocations.next().unwrap();
     println!("{:?}", relocation);
-    assert_eq!(offset, 40);
+    assert_eq!(offset, 0);
     assert_eq!(relocation.kind(), RelocationKind::Absolute);
     assert_eq!(relocation.encoding(), RelocationEncoding::Generic);
     assert_eq!(relocation.size(), 64);
     assert_eq!(
         relocation.target(),
-        read::RelocationTarget::Symbol(tls2_init_symbol)
+        read::RelocationTarget::Symbol(tlv_bootstrap_symbol)
     );
     assert_eq!(relocation.addend(), 0);
 }