Read alignment from Metrowerks .comment section

objdiff-core/src/obj/comment.rs

@@ -0,0 +1,62 @@
+use anyhow::{Result, anyhow};
+
+use crate::util::{read_u8, read_u16, read_u32};
+
+pub const COMMENT_SECTION: &str = ".comment";
+
+const MAGIC: &[u8] = "CodeWarrior".as_bytes();
+const HEADER_SIZE: u8 = 0x2C;
+
+// Partial implementation, does not include all fields
+pub struct MWComment {
+    pub version: u8,
+}
+
+impl MWComment {
+    pub fn from_reader(_obj_file: &object::File, reader: &mut &[u8]) -> Result<Self> {
+        let mut out = MWComment { version: 0 };
+        let magic: [u8; MAGIC.len()] = reader[..MAGIC.len()].try_into()?;
+        *reader = &reader[MAGIC.len()..];
+        if magic != MAGIC {
+            return Err(anyhow!("Invalid .comment section magic: {magic:?}"));
+        }
+        out.version = read_u8(reader)?;
+        if !matches!(out.version, 8 | 10 | 11 | 13 | 14 | 15) {
+            return Err(anyhow!("Unknown .comment section version: {}", out.version));
+        }
+        *reader = &reader[8..];
+        let header_size = read_u8(reader)?;
+        if header_size != HEADER_SIZE {
+            return Err(anyhow!("Expected header size {HEADER_SIZE:#X}, got {header_size:#X}"));
+        }
+        *reader = &reader[0x17..];
+        Ok(out)
+    }
+}
+
+#[derive(Debug, Copy, Clone)]
+pub struct CommentSym {
+    pub align: u32,
+    pub vis_flags: u8,
+    pub active_flags: u8,
+}
+
+impl CommentSym {
+    pub fn from_reader(obj_file: &object::File, reader: &mut &[u8]) -> Result<Self> {
+        let mut out = CommentSym { align: 0, vis_flags: 0, active_flags: 0 };
+        out.align = read_u32(obj_file, reader)?;
+        out.vis_flags = read_u8(reader)?;
+        if !matches!(out.vis_flags, 0 | 0xD | 0xE) {
+            log::warn!("Unknown vis_flags: {:#X}", out.vis_flags);
+        }
+        out.active_flags = read_u8(reader)?;
+        if !matches!(out.active_flags, 0 | 0x8 | 0x10 | 0x20) {
+            log::warn!("Unknown active_flags: {:#X}", out.active_flags);
+        }
+        let padding = read_u16(obj_file, reader)?;
+        if padding != 0 {
+            return Err(anyhow!("Unexpected value after active_flags: {padding:#X}"));
+        }
+        Ok(out)
+    }
+}

objdiff-core/src/obj/mod.rs

@@ -1,3 +1,4 @@
+pub mod comment;
 #[cfg(feature = "dwarf")]
 mod dwarf2;
 mod mdebug;

objdiff-core/src/obj/read.rs

@@ -6,7 +6,10 @@ use alloc::{
     vec,
     vec::Vec,
 };
-use core::{cmp::Ordering, num::NonZeroU64};
+use core::{
+    cmp::Ordering,
+    num::{NonZeroU32, NonZeroU64},
+};
 
 use anyhow::{Context, Result, anyhow, bail, ensure};
 use object::{Architecture, Object as _, ObjectSection as _, ObjectSymbol as _};
