Merge pull request #7435 from roc-lang/revert-PR7424

Revert "Merge pull request #7424 from kubkon/macho-surgery"

crates/compiler/build/src/link.rs

@@ -1041,13 +1041,14 @@ fn link_macos(
             // "--gc-sections",
             "-arch",
             &arch,
+            // Suppress warnings, because otherwise it prints:
+            //
+            //   ld: warning: -undefined dynamic_lookup may not work with chained fixups
+            //
+            // We can't disable that option without breaking either x64 mac or ARM mac
+            "-w",
             "-macos_version_min",
             &get_macos_version(),
-            // Suppress fixup chains to ease working out dynamic relocs by the
-            // surgical linker. In my experience, working with dyld opcodes is
-            // slightly easier than unpacking compressed info from the __got section
-            // and fixups load command.
-            "-no_fixup_chains",
         ])
         .args(input_paths)
         .args(extra_link_flags());

crates/linker/src/generate_dylib/macho.rs

@@ -62,7 +62,6 @@ pub fn create_dylib_macho(
 
     let ld_flag_soname = "-install_name";
     let ld_prefix_args = [big_sur_fix, "-lSystem", "-dylib"];
-    let macos_version = get_macos_version();
 
     let output = Command::new("ld")
         .args(ld_prefix_args)
@@ -72,15 +71,12 @@ pub fn create_dylib_macho(
             dummy_obj_file.path().to_str().unwrap(),
             "-o",
             dummy_lib_file.to_str().unwrap(),
-            // Suppress fixup chains to ease working out dynamic relocs by the
+            // Suppress warnings, because otherwise it prints:
-            // surgical linker. In my experience, working with dyld opcodes is
+            //
-            // slightly easier than unpacking compressed info from the __got section
+            //   ld: warning: -undefined dynamic_lookup may not work with chained fixups
-            // and fixups load command.
+            //
-            "-no_fixup_chains",
+            // We can't disable that option without breaking either x64 mac or ARM mac
-            "-platform_version",
+            "-w",
-            "macos",
-            &macos_version,
-            &macos_version,
         ])
         .output()
         .unwrap();
@@ -102,23 +98,3 @@ pub fn create_dylib_macho(
 
     Ok(std::fs::read(dummy_lib_file).expect("Failed to load dummy library"))
 }
-
-fn get_macos_version() -> String {
-    let mut cmd = Command::new("sw_vers");
-    cmd.arg("-productVersion");
-
-    let cmd_stdout = cmd
-        .output()
-        .expect("Failed to execute command 'sw_vers -productVersion'")
-        .stdout;
-
-    let full_version_string = String::from_utf8(cmd_stdout)
-        .expect("Failed to convert output of command 'sw_vers -productVersion' into a utf8 string");
-
-    full_version_string
-        .trim_end()
-        .split('.')
-        .take(3)
-        .collect::<Vec<&str>>()
-        .join(".")
-}

crates/linker/src/lib.rs

