Skip to content

Commit 868c3ab

Browse files
committed
Fix filtering out symbols from the target side that have a symbol with the same name and a .NON_MATCHING suffix.
- Target side: Show all the symbols except the `.NON_MATCHING` ones. - Base side: Ignore all the `.NON_MATCHING` symbols and also ignore the ones with the same name without the suffix
1 parent 0b95613 commit 868c3ab

File tree

6 files changed

+37
-32
lines changed

6 files changed

+37
-32
lines changed

objdiff-cli/src/cmd/report.rs

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,10 @@ use anyhow::{Context, Result, bail};
44
use argp::FromArgs;
55
use objdiff_core::{
66
bindings::report::{
7-
ChangeItem, ChangeItemInfo, ChangeUnit, Changes, ChangesInput, Measures, REPORT_VERSION,
8-
Report, ReportCategory, ReportItem, ReportItemMetadata, ReportUnit, ReportUnitMetadata,
7+
ChangeItem, ChangeItemInfo, ChangeUnit, Changes, ChangesInput, Measures, Report, ReportCategory, ReportItem, ReportItemMetadata, ReportUnit, ReportUnitMetadata, REPORT_VERSION
98
},
109
config::path::platform_path,
11-
diff, obj,
12-
obj::{SectionKind, SymbolFlag},
10+
diff, obj::{self, DiffSide, SectionKind, SymbolFlag},
1311
};
1412
use prost::Message;
1513
use rayon::iter::{IntoParallelRefIterator, ParallelIterator};
@@ -177,14 +175,14 @@ fn report_object(
177175
.target_path
178176
.as_ref()
179177
.map(|p| {
180-
obj::read::read(p.as_ref(), diff_config).with_context(|| format!("Failed to open {p}"))
178+
obj::read::read(p.as_ref(), diff_config, DiffSide::Target).with_context(|| format!("Failed to open {p}"))
181179
})
182180
.transpose()?;
183181
let base = object
184182
.base_path
185183
.as_ref()
186184
.map(|p| {
187-
obj::read::read(p.as_ref(), diff_config).with_context(|| format!("Failed to open {p}"))
185+
obj::read::read(p.as_ref(), diff_config, DiffSide::Base).with_context(|| format!("Failed to open {p}"))
188186
})
189187
.transpose()?;
190188
let result =

objdiff-core/src/arch/mips.rs

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,9 @@ use rabbitizer::{
1212

1313
use crate::{
1414
arch::{Arch, RelocationOverride, RelocationOverrideTarget},
15-
diff::{DiffObjConfig, MipsAbi, MipsInstrCategory, display::InstructionPart},
15+
diff::{display::InstructionPart, DiffObjConfig, MipsAbi, MipsInstrCategory},
1616
obj::{
17-
InstructionArg, InstructionArgValue, InstructionRef, Relocation, RelocationFlags,
18-
ResolvedInstructionRef, ResolvedRelocation, Section, Symbol, SymbolFlag, SymbolFlagSet,
17+
DiffSide, InstructionArg, InstructionArgValue, InstructionRef, Relocation, RelocationFlags, ResolvedInstructionRef, ResolvedRelocation, Section, Symbol, SymbolFlag, SymbolFlagSet
1918
},
2019
};
2120

@@ -27,6 +26,7 @@ pub struct ArchMips {
2726
pub ri_gp_value: i32,
2827
pub paired_relocations: Vec<BTreeMap<u64, i64>>,
2928
pub ignored_symbols: BTreeSet<usize>,
29+
pub diff_side: DiffSide,
3030
}
3131

3232
const EF_MIPS_ABI: u32 = 0x0000F000;
@@ -38,7 +38,7 @@ const EF_MIPS_MACH_5900: u32 = 0x00920000;
3838
const R_MIPS15_S3: u32 = 119;
3939

4040
impl ArchMips {
41-
pub fn new(object: &object::File) -> Result<Self> {
41+
pub fn new(object: &object::File, diff_side: DiffSide) -> Result<Self> {
4242
let mut abi = Abi::O32;
4343
let mut isa_extension = None;
4444
match object.flags() {
@@ -124,7 +124,7 @@ impl ArchMips {
124124
let Ok(name) = obj_symbol.name() else { continue };
125125
if let Some(prefix) = name.strip_suffix(".NON_MATCHING") {
126126
ignored_symbols.insert(obj_symbol.index().0);
127-
if let Some(target_symbol) = object.symbol_by_name(prefix) {
127+
if diff_side == DiffSide::Base && let Some(target_symbol) = object.symbol_by_name(prefix) {
128128
ignored_symbols.insert(target_symbol.index().0);
129129
}
130130
}
@@ -137,6 +137,7 @@ impl ArchMips {
137137
ri_gp_value,
138138
paired_relocations,
139139
ignored_symbols,
140+
diff_side,
140141
})
141142
}
142143

objdiff-core/src/arch/mod.rs

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -17,13 +17,10 @@ use object::Endian as _;
1717

1818
use crate::{
1919
diff::{
20-
DiffObjConfig,
21-
display::{ContextItem, HoverItem, InstructionPart},
20+
display::{ContextItem, HoverItem, InstructionPart}, DiffObjConfig
2221
},
2322
obj::{
24-
FlowAnalysisResult, InstructionArg, InstructionRef, Object, ParsedInstruction, Relocation,
25-
RelocationFlags, ResolvedInstructionRef, ResolvedSymbol, Section, Symbol, SymbolFlagSet,
26-
SymbolKind,
23+
DiffSide, FlowAnalysisResult, InstructionArg, InstructionRef, Object, ParsedInstruction, Relocation, RelocationFlags, ResolvedInstructionRef, ResolvedSymbol, Section, Symbol, SymbolFlagSet, SymbolKind
2724
},
2825
util::ReallySigned,
2926
};
@@ -418,15 +415,18 @@ pub trait Arch: Any + Debug + Send + Sync {
418415
}
419416
}
420417

421-
pub fn new_arch(object: &object::File) -> Result<Box<dyn Arch>> {
418+
pub fn new_arch(object: &object::File, diff_side: DiffSide) -> Result<Box<dyn Arch>> {
422419
use object::Object as _;
420+
// Avoid unused warnings on non-mips builds
421+
let _ = diff_side;
422+
423423
Ok(match object.architecture() {
424424
#[cfg(feature = "ppc")]
425425
object::Architecture::PowerPc | object::Architecture::PowerPc64 => {
426426
Box::new(ppc::ArchPpc::new(object)?)
427427
}
428428
#[cfg(feature = "mips")]
429-
object::Architecture::Mips => Box::new(mips::ArchMips::new(object)?),
429+
object::Architecture::Mips => Box::new(mips::ArchMips::new(object, diff_side)?),
430430
#[cfg(feature = "x86")]
431431
object::Architecture::I386 | object::Architecture::X86_64 => {
432432
Box::new(x86::ArchX86::new(object)?)

objdiff-core/src/jobs/objdiff.rs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,10 @@ use time::OffsetDateTime;
55
use typed_path::Utf8PlatformPathBuf;
66

77
use crate::{
8-
build::{BuildConfig, BuildStatus, run_make},
9-
diff::{DiffObjConfig, MappingConfig, ObjectDiff, diff_objs},
10-
jobs::{Job, JobContext, JobResult, JobState, start_job, update_status},
11-
obj::{Object, read},
8+
build::{run_make, BuildConfig, BuildStatus},
9+
diff::{diff_objs, DiffObjConfig, MappingConfig, ObjectDiff},
10+
jobs::{start_job, update_status, Job, JobContext, JobResult, JobState},
11+
obj::{read, DiffSide, Object},
1212
};
1313

1414
pub struct ObjDiffConfig {
@@ -117,7 +117,7 @@ fn run_build(
117117
&cancel,
118118
)?;
119119
step_idx += 1;
120-
match read::read(target_path.as_ref(), &config.diff_obj_config) {
120+
match read::read(target_path.as_ref(), &config.diff_obj_config, DiffSide::Target) {
121121
Ok(obj) => Some(obj),
122122
Err(e) => {
123123
first_status = BuildStatus {
@@ -141,7 +141,7 @@ fn run_build(
141141
Some(base_path) if second_status.success => {
142142
update_status(context, format!("Loading base {base_path}"), step_idx, total, &cancel)?;
143143
step_idx += 1;
144-
match read::read(base_path.as_ref(), &config.diff_obj_config) {
144+
match read::read(base_path.as_ref(), &config.diff_obj_config, DiffSide::Base) {
145145
Ok(obj) => Some(obj),
146146
Err(e) => {
147147
second_status = BuildStatus {

objdiff-core/src/obj/mod.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -437,3 +437,11 @@ impl Default for ResolvedInstructionRef<'_> {
437437
}
438438
}
439439
}
440+
441+
#[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)]
442+
pub enum DiffSide {
443+
/// The target/expected side of the diff.
444+
Target,
445+
/// The base side of the diff.
446+
Base,
447+
}

objdiff-core/src/obj/read.rs

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -11,12 +11,10 @@ use anyhow::{Context, Result, anyhow, bail, ensure};
1111
use object::{Object as _, ObjectSection as _, ObjectSymbol as _};
1212

1313
use crate::{
14-
arch::{Arch, RelocationOverride, RelocationOverrideTarget, new_arch},
14+
arch::{new_arch, Arch, RelocationOverride, RelocationOverrideTarget},
1515
diff::DiffObjConfig,
1616
obj::{
17-
FlowAnalysisResult, Object, Relocation, RelocationFlags, Section, SectionData, SectionFlag,
18-
SectionKind, Symbol, SymbolFlag, SymbolKind,
19-
split_meta::{SPLITMETA_SECTION, SplitMeta},
17+
split_meta::{SplitMeta, SPLITMETA_SECTION}, DiffSide, FlowAnalysisResult, Object, Relocation, RelocationFlags, Section, SectionData, SectionFlag, SectionKind, Symbol, SymbolFlag, SymbolKind
2018
},
2119
util::{align_data_slice_to, align_u64_to, read_u16, read_u32},
2220
};
@@ -925,21 +923,21 @@ fn do_combine_sections(
925923
}
926924

927925
#[cfg(feature = "std")]
928-
pub fn read(obj_path: &std::path::Path, config: &DiffObjConfig) -> Result<Object> {
926+
pub fn read(obj_path: &std::path::Path, config: &DiffObjConfig, diff_side: DiffSide) -> Result<Object> {
929927
let (data, timestamp) = {
930928
let file = std::fs::File::open(obj_path)?;
931929
let timestamp = filetime::FileTime::from_last_modification_time(&file.metadata()?);
932930
(unsafe { memmap2::Mmap::map(&file) }?, timestamp)
933931
};
934-
let mut obj = parse(&data, config)?;
932+
let mut obj = parse(&data, config, diff_side)?;
935933
obj.path = Some(obj_path.to_path_buf());
936934
obj.timestamp = Some(timestamp);
937935
Ok(obj)
938936
}
939937

940-
pub fn parse(data: &[u8], config: &DiffObjConfig) -> Result<Object> {
938+
pub fn parse(data: &[u8], config: &DiffObjConfig, diff_side: DiffSide) -> Result<Object> {
941939
let obj_file = object::File::parse(data)?;
942-
let mut arch = new_arch(&obj_file)?;
940+
let mut arch = new_arch(&obj_file, diff_side)?;
943941
let split_meta = parse_split_meta(&obj_file)?;
944942
let (mut sections, section_indices) =
945943
map_sections(arch.as_ref(), &obj_file, split_meta.as_ref())?;

0 commit comments

Comments
 (0)