refactor: Have linker script emit symbol defs directly (#1364)

Author: davidlattimore

libwild/src/elf_writer.rs

@@ -3076,7 +3076,13 @@ fn write_dynamic_symbol_definitions(
                             layout,
                             sym_def.symbol_id,
                             script,
-                        )?;
+                        )
+                        .with_context(|| {
+                            format!(
+                                "Failed to write linker script dynsym: {}",
+                                layout.symbol_debug(sym_def.symbol_id)
+                            )
+                        })?;
                     }
                     FileLayout::Prelude(prelude) => {
                         write_prelude_dynsym(
@@ -3115,14 +3121,13 @@ fn write_linker_script_dynsym(
 
     let info = &script.internal_symbols.symbol_definitions[local_index];
 
-    debug_assert!(
-        !matches!(
-            info.placement,
-            crate::parsing::SymbolPlacement::DefsymSymbol(_)
-                | crate::parsing::SymbolPlacement::DefsymAbsolute(_)
-        ),
-        "Defsym symbols from linker scripts should be emitted via the prelude"
-    );
+    if matches!(
+        info.placement,
+        crate::parsing::SymbolPlacement::DefsymSymbol(_)
+            | crate::parsing::SymbolPlacement::DefsymAbsolute(_)
+    ) {
+        return write_defsym_dynsym(dynsym_writer, layout, symbol_id, info);
+    }
 
     let section_id = info
         .section_id()

libwild/src/layout.rs

@@ -328,38 +328,69 @@ fn update_defsym_symbol_resolutions(
     symbol_db: &SymbolDb,
     resolutions: &mut [Option<Resolution>],
 ) -> Result {
-    if let Some(Group::Prelude(prelude)) = symbol_db.groups.first() {
-        let symbol_id_range = SymbolIdRange::prelude(prelude.symbol_definitions.len());
+    verbose_timing_phase!("Update symdef resolutions");
 
-        for (local_index, def_info) in prelude.symbol_definitions.iter().enumerate() {
-            let SymbolPlacement::DefsymSymbol(target_name) = def_info.placement else {
-                continue;
-            };
+    for group in &symbol_db.groups {
+        let mut symbol_id = group.symbol_id_range().start();
 
-            let symbol_id = symbol_id_range.offset_to_id(local_index);
-            if !symbol_db.is_canonical(symbol_id) {
-                continue;
+        match group {
+            Group::Prelude(prelude) => {
+                for def_info in &prelude.symbol_definitions {
+                    update_defsym_symbol_resolution(symbol_id, def_info, symbol_db, resolutions)?;
+                    symbol_id = symbol_id.next();
+                }
             }
-
-            let Some(target_symbol_id) = symbol_db
-                .get_unversioned(&UnversionedSymbolName::prehashed(target_name.as_bytes()))
-            else {
-                return Err(symbol_db.missing_defsym_target_error(def_info.name, target_name));
-            };
-
-            if let Some(target_value) = resolutions[target_symbol_id.as_usize()]
-                .as_ref()
-                .map(|r| r.raw_value)
-                && let Some(resolution) = &mut resolutions[symbol_id.as_usize()]
-            {
-                resolution.raw_value = target_value;
+            Group::LinkerScripts(scripts) => {
+                for script in scripts {
+                    for def_info in &script.parsed.symbol_defs {
+                        update_defsym_symbol_resolution(
+                            symbol_id,
+                            def_info,
+                            symbol_db,
+                            resolutions,
+                        )?;
+                        symbol_id = symbol_id.next();
+                    }
+                }
             }
+            Group::Objects(_) | Group::Epilogue(_) => {}
         }
     }
 
     Ok(())
 }
 
+fn update_defsym_symbol_resolution(
+    symbol_id: SymbolId,
+    def_info: &InternalSymDefInfo,
+    symbol_db: &SymbolDb,
+    resolutions: &mut [Option<Resolution>],
+) -> Result {
+    let SymbolPlacement::DefsymSymbol(target_name) = def_info.placement else {
+        return Ok(());
+    };
+
+    if !symbol_db.is_canonical(symbol_id) {
+        return Ok(());
+    }
+
+    let Some(target_symbol_id) =
+        symbol_db.get_unversioned(&UnversionedSymbolName::prehashed(target_name.as_bytes()))
+    else {
+        return Err(symbol_db.missing_defsym_target_error(def_info.name, target_name));
+    };
+
+    if let Some(target_value) = resolutions[target_symbol_id.as_usize()]
+        .as_ref()
+        .map(|r| r.raw_value)
+        && let Some(resolution) = &mut resolutions[symbol_id.as_usize()]
+    {
+        resolution.raw_value = target_value;
+    }
+
+    Ok(())
+}
+
 /// Update resolutions for all dynamic symbols that our output file defines.
 fn update_dynamic_symbol_resolutions(
     layouts: &[GroupLayout],
@@ -3400,37 +3431,27 @@ impl<'data> PreludeLayoutState<'data> {
             );
         }
 
-        self.assign_defsym_flags(resources);
+        self.mark_defsyms_as_used(resources);
 
         Ok(())
     }
 
-    fn assign_defsym_flags(&self, resources: &GraphResources) {
-        let needs_dynsym = resources.symbol_db.output_kind.needs_dynsym();
+    /// Mark defsyms from the command-line as being directly referenced so that we emit the symbols
+    /// even if nothing in the code references them.
+    fn mark_defsyms_as_used(&self, resources: &GraphResources) {
         for (index, def_info) in self.internal_symbols.symbol_definitions.iter().enumerate() {
             let symbol_id = self.symbol_id_range.offset_to_id(index);
             if !resources.symbol_db.is_canonical(symbol_id) {
                 continue;
             }
 
             match def_info.placement {
-                SymbolPlacement::DefsymAbsolute(_) => {
+                SymbolPlacement::DefsymAbsolute(_) | SymbolPlacement::DefsymSymbol(_) => {
                     resources
                         .per_symbol_flags
                         .get_atomic(symbol_id)
                         .or_assign(ValueFlags::DIRECT);
                 }
-                SymbolPlacement::DefsymSymbol(_) => {
-                    let flags_to_set = if needs_dynsym {
-                        ValueFlags::DIRECT | ValueFlags::EXPORT_DYNAMIC
-                    } else {
-                        ValueFlags::DIRECT
-                    };
-                    resources
-                        .per_symbol_flags
-                        .get_atomic(symbol_id)
-                        .or_assign(flags_to_set);
-                }
                 _ => {}
             }
         }

libwild/src/parsing.rs

@@ -28,7 +28,7 @@ use rayon::iter::ParallelIterator;
 
 pub(crate) fn parse_input_files<'data>(
     inputs: &[InputBytes<'data>],
-    mut linker_scripts: Vec<ProcessedLinkerScript<'data>>,
+    linker_scripts: Vec<ProcessedLinkerScript<'data>>,
     args: &'data Args,
     output_kind: OutputKind,
 ) -> Result<ParsedInputs<'data>> {
@@ -47,12 +47,7 @@ pub(crate) fn parse_input_files<'data>(
         },
     );
 
-    verbose_timing_phase!("Count symbols");
-
     let objects = objects?;
-    let mut prelude = prelude;
-
-    move_linker_script_defsyms_to_prelude(&mut prelude, &mut linker_scripts);
 
     let num_symbols = count_symbols(&prelude, &objects, &linker_scripts);
 
@@ -262,29 +257,13 @@ impl<'data> Prelude<'data> {
     }
 }
 
-fn move_linker_script_defsyms_to_prelude<'data>(
-    prelude: &mut Prelude<'data>,
-    linker_scripts: &mut [ProcessedLinkerScript<'data>],
-) {
-    for script in linker_scripts {
-        script.symbol_defs.retain(|def| {
-            let is_defsym = matches!(
-                def.placement,
-                SymbolPlacement::DefsymAbsolute(_) | SymbolPlacement::DefsymSymbol(_)
-            );
-            if is_defsym {
-                prelude.symbol_definitions.push(*def);
-            }
-            !is_defsym
-        });
-    }
-}
-
 fn count_symbols(
     prelude: &Prelude,
     objects: &[ParsedInputObject],
     linker_scripts: &[ProcessedLinkerScript],
 ) -> usize {
+    verbose_timing_phase!("Count symbols");
+
     let in_objects = objects.iter().map(|o| o.num_symbols()).sum::<usize>();
 
     let in_linker_scripts = linker_scripts