Merge branch 'main' into fork/sozud/add_superh_data_info

# Conflicts:
#	objdiff-core/src/obj/mod.rs

Author: encounter

objdiff-core/src/arch/arm.rs

@@ -15,7 +15,7 @@ use crate::{
     diff::{ArmArchVersion, ArmR9Usage, DiffObjConfig, display::InstructionPart},
     obj::{
         InstructionRef, Relocation, RelocationFlags, ResolvedInstructionRef, ResolvedRelocation,
-        ScannedInstruction, SymbolFlag, SymbolFlagSet, SymbolKind,
+        ScannedInstruction, Section, SectionKind, Symbol, SymbolFlag, SymbolFlagSet, SymbolKind,
     },
 };
 
@@ -32,7 +32,8 @@ impl ArchArm {
         let endianness = file.endianness();
         match file {
             object::File::Elf32(_) => {
-                let disasm_modes = Self::elf_get_mapping_symbols(file);
+                // The disasm_modes mapping is populated later in the post_init step so that we have access to merged sections.
+                let disasm_modes = BTreeMap::new();
                 let detected_version = Self::elf_detect_arm_version(file)?;
                 Ok(Self { disasm_modes, detected_version, endianness })
             }
@@ -73,18 +74,22 @@ impl ArchArm {
         Ok(None)
     }
 
-    fn elf_get_mapping_symbols(file: &object::File) -> BTreeMap<usize, Vec<DisasmMode>> {
-        file.sections()
-            .filter(|s| s.kind() == object::SectionKind::Text)
-            .map(|s| {
-                let index = s.index();
-                let mut mapping_symbols: Vec<_> = file
-                    .symbols()
-                    .filter(|s| s.section_index().map(|i| i == index).unwrap_or(false))
-                    .filter_map(|s| DisasmMode::from_symbol(&s))
+    fn get_mapping_symbols(
+        sections: &[Section],
+        symbols: &[Symbol],
+    ) -> BTreeMap<usize, Vec<DisasmMode>> {
+        sections
+            .iter()
+            .enumerate()
+            .filter(|(_, section)| section.kind == SectionKind::Code)
+            .map(|(index, _)| {
+                let mut mapping_symbols: Vec<_> = symbols
+                    .iter()
+                    .filter(|s| s.section.map(|i| i == index).unwrap_or(false))
+                    .filter_map(DisasmMode::from_symbol)
                     .collect();
                 mapping_symbols.sort_unstable_by_key(|x| x.address);
-                (s.index().0 - 1, mapping_symbols)
+                (index, mapping_symbols)
             })
             .collect()
     }
@@ -178,6 +183,10 @@ impl ArchArm {
 }
 
 impl Arch for ArchArm {
+    fn post_init(&mut self, sections: &[Section], symbols: &[Symbol]) {
+        self.disasm_modes = Self::get_mapping_symbols(sections, symbols);
+    }
+
     fn scan_instructions(
         &self,
         address: u64,
@@ -338,6 +347,7 @@ impl Arch for ArchArm {
             cb(InstructionPart::reloc())?;
         } else {
             push_args(
+                ins,
                 &parsed_ins,
                 resolved.relocation,
                 resolved.ins_ref.address as u32,
@@ -441,7 +451,7 @@ impl Arch for ArchArm {
 
     fn extra_symbol_flags(&self, symbol: &object::Symbol) -> SymbolFlagSet {
         let mut flags = SymbolFlagSet::default();
-        if DisasmMode::from_symbol(symbol).is_some() {
+        if DisasmMode::from_object_symbol(symbol).is_some() {
             flags |= SymbolFlag::Hidden;
         }
         flags
@@ -455,15 +465,21 @@ struct DisasmMode {
 }
 
 impl DisasmMode {
-    fn from_symbol<'a>(sym: &object::Symbol<'a, '_, &'a [u8]>) -> Option<Self> {
+    fn from_object_symbol<'a>(sym: &object::Symbol<'a, '_, &'a [u8]>) -> Option<Self> {
         sym.name()
             .ok()
             .and_then(unarm::ParseMode::from_mapping_symbol)
             .map(|mapping| DisasmMode { address: sym.address() as u32, mapping })
     }
+
+    fn from_symbol(sym: &Symbol) -> Option<Self> {
+        unarm::ParseMode::from_mapping_symbol(&sym.name)
+            .map(|mapping| DisasmMode { address: sym.address as u32, mapping })
+    }
 }
 
 fn push_args(
+    ins: unarm::Ins,
     parsed_ins: &unarm::ParsedIns,
     relocation: Option<ResolvedRelocation>,
     cur_addr: u32,
@@ -609,6 +625,14 @@ fn push_args(
             arg_cb(InstructionPart::opaque("!"))?;
         }
     }
+
+    let branch_dest = get_pc_relative_load_address(ins, cur_addr);
+    if let Some(branch_dest) = branch_dest {
+        arg_cb(InstructionPart::basic(" (->"))?;
+        arg_cb(InstructionPart::branch_dest(branch_dest))?;
+        arg_cb(InstructionPart::basic(")"))?;
+    }
+
     Ok(())
 }
 
@@ -636,3 +660,21 @@ fn find_reloc_arg(
         None
     }
 }
+
+fn get_pc_relative_load_address(ins: unarm::Ins, address: u32) -> Option<u32> {
+    match ins {
+        unarm::Ins::Arm(ins)
+            if ins.op == arm::Opcode::Ldr
+                && ins.modifier_addr_ldr_str() == arm::AddrLdrStr::Imm
+                && ins.field_rn_deref().reg == args::Register::Pc =>
+        {
+            let offset = ins.field_offset_12().value;
+            Some(address.wrapping_add_signed(offset + 8))
+        }
+        unarm::Ins::Thumb(ins) if ins.op == thumb::Opcode::LdrPc => {
+            let offset = ins.field_rel_immed_8().value;
+            Some((address & !3).wrapping_add_signed(offset + 4))
+        }
+        _ => None,
+    }
+}

objdiff-core/src/arch/mod.rs

@@ -12,7 +12,7 @@ use crate::{
     },
     obj::{
         InstructionArg, Object, ParsedInstruction, Relocation, RelocationFlags,
-        ResolvedInstructionRef, ScannedInstruction, Symbol, SymbolFlagSet, SymbolKind,
+        ResolvedInstructionRef, ScannedInstruction, Section, Symbol, SymbolFlagSet, SymbolKind,
     },
     util::ReallySigned,
 };
@@ -183,6 +183,9 @@ impl DataType {
 }
 
 pub trait Arch: Send + Sync + Debug {
+    // Finishes arch-specific initialization that must be done after sections have been combined.
+    fn post_init(&mut self, _sections: &[Section], _symbols: &[Symbol]) {}
+
     /// Generate a list of instructions references (offset, size, opcode) from the given code.
     ///
     /// The opcode IDs are used to generate the initial diff. Implementations should do as little

objdiff-core/src/obj/mod.rs

@@ -9,7 +9,10 @@ use alloc::{
     vec,
     vec::Vec,
 };
-use core::{fmt, num::NonZeroU32};
+use core::{
+    fmt,
+    num::{NonZeroU32, NonZeroU64},
+};
 
 use flagset::{FlagSet, flags};
 
@@ -70,6 +73,7 @@ pub struct Section {
     pub kind: SectionKind,
     pub data: SectionData,
     pub flags: SectionFlagSet,
+    pub align: Option<NonZeroU64>,
     pub relocations: Vec<Relocation>,
     /// Line number info (.line or .debug_line section)
     pub line_info: BTreeMap<u64, u32>,
@@ -110,6 +114,12 @@ impl Section {
         self.data.get(offset as usize..offset as usize + symbol.size as usize)
     }
 
+    // The alignment to use when "Combine data/text sections" is enabled.
+    pub fn combined_alignment(&self) -> u64 {
+        const MIN_ALIGNMENT: u64 = 4;
+        self.align.map(|align| align.get().max(MIN_ALIGNMENT)).unwrap_or(MIN_ALIGNMENT)
+    }
+
     pub fn relocation_at<'obj>(
         &'obj self,
         obj: &'obj Object,
@@ -368,6 +378,7 @@ static DUMMY_SECTION: Section = Section {
     kind: SectionKind::Unknown,
     data: SectionData(Vec::new()),
     flags: SectionFlagSet::empty(),
+    align: None,
     relocations: Vec::new(),
     line_info: BTreeMap::new(),
     virtual_address: None,

objdiff-core/src/obj/read.rs

@@ -4,7 +4,7 @@ use alloc::{
     string::{String, ToString},
     vec::Vec,
 };
-use core::cmp::Ordering;
+use core::{cmp::Ordering, num::NonZeroU64};
 
 use anyhow::{Context, Result, anyhow, bail, ensure};
 use object::{Object as _, ObjectSection as _, ObjectSymbol as _};
@@ -17,7 +17,7 @@ use crate::{
         Symbol, SymbolFlag, SymbolKind,
         split_meta::{SPLITMETA_SECTION, SplitMeta},
     },
-    util::{read_u16, read_u32},
+    util::{align_data_slice_to, align_u64_to, read_u16, read_u32},
 };
 
 fn map_section_kind(section: &object::Section) -> SectionKind {
@@ -257,6 +257,7 @@ fn map_sections(
             kind,
             data: SectionData(data),
             flags: Default::default(),
+            align: NonZeroU64::new(section.align()),
             relocations: Default::default(),
             virtual_address,
             line_info: Default::default(),
@@ -739,7 +740,10 @@ fn do_combine_sections(
         }
         offsets.push(current_offset);
         current_offset += section.size;
+        let align = section.combined_alignment();
+        current_offset = align_u64_to(current_offset, align);
         data_size += section.data.len();
+        data_size = align_u64_to(data_size as u64, align) as usize;
         num_relocations += section.relocations.len();
     }
     if data_size > 0 {
@@ -754,6 +758,7 @@ fn do_combine_sections(
         let section = &mut sections[i];
         section.size = 0;
         data.append(&mut section.data.0);
+        align_data_slice_to(&mut data, section.combined_alignment());
         section.relocations.iter_mut().for_each(|r| r.address += offset);
         relocations.append(&mut section.relocations);
         line_info.append(&mut section.line_info.iter().map(|(&a, &l)| (a + offset, l)).collect());
@@ -843,7 +848,7 @@ pub fn read(obj_path: &std::path::Path, config: &DiffObjConfig) -> Result<Object
 
 pub fn parse(data: &[u8], config: &DiffObjConfig) -> Result<Object> {
     let obj_file = object::File::parse(data)?;
-    let arch = new_arch(&obj_file)?;
+    let mut arch = new_arch(&obj_file)?;
     let split_meta = parse_split_meta(&obj_file)?;
     let (mut sections, section_indices) =
         map_sections(arch.as_ref(), &obj_file, split_meta.as_ref())?;
@@ -857,6 +862,7 @@ pub fn parse(data: &[u8], config: &DiffObjConfig) -> Result<Object> {
     if config.combine_data_sections || config.combine_text_sections {
         combine_sections(&mut sections, &mut symbols, config)?;
     }
+    arch.post_init(&sections, &symbols);
     Ok(Object {
         arch,
         endianness: obj_file.endianness(),

objdiff-core/src/obj/snapshots/objdiff_core__obj__read__test__combine_sections.snap

@@ -14,6 +14,7 @@ expression: "(sections, symbols)"
                 8,
             ),
             flags: FlagSet(),
+            align: None,
             relocations: [
                 Relocation {
                     flags: Elf(
@@ -53,6 +54,7 @@ expression: "(sections, symbols)"
                 12,
             ),
             flags: FlagSet(Combined),
+            align: None,
             relocations: [
                 Relocation {
                     flags: Elf(
@@ -87,6 +89,7 @@ expression: "(sections, symbols)"
                 0,
             ),
             flags: FlagSet(Hidden),
+            align: None,
             relocations: [],
             line_info: {},
             virtual_address: None,
@@ -101,6 +104,7 @@ expression: "(sections, symbols)"
                 0,
             ),
             flags: FlagSet(Hidden),
+            align: None,
             relocations: [],
             line_info: {},
             virtual_address: None,

objdiff-core/src/obj/split_meta.rs

@@ -3,6 +3,10 @@ use alloc::{string::String, vec, vec::Vec};
 use anyhow::{Result, anyhow};
 use object::{Endian, ObjectSection, elf::SHT_NOTE};
 
+#[cfg(feature = "std")]
+use crate::util::align_data_to_4;
+use crate::util::align_size_to_4;
+
 pub const SPLITMETA_SECTION: &str = ".note.split";
 pub const SHT_SPLITMETA: u32 = SHT_NOTE;
 pub const ELF_NOTE_SPLIT: &[u8] = b"Split";
@@ -190,17 +194,6 @@ where E: Endian
     }
 }
 
-fn align_size_to_4(size: usize) -> usize { (size + 3) & !3 }
-
-#[cfg(feature = "std")]
-fn align_data_to_4<W: std::io::Write + ?Sized>(writer: &mut W, len: usize) -> std::io::Result<()> {
-    const ALIGN_BYTES: &[u8] = &[0; 4];
-    if len % 4 != 0 {
-        writer.write_all(&ALIGN_BYTES[..4 - len % 4])?;
-    }
-    Ok(())
-}
-
 // ELF note format:
 // Name Size | 4 bytes (integer)
 // Desc Size | 4 bytes (integer)

objdiff-core/src/util.rs

@@ -1,4 +1,4 @@
-use alloc::format;
+use alloc::{format, vec::Vec};
 use core::fmt;
 
 use anyhow::{Result, ensure};
@@ -39,3 +39,24 @@ pub fn read_u16(obj_file: &object::File, reader: &mut &[u8]) -> Result<u16> {
     *reader = &reader[2..];
     Ok(obj_file.endianness().read_u16(value))
 }
+
+pub fn align_size_to_4(size: usize) -> usize { (size + 3) & !3 }
+
+#[cfg(feature = "std")]
+pub fn align_data_to_4<W: std::io::Write + ?Sized>(
+    writer: &mut W,
+    len: usize,
+) -> std::io::Result<()> {
+    const ALIGN_BYTES: &[u8] = &[0; 4];
+    if len % 4 != 0 {
+        writer.write_all(&ALIGN_BYTES[..4 - len % 4])?;
+    }
+    Ok(())
+}
+
+pub fn align_u64_to(len: u64, align: u64) -> u64 { len + ((align - (len % align)) % align) }
+
+pub fn align_data_slice_to(data: &mut Vec<u8>, align: u64) {
+    const ALIGN_BYTE: u8 = 0;
+    data.resize(align_u64_to(data.len() as u64, align) as usize, ALIGN_BYTE);
+}

objdiff-core/tests/arch_arm.rs

@@ -5,7 +5,7 @@ mod common;
 #[test]
 #[cfg(feature = "arm")]
 fn read_arm() {
-    let diff_config = diff::DiffObjConfig { mips_register_prefix: true, ..Default::default() };
+    let diff_config = diff::DiffObjConfig { ..Default::default() };
     let obj = obj::read::parse(include_object!("data/arm/LinkStateItem.o"), &diff_config).unwrap();
     insta::assert_debug_snapshot!(obj);
     let symbol_idx =
@@ -19,7 +19,7 @@ fn read_arm() {
 #[test]
 #[cfg(feature = "arm")]
 fn read_thumb() {
-    let diff_config = diff::DiffObjConfig { mips_register_prefix: true, ..Default::default() };
+    let diff_config = diff::DiffObjConfig { ..Default::default() };
     let obj = obj::read::parse(include_object!("data/arm/thumb.o"), &diff_config).unwrap();
     insta::assert_debug_snapshot!(obj);
     let symbol_idx = obj
@@ -32,3 +32,15 @@ fn read_thumb() {
     let output = common::display_diff(&obj, &diff, symbol_idx, &diff_config);
     insta::assert_snapshot!(output);
 }
+
+#[test]
+#[cfg(feature = "arm")]
+fn combine_text_sections() {
+    let diff_config = diff::DiffObjConfig { combine_text_sections: true, ..Default::default() };
+    let obj = obj::read::parse(include_object!("data/arm/enemy300.o"), &diff_config).unwrap();
+    let symbol_idx = obj.symbols.iter().position(|s| s.name == "Enemy300Draw").unwrap();
+    let diff = diff::code::no_diff_code(&obj, symbol_idx, &diff_config).unwrap();
+    insta::assert_debug_snapshot!(diff.instruction_rows);
+    let output = common::display_diff(&obj, &diff, symbol_idx, &diff_config);
+    insta::assert_snapshot!(output);
+}

objdiff-core/tests/data/arm/enemy300.o

@@ -0,0 +1,7 @@
+---
+source: objdiff-core/tests/arch_arm.rs
+expression: output
+---
+[(Line(90), Dim, 5), (Address(0), Normal, 5), (Spacing(4), Normal, 0), (Opcode("ldr", 32799), Normal, 10), (Argument(Opaque("r12")), Normal, 0), (Basic(", "), Normal, 0), (Basic("["), Normal, 0), (Argument(Opaque("pc")), Normal, 0), (Basic(", "), Normal, 0), (Basic("#"), Normal, 0), (Argument(Signed(0)), Normal, 0), (Basic("]"), Normal, 0), (Basic(" (->"), Normal, 0), (BranchDest(8), Normal, 0), (Basic(")"), Normal, 0), (Eol, Normal, 0)]
+[(Line(90), Dim, 5), (Address(4), Normal, 5), (Spacing(4), Normal, 0), (Opcode("bx", 32779), Normal, 10), (Argument(Opaque("r12")), Normal, 0), (Eol, Normal, 0)]
+[(Line(90), Dim, 5), (Address(8), Normal, 5), (Spacing(4), Normal, 0), (Opcode(".word", 65535), Normal, 10), (Symbol(Symbol { name: "esEnemyDraw", demangled_name: None, address: 0, size: 0, kind: Unknown, section: None, flags: FlagSet(Global), align: None, virtual_address: None }), Bright, 0), (Eol, Normal, 0)]