Fix up .comment section writing & support symbol force_active

Cargo.lock

@@ -214,7 +214,7 @@ dependencies = [
 
 [[package]]
 name = "decomp-toolkit"
-version = "0.3.2"
+version = "0.3.3"
 dependencies = [
  "anyhow",
  "ar",

src/util/comment.rs

@@ -3,9 +3,10 @@ use std::{
     ops::Deref,
 };
 
-use anyhow::{bail, Context, Result};
+use anyhow::{bail, ensure, Context, Result};
 use byteorder::{BigEndian, ReadBytesExt, WriteBytesExt};
 use num_enum::{IntoPrimitive, TryFromPrimitive};
+use object::Symbol;
 
 use crate::obj::{ObjSymbol, ObjSymbolKind};
 
@@ -19,6 +20,7 @@ pub enum MWFloatKind {
 
 #[derive(Debug, Clone)]
 pub struct MWComment {
+    pub comment_version: u8,
     pub compiler_version: [u8; 4],
     pub pool_data: bool,
     pub float: MWFloatKind,
@@ -31,6 +33,7 @@ pub struct MWComment {
 impl Default for MWComment {
     fn default() -> Self {
         Self {
+            comment_version: 10,
             // Metrowerks C/C++ Compiler for Embedded PowerPC
             // Version 2.4.2 build 81
             // (CodeWarrior for GameCube 1.3.2)
@@ -43,14 +46,31 @@ impl Default for MWComment {
             unsafe_global_reg_vars: false,
         }
     }
+
+    // fn default() -> Self {
+    //     Self {
+    //         comment_version: 11,
+    //         // Metrowerks C/C++ Compiler for Embedded PowerPC.
+    //         // Version 2.4.7 build 108
+    //         // (CodeWarrior for GameCube 2.7)
+    //         compiler_version: [2, 4, 7, 1],
+    //         pool_data: true,
+    //         float: MWFloatKind::Hard,
+    //         processor: 0x16, // gekko
+    //         incompatible_return_small_structs: false,
+    //         incompatible_sfpe_double_params: false,
+    //         unsafe_global_reg_vars: false,
+    //     }
+    // }
 }
 
-const MAGIC: &[u8] = "CodeWarrior\n".as_bytes();
+const MAGIC: &[u8] = "CodeWarrior".as_bytes();
 const PADDING: &[u8] = &[0u8; 0x16];
 
 impl MWComment {
     pub fn parse_header<R: Read + Seek>(reader: &mut R) -> Result<MWComment> {
         let mut header = MWComment {
+            comment_version: 0,
             compiler_version: [0; 4],
             pool_data: false,
             float: MWFloatKind::None,
@@ -59,12 +79,19 @@ impl MWComment {
             incompatible_sfpe_double_params: false,
             unsafe_global_reg_vars: false,
         };
-        // 0x0 - 0xB
+        // 0x0 - 0xA
         let mut magic = vec![0u8; MAGIC.len()];
         reader.read_exact(&mut magic).context("While reading magic")?;
         if magic.deref() != MAGIC {
             bail!("Invalid comment section magic: {:?}", magic);
         }
+        // 0xB
+        header.comment_version = reader.read_u8()?;
+        ensure!(
+            matches!(header.comment_version, 8 | 10 | 11),
+            "Unknown comment version: {}",
+            header.comment_version
+        );
         // 0xC - 0xF
         reader
             .read_exact(&mut header.compiler_version)
@@ -106,12 +133,21 @@ impl MWComment {
     }
 
     pub fn write_header<W: Write>(&self, w: &mut W) -> Result<()> {
+        // 0x0 - 0xA
         w.write_all(MAGIC)?;
+        // 0xB
+        w.write_u8(self.comment_version)?;
+        // 0xC - 0xF
         w.write_all(&self.compiler_version)?;
+        // 0x10
         w.write_u8(if self.pool_data { 1 } else { 0 })?;
+        // 0x11
         w.write_u8(self.float.into())?;
+        // 0x12 - 0x13
         w.write_u16::<BigEndian>(self.processor)?;
+        // 0x14
         w.write_u8(0x2C)?;
+        // 0x15
         let mut flags = 0u8;
         if self.incompatible_return_small_structs {
             flags |= 1;
@@ -123,12 +159,21 @@ impl MWComment {
             flags |= 4;
         }
         w.write_u8(flags)?;
+        // 0x16 - 0x2C
         w.write_all(PADDING)?;
         Ok(())
     }
 }
 
-pub fn write_comment_sym<W: Write>(w: &mut W, symbol: &ObjSymbol) -> Result<()> {
+#[derive(Debug, Copy, Clone)]
+pub struct CommentSym {
+    pub align: u32,
+    pub vis_flags: u8,
+    pub active_flags: u8,
+}
+
+impl CommentSym {
+    pub fn from(symbol: &ObjSymbol) -> Self {
         let align = match symbol.align {
             Some(align) => align,
             None => {
@@ -144,18 +189,39 @@ pub fn write_comment_sym<W: Write>(w: &mut W, symbol: &ObjSymbol) -> Result<()>
                 }
             }
         };
-    w.write_u32::<BigEndian>(align)?;
         let mut vis_flags = 0;
         if symbol.flags.is_weak() {
-        vis_flags |= 0xE; // TODO 0xD?
+            vis_flags |= 0xD;
         }
-    w.write_u8(vis_flags)?;
         let mut active_flags = 0;
         if symbol.flags.is_force_active() {
-        active_flags |= 8;
+            active_flags |= 0x8; // TODO what is 0x10?
         }
-    w.write_u8(active_flags)?;
+        Self { align, vis_flags, active_flags }
+    }
+}
+
+pub fn write_comment_sym<W: Write>(w: &mut W, symbol: CommentSym) -> Result<()> {
+    w.write_u32::<BigEndian>(symbol.align)?;
+    w.write_u8(symbol.vis_flags)?;
+    w.write_u8(symbol.active_flags)?;
     w.write_u8(0)?;
     w.write_u8(0)?;
     Ok(())
 }
+
+pub fn read_comment_sym<R: Read>(r: &mut R, x: &Symbol) -> Result<CommentSym> {
+    let mut out = CommentSym { align: 0, vis_flags: 0, active_flags: 0 };
+    out.align = r.read_u32::<BigEndian>()?;
+    out.vis_flags = r.read_u8()?;
+    ensure!(matches!(out.vis_flags, 0 | 0xD | 0xE), "Unknown vis_flags {}", out.vis_flags);
+    out.active_flags = r.read_u8()?;
+    ensure!(
+        matches!(out.active_flags, 0 | 0x8 | 0x10),
+        "Unknown active_flags {}",
+        out.active_flags
+    );
+    ensure!(r.read_u8()? == 0, "Unexpected value after active_flags (1)");
+    ensure!(r.read_u8()? == 0, "Unexpected value after active_flags (2)");
+    Ok(out)
+}

src/util/config.rs

@@ -75,6 +75,9 @@ pub fn parse_symbol_line(line: &str, obj: &mut ObjInfo) -> Result<Option<ObjSymb
                     "hidden" => {
                         symbol.flags.0 |= ObjSymbolFlags::Hidden;
                     }
+                    "force_active" => {
+                        symbol.flags.0 |= ObjSymbolFlags::ForceActive;
+                    }
                     "noreloc" => {
                         ensure!(
                             symbol.size != 0,
@@ -155,6 +158,9 @@ fn write_symbol<W: Write>(w: &mut W, obj: &ObjInfo, symbol: &ObjSymbol) -> Resul
     if symbol.flags.is_hidden() {
         write!(w, " hidden")?;
     }
+    if symbol.flags.is_force_active() {
+        write!(w, " force_active")?;
+    }
     if obj.blocked_ranges.contains_key(&(symbol.address as u32)) {
         write!(w, " noreloc")?;
     }
@@ -202,9 +208,7 @@ fn symbol_kind_from_str(s: &str) -> Option<ObjSymbolKind> {
 
 #[inline]
 fn symbol_flags_to_str(flags: ObjSymbolFlagSet) -> Option<&'static str> {
-    if flags.0.contains(ObjSymbolFlags::Common) {
+    if flags.0.contains(ObjSymbolFlags::Weak) {
-        Some("common")
-    } else if flags.0.contains(ObjSymbolFlags::Weak) {
         Some("weak")
     } else if flags.0.contains(ObjSymbolFlags::Global) {
         Some("global")

src/util/elf.rs

@@ -26,7 +26,7 @@ use crate::{
         ObjSplit, ObjSymbol, ObjSymbolFlagSet, ObjSymbolFlags, ObjSymbolKind,
     },
     util::{
-        comment::{write_comment_sym, MWComment},
+        comment::{read_comment_sym, write_comment_sym, CommentSym, MWComment},
         file::map_file,
         nested::NestedVec,
     },
@@ -296,8 +296,12 @@ pub fn process_elf<P: AsRef<Path>>(path: P) -> Result<ObjInfo> {
         let data = comment_section.uncompressed_data()?;
         let mut reader = Cursor::new(&*data);
         let header = MWComment::parse_header(&mut reader)?;
-        log::info!("Loaded comment header {:?}", header);
+        log::debug!("Loaded comment header {:?}", header);
-
+        for symbol in obj_file.symbols() {
+            let comment_sym = read_comment_sym(&mut reader, &symbol)?;
+            log::debug!("Symbol {:?} -> Comment {:?}", symbol, comment_sym);
+        }
+        ensure!(data.len() - reader.position() as usize == 0, "Comment data not fully read");
         header
     } else {
         MWComment::default()
@@ -369,20 +373,25 @@ pub fn write_elf(obj: &ObjInfo) -> Result<Vec<u8>> {
 
     // Generate comment section
     let mut comment_data = if obj.kind == ObjKind::Relocatable {
-        // let mut comment_data = Vec::<u8>::with_capacity(0x2C + obj.symbols.len() * 8);
+        let mut comment_data = Vec::<u8>::with_capacity(0x2C + obj.symbols.count() * 8);
-        // let name = writer.add_section_name(".comment".as_bytes());
+        let name = writer.add_section_name(".comment".as_bytes());
-        // let index = writer.reserve_section_index();
+        let index = writer.reserve_section_index();
-        // out_sections.push(OutSection {
+        out_sections.push(OutSection {
-        //     index,
+            index,
-        //     rela_index: None,
+            rela_index: None,
-        //     offset: 0,
+            offset: 0,
-        //     rela_offset: 0,
+            rela_offset: 0,
-        //     name,
+            name,
-        //     rela_name: None,
+            rela_name: None,
-        // });
+        });
-        // obj.mw_comment.write_header(&mut comment_data)?;
+        obj.mw_comment.write_header(&mut comment_data)?;
-        // Some(comment_data)
+        // Null symbol
-        None::<Vec<u8>>
+        write_comment_sym(&mut comment_data, CommentSym {
+            align: 0,
+            vis_flags: 0,
+            active_flags: 0,
+        })?;
+        Some(comment_data)
     } else {
         None
     };
@@ -420,7 +429,11 @@ pub fn write_elf(obj: &ObjInfo) -> Result<Vec<u8>> {
             },
         });
         if let Some(comment_data) = &mut comment_data {
-            comment_data.write_u64::<BigEndian>(0)?;
+            write_comment_sym(comment_data, CommentSym {
+                align: 1,
+                vis_flags: 0,
+                active_flags: 0,
+            })?;
         }
         section_symbol_offset += 1;
     }
@@ -438,10 +451,17 @@ pub fn write_elf(obj: &ObjInfo) -> Result<Vec<u8>> {
                 st_other: elf::STV_DEFAULT,
                 st_shndx: 0,
                 st_value: 0,
-                st_size: section.size,
+                st_size: 0, // section.size
             };
             num_local = writer.symbol_count();
             out_symbols.push(OutSymbol { index, sym });
+            if let Some(comment_data) = &mut comment_data {
+                write_comment_sym(comment_data, CommentSym {
+                    align: section.align as u32,
+                    vis_flags: 0,
+                    active_flags: 0,
+                })?;
+            }
         }
     }
 
@@ -500,7 +520,7 @@ pub fn write_elf(obj: &ObjInfo) -> Result<Vec<u8>> {
         out_symbols.push(OutSymbol { index, sym });
         *symbol_map = Some(index.0);
         if let Some(comment_data) = &mut comment_data {
-            write_comment_sym(comment_data, symbol)?;
+            write_comment_sym(comment_data, CommentSym::from(symbol))?;
         }
     }
 
@@ -625,7 +645,7 @@ pub fn write_elf(obj: &ObjInfo) -> Result<Vec<u8>> {
                     elf::R_PPC_REL14
                 }
                 ObjRelocKind::PpcEmbSda21 => {
-                    r_offset = (r_offset & !3) + 2;
+                    r_offset &= !3;
                     elf::R_PPC_EMB_SDA21
                 }
             };
@@ -759,9 +779,9 @@ fn to_obj_symbol(
         kind: match symbol.kind() {
             SymbolKind::Text => ObjSymbolKind::Function,
             SymbolKind::Data => ObjSymbolKind::Object,
-            SymbolKind::Unknown => ObjSymbolKind::Unknown,
+            SymbolKind::Unknown | SymbolKind::Label => ObjSymbolKind::Unknown,
             SymbolKind::Section => ObjSymbolKind::Section,
-            _ => bail!("Unsupported symbol kind: {:?}", symbol.kind()),
+            _ => bail!("Unsupported symbol kind: {:?}", symbol),
         },
         // TODO common symbol value?
         align: None,
@@ -798,7 +818,9 @@ fn to_obj_reloc(
     let target_symbol = symbol_indexes[symbol.index().0]
         .ok_or_else(|| anyhow!("Relocation against stripped symbol: {symbol:?}"))?;
     let addend = match symbol.kind() {
-        SymbolKind::Text | SymbolKind::Data | SymbolKind::Unknown => Ok(reloc.addend()),
+        SymbolKind::Text | SymbolKind::Data | SymbolKind::Unknown | SymbolKind::Label => {
+            Ok(reloc.addend())
+        }
         SymbolKind::Section => {
             let addend = if reloc.has_implicit_addend() {
                 let addend = u32::from_be_bytes(