From 6bb4ffd492ae9fdb93a7896aa4ad0847315841bf Mon Sep 17 00:00:00 2001 From: OndrikB Date: Tue, 17 Sep 2024 19:34:00 +0200 Subject: [PATCH 1/3] Disambiguate dummy symbols --- objdiff-core/src/obj/read.rs | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/objdiff-core/src/obj/read.rs b/objdiff-core/src/obj/read.rs index 04665c1..c942fa3 100644 --- a/objdiff-core/src/obj/read.rs +++ b/objdiff-core/src/obj/read.rs @@ -1,4 +1,4 @@ -use std::{collections::HashSet, fs, io::Cursor, mem::size_of, path::Path}; +use std::{collections::{HashMap, HashSet}, fs, io::Cursor, mem::size_of, path::Path}; use anyhow::{anyhow, bail, ensure, Context, Result}; use filetime::FileTime; @@ -627,6 +627,27 @@ pub fn parse(data: &[u8], config: &DiffObjConfig) -> Result { section.relocations = relocations_by_section(arch.as_ref(), &obj_file, section, split_meta.as_ref())?; } + // The dummy symbols all have the same name, which means that only the first one can be + // compared + let all_names = sections + .iter() + .flat_map(|section| section.symbols.iter().map(|symbol| symbol.name.clone())); + let mut name_counts: HashMap = HashMap::new(); + for name in all_names { + name_counts.entry(name).and_modify(|i| *i += 1).or_insert(1); + } + // Reversing the iterator here means the symbol sections will be given in the expected order + // since they're assigned from the count down to prevent + // having to use another hashmap which counts up + // TODO what about reverse_fn_order? + for section in sections.iter_mut().rev() { + for symbol in section.symbols.iter_mut().rev() { + if name_counts[&symbol.name] > 1 { + name_counts.entry(symbol.name.clone()).and_modify(|i| *i -= 1); + symbol.name = format!("{} {}", &symbol.name, name_counts[&symbol.name]); + } + } + } if config.combine_data_sections { combine_data_sections(&mut sections)?; } From bd9b9e3cc83fc6c8d8350e0c79a39fea9c5e18f3 Mon Sep 17 00:00:00 2001 From: OndrikB Date: Tue, 17 Sep 2024 19:39:17 +0200 Subject: [PATCH 2/3] Small formatting improvement --- objdiff-core/src/obj/read.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/objdiff-core/src/obj/read.rs b/objdiff-core/src/obj/read.rs index c942fa3..e8f7d22 100644 --- a/objdiff-core/src/obj/read.rs +++ b/objdiff-core/src/obj/read.rs @@ -644,7 +644,7 @@ pub fn parse(data: &[u8], config: &DiffObjConfig) -> Result { for symbol in section.symbols.iter_mut().rev() { if name_counts[&symbol.name] > 1 { name_counts.entry(symbol.name.clone()).and_modify(|i| *i -= 1); - symbol.name = format!("{} {}", &symbol.name, name_counts[&symbol.name]); + symbol.name = format!("{} ({})", &symbol.name, name_counts[&symbol.name]); } } } From ba7e158788cc65150f86e3b19b35b62b07795acc Mon Sep 17 00:00:00 2001 From: OndrikB Date: Thu, 26 Sep 2024 20:33:49 +0200 Subject: [PATCH 3/3] Put HashMap logic into symbol creation --- objdiff-core/src/obj/read.rs | 48 +++++++++++++++++------------------- 1 file changed, 23 insertions(+), 25 deletions(-) diff --git a/objdiff-core/src/obj/read.rs b/objdiff-core/src/obj/read.rs index e8f7d22..1e093d6 100644 --- a/objdiff-core/src/obj/read.rs +++ b/objdiff-core/src/obj/read.rs @@ -1,4 +1,10 @@ -use std::{collections::{HashMap, HashSet}, fs, io::Cursor, mem::size_of, path::Path}; +use std::{ + collections::{HashMap, HashSet}, + fs, + io::Cursor, + mem::size_of, + path::Path, +}; use anyhow::{anyhow, bail, ensure, Context, Result}; use filetime::FileTime; @@ -136,6 +142,7 @@ fn symbols_by_section( obj_file: &File<'_>, section: &ObjSection, split_meta: Option<&SplitMeta>, + name_counts: &mut HashMap, ) -> Result> { let mut result = Vec::::new(); for symbol in obj_file.symbols() { @@ -168,8 +175,14 @@ fn symbols_by_section( } if result.is_empty() { // Dummy symbol for empty sections + *name_counts.entry(section.name.clone()).or_insert(0) += 1; + let current_count: u32 = *name_counts.get(§ion.name).unwrap(); result.push(ObjSymbol { - name: format!("[{}]", section.name), + name: if current_count > 1 { + format!("[{} ({})]", section.name, current_count) + } else { + format!("[{}]", section.name) + }, demangled_name: None, address: 0, section_address: 0, @@ -621,33 +634,18 @@ pub fn parse(data: &[u8], config: &DiffObjConfig) -> Result { let arch = new_arch(&obj_file)?; let split_meta = split_meta(&obj_file)?; let mut sections = filter_sections(&obj_file, split_meta.as_ref())?; + let mut name_counts: HashMap = HashMap::new(); for section in &mut sections { - section.symbols = - symbols_by_section(arch.as_ref(), &obj_file, section, split_meta.as_ref())?; + section.symbols = symbols_by_section( + arch.as_ref(), + &obj_file, + section, + split_meta.as_ref(), + &mut name_counts, + )?; section.relocations = relocations_by_section(arch.as_ref(), &obj_file, section, split_meta.as_ref())?; } - // The dummy symbols all have the same name, which means that only the first one can be - // compared - let all_names = sections - .iter() - .flat_map(|section| section.symbols.iter().map(|symbol| symbol.name.clone())); - let mut name_counts: HashMap = HashMap::new(); - for name in all_names { - name_counts.entry(name).and_modify(|i| *i += 1).or_insert(1); - } - // Reversing the iterator here means the symbol sections will be given in the expected order - // since they're assigned from the count down to prevent - // having to use another hashmap which counts up - // TODO what about reverse_fn_order? - for section in sections.iter_mut().rev() { - for symbol in section.symbols.iter_mut().rev() { - if name_counts[&symbol.name] > 1 { - name_counts.entry(symbol.name.clone()).and_modify(|i| *i -= 1); - symbol.name = format!("{} ({})", &symbol.name, name_counts[&symbol.name]); - } - } - } if config.combine_data_sections { combine_data_sections(&mut sections)?; }