@@ -35,7 +35,6 @@ pub fn supported(link_type: LinkType, target: Target) -> bool {
             Target::WinX64 => true,
             // macho support is incomplete
             Target::MacX64 => false,
-            Target::MacArm64 => true,
             _ => false,
         }
     } else {

crates/linker/src/macho.rs

@@ -109,19 +109,20 @@ fn collect_roc_definitions<'a>(object: &object::File<'a, &'a [u8]>) -> MutMap<St
     let mut vaddresses = MutMap::default();
 
     for sym in object.symbols().filter(is_roc_definition) {
-        let name = sym.name().unwrap().trim_start_matches('_');
+        // remove potentially trailing "@version".
+        let name = sym
+            .name()
+            .unwrap()
+            .trim_start_matches('_')
+            .split('@')
+            .next()
+            .unwrap();
+
         let address = sym.address();
 
         // special exceptions for memcpy and memset.
-        let direct_mapping = match name {
+        if name == "roc_memset" {
-            "roc_memset" => Some("memset"),
+            vaddresses.insert("memset".to_string(), address);
-            "roc_memmove" => Some("memmove"),
-
-            _ => None,
-        };
-
-        if let Some(libc_symbol) = direct_mapping {
-            vaddresses.insert(libc_symbol.to_string(), address);
         }
 
         vaddresses.insert(name.to_string(), address);
@@ -1207,7 +1208,6 @@ fn surgery_macho_help(
     let rodata_sections: Vec<Section> = app_obj
         .sections()
         .filter(|sec| sec.kind() == SectionKind::ReadOnlyData)
-        .filter(|sec| sec.name().unwrap_or("") != "__eh_frame") // ignore __eh_frame for now
         .collect();
 
     // bss section is like rodata section, but it has zero file size and non-zero virtual size.
@@ -1224,15 +1224,6 @@ fn surgery_macho_help(
         internal_error!("No text sections found. This application has no code.");
     }
 
-    if verbose {
-        println!();
-        println!("Roc symbol addresses: {:+x?}", md.roc_symbol_vaddresses);
-        println!("App functions: {:?}", md.app_functions);
-        println!("Dynamic symbol indices: {:+x?}", md.dynamic_symbol_indices);
-        println!("PLT addresses: {:+x?}", md.plt_addresses);
-        println!();
-    }
-
     // Calculate addresses and load symbols.
     // Note, it is important the bss sections come after the rodata sections.
     for sec in rodata_sections
@@ -1249,7 +1240,7 @@ fn surgery_macho_help(
                 sec.name().unwrap(),
                 offset,
                 virt_offset
-            );
+            )
         }
         section_offset_map.insert(sec.index(), (offset, virt_offset));
         for sym in symbols.iter() {
@@ -1268,7 +1259,7 @@ fn surgery_macho_help(
             Some((_, size)) => size,
             None => 0,
         };
-        if sec.name().unwrap_or_default().starts_with("__bss") {
+        if sec.name().unwrap_or_default().starts_with("__BSS") {
             // bss sections only modify the virtual size.
             virt_offset += sec.size() as usize;
         } else if section_size != sec.size() {
@@ -1295,137 +1286,98 @@ fn surgery_macho_help(
     for sec in rodata_sections
         .iter()
         .chain(bss_sections.iter())
-        // TODO why do we even include uninitialized data if it cannot
-        // ever have any relocations in the first place?
         .chain(text_sections.iter())
     {
-        let data = sec.data().unwrap_or_else(|err| {
+        let data = match sec.data() {
-            internal_error!(
+            Ok(data) => data,
-                "Failed to load data for section, {:+x?}: {err}",
+            Err(err) => {
-                sec.name().unwrap(),
+                internal_error!(
-            );
+                    "Failed to load data for section, {:+x?}: {}",
-        });
+                    sec.name().unwrap(),
+                    err
+                );
+            }
+        };
         let (section_offset, section_virtual_offset) =
             section_offset_map.get(&sec.index()).unwrap();
         let (section_offset, section_virtual_offset) = (*section_offset, *section_virtual_offset);
         exec_mmap[section_offset..section_offset + data.len()].copy_from_slice(data);
         // Deal with definitions and relocations for this section.
         if verbose {
-            let segname = sec
-                .segment_name()
-                .expect(
-                    "valid segment
-                    name",
-                )
-                .unwrap();
-            let sectname = sec.name().unwrap();
             println!();
             println!(
-                "Processing Relocations for Section '{segname},{sectname}': 0x{sec:+x?} @ {section_offset:+x} (virt: {section_virtual_offset:+x})"
+                "Processing Relocations for Section: 0x{sec:+x?} @ {section_offset:+x} (virt: {section_virtual_offset:+x})"
             );
         }
-
-        let mut subtractor: Option<SymbolIndex> = None;
         for rel in sec.relocations() {
             if verbose {
                 println!("\tFound Relocation: {rel:+x?}");
             }
             match rel.1.target() {
                 RelocationTarget::Symbol(index) => {
-                    let target_offset = if let Some(target_offset) =
+                    let target_offset = if let Some(target_offset) = symbol_vaddr_map.get(&index) {
-                        get_target_offset(index, &app_obj, md, &symbol_vaddr_map, verbose)
+                        if verbose {
-                    {
+                            println!("\t\tRelocation targets symbol in app at: {target_offset:+x}");
-                        target_offset
+                        }
-                    } else if matches!(app_obj.symbol_by_index(index), Ok(sym) if ["__divti3", "__udivti3", "___divti3", "___udivti3"].contains(&sym.name().unwrap_or_default()))
+                        Some(*target_offset as i64)
-                    {
+                    } else {
-                        // Explicitly ignore some symbols that are currently always linked.
+                        app_obj
-                        continue;
-                    } else if matches!(app_obj.symbol_by_index(index), Ok(sym) if ["_longjmp", "_setjmp"].contains(&sym.name().unwrap_or_default()))
-                    {
-                        // These symbols have to stay undefined as we dynamically link them from libSystem.dylib at runtime.
-                        // TODO have a table of all known symbols; perhaps parse and use an Apple provided libSystem.tbd stub file?
-                        let name = app_obj
                             .symbol_by_index(index)
                             .and_then(|sym| sym.name())
                             .ok()
-                            .unwrap();
+                            .and_then(|name| {
-                        match rel.1.kind() {
+                                md.roc_symbol_vaddresses.get(name).map(|address| {
-                            RelocationKind::PltRelative => {
+                                    let vaddr = (*address + md.added_byte_count) as i64;
-                                println!("\t\tTODO synthesise __stub entry for {name}")
+                                    if verbose {
+                                        println!(
+                                            "\t\tRelocation targets symbol in host: {name} @ {vaddr:+x}"
+                                        );
+                                    }
+                                    vaddr
+                                })
+                            })
+                    };
+
+                    if let Some(target_offset) = target_offset {
+                        let virt_base = section_virtual_offset + rel.0 as usize;
+                        let base = section_offset + rel.0 as usize;
+                        let target: i64 = match rel.1.kind() {
+                            RelocationKind::Relative | RelocationKind::PltRelative => {
+                                target_offset - virt_base as i64 + rel.1.addend()
                             }
-                            RelocationKind::Got => {
+                            x => {
-                                println!("\t\tTODO synthesise __got entry for {name}")
+                                internal_error!("Relocation Kind not yet support: {:?}", x);
                             }
-                            _ => internal_error!(
+                        };
-                                "Invalid relocation for libc symbol, {:+x?}: {name}",
+                        if verbose {
-                                rel
+                            println!(
-                            ),
+                                "\t\tRelocation base location: {base:+x} (virt: {virt_base:+x})"
+                            );
+                            println!("\t\tFinal relocation target offset: {target:+x}");
                         }
+                        match rel.1.size() {
+                            32 => {
+                                let data = (target as i32).to_le_bytes();
+                                exec_mmap[base..base + 4].copy_from_slice(&data);
+                            }
+                            64 => {
+                                let data = target.to_le_bytes();
+                                exec_mmap[base..base + 8].copy_from_slice(&data);
+                            }
+                            x => {
+                                internal_error!("Relocation size not yet supported: {}", x);
+                            }
+                        }
+                    } else if matches!(app_obj.symbol_by_index(index), Ok(sym) if ["__divti3", "__udivti3", "___divti3", "___udivti3"].contains(&sym.name().unwrap_or_default()))
+                    {
+                        // Explicitly ignore some symbols that are currently always linked.
                         continue;
                     } else {
                         internal_error!(
                             "Undefined Symbol in relocation, {:+x?}: {:+x?}",
                             rel,
                             app_obj.symbol_by_index(index)
-                        )
+                        );
-                    };
-
-                    let virt_base = section_virtual_offset + rel.0 as usize;
-                    let base = section_offset + rel.0 as usize;
-                    let target: i64 = match rel.1.kind() {
-                        RelocationKind::Relative | RelocationKind::PltRelative => {
-                            target_offset - virt_base as i64 + rel.1.addend()
-                        }
-                        RelocationKind::Absolute => {
-                            target_offset + rel.1.addend()
-                                - subtractor
-                                    .take()
-                                    .map(|index| {
-                                        get_target_offset(
-                                            index,
-                                            &app_obj,
-                                            md,
-                                            &symbol_vaddr_map,
-                                            verbose,
-                                        )
-                                        .unwrap_or(0)
-                                    })
-                                    .unwrap()
-                        }
-                        RelocationKind::MachO { value, relative: _ } => match value {
-                            macho::ARM64_RELOC_SUBTRACTOR => {
-                                if subtractor.is_some() {
-                                    internal_error!("Malformed object: SUBTRACTOR must not be followed by SUBTRACTOR");
-                                } else {
-                                    subtractor = Some(index);
-                                }
-                                continue;
-                            }
-                            _ => {
-                                println!("\t\tHandle other MachO relocs: {value}");
-                                0
-                            }
-                        },
-                        x => {
-                            internal_error!("Relocation Kind not yet support: {:?}", x);
-                        }
-                    };
-                    if verbose {
-                        println!("\t\tRelocation base location: {base:+x} (virt: {virt_base:+x})");
-                        println!("\t\tFinal relocation target offset: {target:+x}");
-                    }
-                    match rel.1.size() {
-                        32 => {
-                            let data = (target as i32).to_le_bytes();
-                            exec_mmap[base..base + 4].copy_from_slice(&data);
-                        }
-                        64 => {
-                            let data = target.to_le_bytes();
-                            exec_mmap[base..base + 8].copy_from_slice(&data);
-                        }
-                        x => {
-                            internal_error!("Relocation size not yet supported: {}", x);
-                        }
                     }
                 }
 
@@ -1626,32 +1578,3 @@ fn surgery_macho_help(
 
     *offset_ref = offset;
 }
-
-fn get_target_offset(
-    index: SymbolIndex,
-    app_obj: &object::File,
-    md: &Metadata,
-    symbol_vaddr_map: &MutMap<SymbolIndex, usize>,
-    verbose: bool,
-) -> Option<i64> {
-    if let Some(target_offset) = symbol_vaddr_map.get(&index) {
-        if verbose {
-            println!("\t\tRelocation targets symbol in app at: {target_offset:+x}");
-        }
-        Some(*target_offset as i64)
-    } else {
-        app_obj
-            .symbol_by_index(index)
-            .and_then(|sym| sym.name().map(|name| name.trim_start_matches('_')))
-            .ok()
-            .and_then(|name| {
-                md.roc_symbol_vaddresses.get(name).map(|address| {
-                    let vaddr = (*address + md.added_byte_count) as i64;
-                    if verbose {
-                        println!("\t\tRelocation targets symbol in host: {name} @ {vaddr:+x}");
-                    }
-                    vaddr
-                })
-            })
-    }
-}