@@ -17,6 +20,7 @@ use crate::{
     obj::{
         FlowAnalysisResult, Object, Relocation, RelocationFlags, Section, SectionData, SectionFlag,
         SectionKind, Symbol, SymbolFlag, SymbolFlagSet, SymbolKind,
+        comment::{COMMENT_SECTION, CommentSym, MWComment},
         split_meta::{SPLITMETA_SECTION, SplitMeta},
     },
     util::{align_data_slice_to, align_u64_to, read_u16, read_u32},
@@ -103,6 +107,7 @@ fn map_symbol(
     symbol: &object::Symbol,
     section_indices: &[usize],
     split_meta: Option<&SplitMeta>,
+    comment_syms: Option<&Vec<CommentSym>>,
     config: &DiffObjConfig,
 ) -> Result<Symbol> {
     let mut name = symbol.name().context("Failed to process symbol name")?.to_string();
@@ -156,6 +161,9 @@ fn map_symbol(
     };
     let address = arch.symbol_address(symbol.address(), kind);
     let demangled_name = config.demangler.demangle(&name);
+    // Find the alignment for the symbol if available
+    let comment_sym = comment_syms.map(|vec| vec[symbol.index().0 - 1]);
+    let align = comment_sym.and_then(|c| NonZeroU32::new(c.align));
     // Find the virtual address for the symbol if available
     let virtual_address = split_meta
         .and_then(|m| m.virtual_addresses.as_ref())
@@ -175,7 +183,7 @@ fn map_symbol(
         kind,
         section,
         flags,
-        align: None, // TODO parse .comment
+        align,
         virtual_address,
     })
 }
@@ -185,6 +193,7 @@ fn map_symbols(
     obj_file: &object::File,
     section_indices: &[usize],
     split_meta: Option<&SplitMeta>,
+    comment_syms: Option<&Vec<CommentSym>>,
     config: &DiffObjConfig,
 ) -> Result<(Vec<Symbol>, Vec<usize>)> {
     // symbols() is not guaranteed to be sorted by address.
@@ -219,7 +228,15 @@ fn map_symbols(
     let mut symbols = Vec::<Symbol>::with_capacity(obj_symbols.len() + obj_file.sections().count());
     let mut symbol_indices = vec![usize::MAX; max_index + 1];
     for obj_symbol in obj_symbols {
-        let symbol = map_symbol(arch, obj_file, &obj_symbol, section_indices, split_meta, config)?;
+        let symbol = map_symbol(
+            arch,
+            obj_file,
+            &obj_symbol,
+            section_indices,
+            split_meta,
+            comment_syms,
+            config,
+        )?;
         symbol_indices[obj_symbol.index().0] = symbols.len();
         symbols.push(symbol);
     }
@@ -1071,10 +1088,17 @@ pub fn parse(data: &[u8], config: &DiffObjConfig, diff_side: DiffSide) -> Result
     let obj_file = object::File::parse(data)?;
     let mut arch = new_arch(&obj_file, diff_side)?;
     let split_meta = parse_split_meta(&obj_file)?;
+    let comment_syms = parse_mw_comment_syms(&obj_file)?;
     let (mut sections, section_indices) =
         map_sections(arch.as_ref(), &obj_file, split_meta.as_ref())?;
-    let (mut symbols, symbol_indices) =
-        map_symbols(arch.as_ref(), &obj_file, &section_indices, split_meta.as_ref(), config)?;
+    let (mut symbols, symbol_indices) = map_symbols(
+        arch.as_ref(),
+        &obj_file,
+        &section_indices,
+        split_meta.as_ref(),
+        comment_syms.as_ref(),
+        config,
+    )?;
     map_relocations(arch.as_ref(), &obj_file, &mut sections, &section_indices, &symbol_indices)?;
     // Infer symbol sizes for 0-size symbols (must be done after map_relocations is called)
     infer_symbol_sizes(arch.as_ref(), &mut symbols, &sections)?;
@@ -1124,6 +1148,27 @@ fn parse_split_meta(obj_file: &object::File) -> Result<Option<SplitMeta>> {
     })
 }
 
