REL section alignment fixes

- Honor section alignment from splits.txt when writing RELs
- Better heuristic for determining REL section alignment on initial analysis

Cargo.lock

@@ -331,7 +331,7 @@ dependencies = [
 
 [[package]]
 name = "decomp-toolkit"
-version = "0.5.3"
+version = "0.5.4"
 dependencies = [
  "anyhow",
  "ar",

Cargo.toml

@@ -3,7 +3,7 @@ name = "decomp-toolkit"
 description = "Yet another GameCube/Wii decompilation toolkit."
 authors = ["Luke Street <luke@street.dev>"]
 license = "MIT OR Apache-2.0"
-version = "0.5.3"
+version = "0.5.4"
 edition = "2021"
 publish = false
 build = "build.rs"

src/cmd/dol.rs

@@ -48,7 +48,7 @@ use crate::{
         file::{buf_reader, buf_writer, map_file, touch, verify_hash, FileIterator},
         lcf::{asm_path_for_unit, generate_ldscript, obj_path_for_unit},
         map::apply_map_file,
-        rel::{process_rel, process_rel_header},
+        rel::{process_rel, process_rel_header, update_rel_section_alignment},
         rso::{process_rso, DOL_SECTION_ABS, DOL_SECTION_NAMES},
         split::{is_linker_generated_object, split_obj, update_splits},
         IntoCow, ToCow,
@@ -856,7 +856,7 @@ fn load_analyze_rel(config: &ProjectConfig, module_config: &ModuleConfig) -> Res
     if let Some(hash_str) = &module_config.hash {
         verify_hash(file.as_slice(), hash_str)?;
     }
-    let (_, mut module_obj) =
+    let (header, mut module_obj) =
         process_rel(&mut Cursor::new(file.as_slice()), module_config.name().as_ref())?;
 
     if let Some(comment_version) = config.mw_comment_version {
@@ -895,6 +895,9 @@ fn load_analyze_rel(config: &ProjectConfig, module_config: &ModuleConfig) -> Res
     // Create _ctors and _dtors symbols if missing
     update_ctors_dtors(&mut module_obj)?;
 
+    // Determine REL section alignment
+    update_rel_section_alignment(&mut module_obj, &header)?;
+
     Ok((module_obj, dep))
 }
 

src/cmd/rel.rs

@@ -30,7 +30,7 @@ use crate::{
     cmd::dol::{ModuleConfig, ProjectConfig},
     obj::{ObjInfo, ObjReloc, ObjRelocKind, ObjSection, ObjSectionKind, ObjSymbol},
     util::{
-        config::is_auto_symbol,
+        config::{is_auto_symbol, read_splits_sections, SectionDef},
         dol::process_dol,
         elf::{to_obj_reloc_kind, write_elf},
         file::{buf_reader, buf_writer, map_file, process_rsp, verify_hash, FileIterator},
@@ -153,7 +153,7 @@ fn match_section_index(
     //     })
 }
 
-fn load_rel(module_config: &ModuleConfig) -> Result<(RelHeader, Vec<RelSectionHeader>)> {
+fn load_rel(module_config: &ModuleConfig) -> Result<RelInfo> {
     let file = map_file(&module_config.object)?;
     if let Some(hash_str) = &module_config.hash {
         verify_hash(file.as_slice(), hash_str)?;
@@ -161,12 +161,17 @@ fn load_rel(module_config: &ModuleConfig) -> Result<(RelHeader, Vec<RelSectionHe
     let mut reader = file.as_reader();
     let header = process_rel_header(&mut reader)?;
     let sections = process_rel_sections(&mut reader, &header)?;
-    Ok((header, sections))
+    let section_defs = if let Some(splits_path) = &module_config.splits {
+        read_splits_sections(splits_path)?
+    } else {
+        None
+    };
+    Ok((header, sections, section_defs))
 }
 
 fn resolve_relocations(
     module: &File,
-    existing_headers: &BTreeMap<u32, (RelHeader, Vec<RelSectionHeader>)>,
+    existing_headers: &BTreeMap<u32, RelInfo>,
     module_id: usize,
     symbol_map: &FxHashMap<&[u8], (usize, SymbolIndex)>,
     modules: &[(File, PathBuf)],
@@ -177,7 +182,8 @@ fn resolve_relocations(
         if !matches!(section.name(), Ok(name) if PERMITTED_SECTIONS.contains(&name)) {
             continue;
         }
-        let section_index = if let Some((_, sections)) = existing_headers.get(&(module_id as u32)) {
+        let section_index =
+            if let Some((_, sections, _)) = existing_headers.get(&(module_id as u32)) {
                 match_section_index(module, section.index(), sections)?
             } else {
                 section.index().0
@@ -208,7 +214,7 @@ fn resolve_relocations(
                 (module_id, reloc_target)
             };
             let target_section_index = target_symbol.section_index().unwrap();
-            let target_section = if let Some((_, sections)) =
+            let target_section = if let Some((_, sections, _)) =
                 existing_headers.get(&(target_module_id as u32))
             {
                 match_section_index(&modules[target_module_id].0, target_section_index, sections)?
@@ -231,19 +237,21 @@ fn resolve_relocations(
     Ok(resolved)
 }
 
+type RelInfo = (RelHeader, Vec<RelSectionHeader>, Option<Vec<SectionDef>>);
+
 fn make(args: MakeArgs) -> Result<()> {
     let total = Instant::now();
 
     // Load existing REL headers (if specified)
-    let mut existing_headers = BTreeMap::<u32, (RelHeader, Vec<RelSectionHeader>)>::new();
+    let mut existing_headers = BTreeMap::<u32, RelInfo>::new();
     if let Some(config_path) = &args.config {
         let config: ProjectConfig = serde_yaml::from_reader(&mut buf_reader(config_path)?)?;
         for module_config in &config.modules {
             let _span = info_span!("module", name = %module_config.name()).entered();
-            let (header, sections) = load_rel(module_config).with_context(|| {
+            let info = load_rel(module_config).with_context(|| {
                 format!("While loading REL '{}'", module_config.object.display())
             })?;
-            existing_headers.insert(header.module_id, (header, sections));
+            existing_headers.insert(info.0.module_id, info);
         }
     }
 
@@ -316,14 +324,19 @@ fn make(args: MakeArgs) -> Result<()> {
             bss_align: None,
             section_count: None,
             quiet: args.no_warn,
+            section_align: None,
         };
-        if let Some((header, _)) = existing_headers.get(&(module_id as u32)) {
+        if let Some((header, _, section_defs)) = existing_headers.get(&(module_id as u32)) {
             info.version = header.version;
             info.name_offset = Some(header.name_offset);
             info.name_size = Some(header.name_size);
             info.align = header.align;
             info.bss_align = header.bss_align;
             info.section_count = Some(header.num_sections as usize);
+            info.section_align = section_defs
+                .as_ref()
+                .map(|defs| defs.iter().map(|def| def.align).collect())
+                .unwrap_or_default();
         }
         let rel_path = path.with_extension("rel");
         let mut w = buf_writer(&rel_path)?;

src/util/config.rs

@@ -390,10 +390,10 @@ struct SplitUnit {
     comment_version: Option<u8>,
 }
 
-struct SectionDef {
+pub struct SectionDef {
-    name: String,
+    pub name: String,
-    kind: Option<ObjSectionKind>,
+    pub kind: Option<ObjSectionKind>,
-    align: Option<u32>,
+    pub align: Option<u32>,
 }
 
 enum SplitLine {
@@ -626,3 +626,42 @@ pub fn apply_splits<R: BufRead>(r: R, obj: &mut ObjInfo) -> Result<()> {
     }
     Ok(())
 }
+
+pub fn read_splits_sections<P: AsRef<Path>>(path: P) -> Result<Option<Vec<SectionDef>>> {
+    if !path.as_ref().is_file() {
+        return Ok(None);
+    }
+    let file = map_file(path)?;
+    let r = file.as_reader();
+    let mut sections = Vec::new();
+    let mut state = SplitState::None;
+    for result in r.lines() {
+        let line = match result {
+            Ok(line) => line,
+            Err(e) => return Err(e.into()),
+        };
+        let split_line = parse_split_line(&line, &state)?;
+        match (&mut state, split_line) {
+            (SplitState::None | SplitState::Unit(_), SplitLine::SectionsStart) => {
+                state = SplitState::Sections(0);
+            }
+            (SplitState::Sections(index), SplitLine::Section(def)) => {
+                sections.push(def);
+                *index += 1;
+            }
+            (SplitState::Sections(_), SplitLine::None) => {
+                // Continue
+            }
+            (SplitState::Sections(_), _) => {
+                // End of sections
+                break;
+            }
+            _ => {}
+        }
+    }
+    if sections.is_empty() {
+        Ok(None)
+    } else {
+        Ok(Some(sections))
+    }
+}

src/util/lcf.rs

@@ -4,10 +4,10 @@ use anyhow::{bail, Result};
 use itertools::Itertools;
 use path_slash::PathBufExt;
 
-use crate::obj::{ObjInfo, ObjKind};
+use crate::{
-
+    obj::{ObjInfo, ObjKind},
-#[inline]
+    util::align_up,
-const fn align_up(value: u32, align: u32) -> u32 { (value + (align - 1)) & !(align - 1) }
+};
 
 const LCF_TEMPLATE: &str = include_str!("../../assets/ldscript.lcf");
 const LCF_PARTIAL_TEMPLATE: &str = include_str!("../../assets/ldscript_partial.lcf");

src/util/mod.rs

@@ -18,6 +18,9 @@ pub mod signatures;
 pub mod split;
 pub mod yaz0;
 
+#[inline]
+pub const fn align_up(value: u32, align: u32) -> u32 { (value + (align - 1)) & !(align - 1) }
+
 /// Creates a fixed-size array reference from a slice.
 #[macro_export]
 macro_rules! array_ref {

src/util/rel.rs

@@ -15,7 +15,7 @@ use crate::{
         ObjArchitecture, ObjInfo, ObjKind, ObjRelocKind, ObjSection, ObjSectionKind, ObjSymbol,
         ObjSymbolFlagSet, ObjSymbolFlags, ObjSymbolKind,
     },
-    util::IntoCow,
+    util::{align_up, split::default_section_align, IntoCow},
 };
 
 /// Do not relocate anything, but accumulate the offset field for the next relocation offset calculation.
@@ -198,7 +198,7 @@ pub fn process_rel<R: Read + Seek>(reader: &mut R, name: &str) -> Result<(RelHea
             data,
             align: match offset {
                 0 => header.bss_align,
-                _ => header.align,
+                _ => None, // determined later
             }
             .unwrap_or_default() as u64,
             elf_index: idx,
@@ -426,14 +426,19 @@ pub struct RelWriteInfo {
     pub section_count: Option<usize>,
     /// If true, don't print warnings about overriding values.
     pub quiet: bool,
+    /// Override individual section alignment in the file.
+    pub section_align: Option<Vec<u32>>,
 }
 
 pub const PERMITTED_SECTIONS: [&str; 7] =
     [".init", ".text", ".ctors", ".dtors", ".rodata", ".data", ".bss"];
 
-pub fn should_write_section(section: &object::Section) -> bool {
+pub fn is_permitted_section(section: &object::Section) -> bool {
     matches!(section.name(), Ok(name) if PERMITTED_SECTIONS.contains(&name))
-        && section.kind() != object::SectionKind::UninitializedData
+}
+
+pub fn should_write_section(section: &object::Section) -> bool {
+    section.kind() != object::SectionKind::UninitializedData
 }
 
 pub fn write_rel<W: Write>(
@@ -468,7 +473,7 @@ pub fn write_rel<W: Write>(
 
     let mut apply_relocations = vec![];
     relocations.retain(|r| {
-        if !should_write_section(
+        if !is_permitted_section(
             &file.section_by_index(object::SectionIndex(r.original_section as usize)).unwrap(),
         ) {
             return false;
@@ -481,10 +486,34 @@ pub fn write_rel<W: Write>(
         }
     });
 
-    let mut align =
+    /// Get the alignment of a section, checking for overrides.
-        file.sections().filter(should_write_section).map(|s| s.align() as u32).max().unwrap_or(0);
+    /// permitted_section_idx increments whenever a permitted section is encountered,
-    let bss = file.sections().find(|s| s.name() == Ok(".bss"));
+    /// rather than being the raw ELF section index.
-    let mut bss_align = bss.as_ref().map(|s| s.align() as u32).unwrap_or(1);
+    fn section_align(
+        permitted_section_idx: usize,
+        section: &object::Section,
+        info: &RelWriteInfo,
+    ) -> u32 {
+        info.section_align
+            .as_ref()
+            .and_then(|v| v.get(permitted_section_idx))
+            .cloned()
+            .unwrap_or(section.align() as u32)
+    }
+
+    let mut align = file
+        .sections()
+        .filter(is_permitted_section)
+        .enumerate()
+        .map(|(i, s)| section_align(i, &s, info))
+        .max()
+        .unwrap_or(0);
+    let bss = file
+        .sections()
+        .filter(is_permitted_section)
+        .enumerate()
+        .find(|(_, s)| s.name() == Ok(".bss"));
+    let mut bss_align = bss.as_ref().map(|(i, s)| section_align(*i, s, info)).unwrap_or(1);
     let mut num_sections = file.sections().count() as u32;
 
     // Apply overrides
@@ -521,7 +550,7 @@ pub fn write_rel<W: Write>(
         name_offset: info.name_offset.unwrap_or(0),
         name_size: info.name_size.unwrap_or(0),
         version: info.version,
-        bss_size: bss.as_ref().map(|s| s.size() as u32).unwrap_or(0),
+        bss_size: bss.as_ref().map(|(_, s)| s.size() as u32).unwrap_or(0),
         rel_offset: 0,
         imp_offset: 0,
         imp_size: 0,
@@ -538,8 +567,11 @@ pub fn write_rel<W: Write>(
     let mut offset = header.section_info_offset;
     offset += num_sections * 8;
     let section_data_offset = offset;
-    for section in file.sections().filter(should_write_section) {
+    for (idx, section) in file.sections().filter(is_permitted_section).enumerate() {
-        let align = section.align() as u32 - 1;
+        if !should_write_section(&section) {
+            continue;
+        }
+        let align = section_align(idx, &section, info) - 1;
         offset = (offset + align) & !align;
         offset += section.size() as u32;
     }
@@ -649,16 +681,17 @@ pub fn write_rel<W: Write>(
     header.write_be(&mut w)?;
     ensure!(w.stream_position()? as u32 == header.section_info_offset);
     let mut current_data_offset = section_data_offset;
+    let mut permitted_section_idx = 0;
     for section_index in 0..num_sections {
         let Ok(section) = file.section_by_index(object::SectionIndex(section_index as usize))
         else {
             RelSectionHeader::new(0, 0, false).write_be(&mut w)?;
             continue;
         };
-        if matches!(section.name(), Ok(name) if PERMITTED_SECTIONS.contains(&name)) {
+        if is_permitted_section(&section) {
             let mut offset = 0;
-            if section.kind() != object::SectionKind::UninitializedData {
+            if should_write_section(&section) {
-                let align = section.align() as u32 - 1;
+                let align = section_align(permitted_section_idx, &section, info) - 1;
                 current_data_offset = (current_data_offset + align) & !align;
                 offset = current_data_offset;
                 current_data_offset += section.size() as u32;
@@ -669,18 +702,24 @@ pub fn write_rel<W: Write>(
                 section.kind() == object::SectionKind::Text,
             )
             .write_be(&mut w)?;
+            permitted_section_idx += 1;
         } else {
             RelSectionHeader::new(0, 0, false).write_be(&mut w)?;
         }
     }
     ensure!(w.stream_position()? as u32 == section_data_offset);
-    for section in file.sections().filter(should_write_section) {
+    for (idx, section) in file.sections().filter(is_permitted_section).enumerate() {
+        if !should_write_section(&section) {
+            continue;
+        }
+
         fn calculate_padding(position: u64, align: u64) -> u64 {
             let align = align - 1;
             ((position + align) & !align) - position
         }
         let position = w.stream_position()?;
-        w.write_all(&vec![0; calculate_padding(position, section.align()) as usize])?;
+        let align = section_align(idx, &section, info);
+        w.write_all(&vec![0u8; calculate_padding(position, align as u64) as usize])?;
 
         let section_index = section.index().0 as u8;
         let mut section_data = section.uncompressed_data()?;
@@ -704,3 +743,46 @@ pub fn write_rel<W: Write>(
     ensure!(w.stream_position()? as u32 == offset);
     Ok(())
 }
+
+/// Determines REL section alignment based on its file offset.
+pub fn update_rel_section_alignment(obj: &mut ObjInfo, header: &RelHeader) -> Result<()> {
+    let mut last_offset = header.section_info_offset + header.num_sections * 8;
+    for (_, section) in obj.sections.iter_mut() {
+        let prev_offset = last_offset;
+        last_offset = (section.file_offset + section.size) as u32;
+
+        if section.align > 0 {
+            // Already set
+            continue;
+        }
+
+        if section.section_known {
+            // Try the default section alignment for known sections
+            let default_align = default_section_align(section);
+            if align_up(prev_offset, default_align as u32) == section.file_offset as u32 {
+                section.align = default_align;
+                continue;
+            }
+        }
+
+        // Work our way down from the REL header alignment
+        let mut align = header.align.unwrap_or(32);
+        while align >= 4 {
+            if align_up(prev_offset, align) == section.file_offset as u32 {
+                section.align = align as u64;
+                break;
+            }
+            align /= 2;
+        }
+
+        if section.align == 0 {
+            bail!(
+                "Failed to determine alignment for REL section {}: {:#X} -> {:#X}",
+                section.name,
+                prev_offset,
+                section.file_offset
+            );
+        }
+    }
+    Ok(())
+}

src/util/split.rs

@@ -15,7 +15,7 @@ use crate::{
         ObjSplit, ObjSymbol, ObjSymbolFlagSet, ObjSymbolFlags, ObjSymbolKind, ObjSymbolScope,
         ObjUnit,
     },
-    util::comment::MWComment,
+    util::{align_up, comment::MWComment},
 };
 
 /// Create splits for function pointers in the given section.
@@ -644,9 +644,6 @@ fn add_padding_symbols(obj: &mut ObjInfo) -> Result<()> {
     Ok(())
 }
 
-#[inline]
-const fn align_up(value: u32, align: u32) -> u32 { (value + (align - 1)) & !(align - 1) }
-
 #[allow(dead_code)]
 fn trim_split_alignment(obj: &mut ObjInfo) -> Result<()> {
     // For each split, set the end of split to the end of the last symbol in the split.