+fn parse_mw_comment_syms(obj_file: &object::File) -> Result<Option<Vec<CommentSym>>> {
+    Ok(if let Some(section) = obj_file.section_by_name(COMMENT_SECTION) {
+        let data = section.uncompressed_data()?;
+        let mut reader: &[u8] = data.as_ref();
+        if let Ok(_header) = MWComment::from_reader(obj_file, &mut reader) {
+            CommentSym::from_reader(obj_file, &mut reader)?; // Null symbol
+            let mut comment_syms = Vec::with_capacity(obj_file.symbols().count());
+            for _symbol in obj_file.symbols() {
+                let comment_sym = CommentSym::from_reader(obj_file, &mut reader)?;
+                comment_syms.push(comment_sym);
+            }
+            Some(comment_syms)
+        } else {
+            // .comment section exists but the header failed to parse, likely an unsupported compiler version (e.g. mwccarm)
+            None
+        }
+    } else {
+        None
+    })
+}
+
 #[cfg(test)]
 mod test {
     use super::*;

objdiff-core/src/util.rs

@@ -40,6 +40,13 @@ pub fn read_u16(obj_file: &object::File, reader: &mut &[u8]) -> Result<u16> {
     Ok(obj_file.endianness().read_u16(value))
 }
 
+pub fn read_u8(reader: &mut &[u8]) -> Result<u8> {
+    ensure!(!reader.is_empty(), "Not enough bytes to read u8");
+    let value = reader[0];
+    *reader = &reader[1..];
+    Ok(value)
+}
+
 pub fn align_size_to_4(size: usize) -> usize { (size + 3) & !3 }
 
 #[cfg(feature = "std")]

objdiff-core/tests/snapshots/arch_ppc__read_extab.snap

@@ -137,7 +137,9 @@ Object {
                 0,
             ),
             flags: FlagSet(Local),
-            align: None,
+            align: Some(
+                4,
+            ),
             virtual_address: None,
         },
         Symbol {
@@ -151,7 +153,9 @@ Object {
                 0,
             ),
             flags: FlagSet(Global | HasExtra),
-            align: None,
+            align: Some(
+                4,
+            ),
             virtual_address: None,
         },
         Symbol {
@@ -165,7 +169,9 @@ Object {
                 0,
             ),
             flags: FlagSet(Global | HasExtra),
-            align: None,
+            align: Some(
+                4,
+            ),
             virtual_address: None,
         },
         Symbol {
@@ -181,7 +187,9 @@ Object {
                 0,
             ),
             flags: FlagSet(Global | Weak | HasExtra),
-            align: None,
+            align: Some(
+                4,
+            ),
             virtual_address: None,
         },
         Symbol {
@@ -195,7 +203,9 @@ Object {
                 1,
             ),
             flags: FlagSet(Local),
-            align: None,
+            align: Some(
+                4,
+            ),
             virtual_address: None,
         },
         Symbol {
@@ -209,7 +219,9 @@ Object {
                 1,
             ),
             flags: FlagSet(Local | CompilerGenerated),
-            align: None,
+            align: Some(
+                4,
+            ),
             virtual_address: None,
         },
         Symbol {
@@ -223,7 +235,9 @@ Object {
                 1,
             ),
             flags: FlagSet(Local | CompilerGenerated),
-            align: None,
+            align: Some(
+                4,
+            ),
             virtual_address: None,
         },
         Symbol {
@@ -237,7 +251,9 @@ Object {
                 1,
             ),
             flags: FlagSet(Local | CompilerGenerated),
-            align: None,
+            align: Some(
+                4,
+            ),
             virtual_address: None,
         },
         Symbol {
@@ -251,7 +267,9 @@ Object {
                 2,
             ),
             flags: FlagSet(Local),
-            align: None,
+            align: Some(
+                4,
+            ),
             virtual_address: None,
         },
         Symbol {
@@ -265,7 +283,9 @@ Object {
                 2,
             ),
             flags: FlagSet(Local | CompilerGenerated),
-            align: None,
+            align: Some(
+                4,
+            ),
             virtual_address: None,
         },
         Symbol {
@@ -279,7 +299,9 @@ Object {
                 2,
             ),
             flags: FlagSet(Local | CompilerGenerated),
-            align: None,
+            align: Some(
+                4,
+            ),
             virtual_address: None,
         },
         Symbol {
@@ -293,7 +315,9 @@ Object {
                 2,
             ),
             flags: FlagSet(Local | CompilerGenerated),
-            align: None,
+            align: Some(
+                4,
+            ),
             virtual_address: None,
         },
         Symbol {