Skip to content

Commit 5898d7a

Browse files
encounterencounter
committed
Fix section ordering with many same-named sections
Before, this was comparing, for example, `.text-2` with `.text-10` with standard string comparison, yielding `.text-10` before `.text-2`. Instead, this simply uses a stable sort by name, which preserves the relative ordering of sections.
1 parent ffb38d1 commit 5898d7a

File tree

4 files changed

+227
-2
lines changed

4 files changed

+227
-2
lines changed

‎objdiff-core/src/diff/display.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -701,7 +701,7 @@ pub fn display_sections(
701701
});
702702
}
703703
}
704-
sections.sort_by(|a, b| a.id.cmp(&b.id));
704+
sections.sort_by(|a, b| a.name.cmp(&b.name));
705705
sections
706706
}
707707

‎objdiff-core/tests/arch_x86.rs

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
use objdiff_core::{diff, obj};
1+
use objdiff_core::{diff, diff::display::SymbolFilter, obj};
22

33
mod common;
44

@@ -40,3 +40,18 @@ fn read_x86_64() {
4040
let output = common::display_diff(&obj, &diff, symbol_idx, &diff_config);
4141
insta::assert_snapshot!(output);
4242
}
43+
44+
#[test]
45+
#[cfg(feature = "x86")]
46+
fn display_section_ordering() {
47+
let diff_config = diff::DiffObjConfig::default();
48+
let obj = obj::read::parse(include_object!("data/x86/basenode.obj"), &diff_config).unwrap();
49+
let obj_diff =
50+
diff::diff_objs(Some(&obj), None, None, &diff_config, &diff::MappingConfig::default())
51+
.unwrap()
52+
.left
53+
.unwrap();
54+
let section_display =
55+
diff::display::display_sections(&obj, &obj_diff, SymbolFilter::None, false, false, false);
56+
insta::assert_debug_snapshot!(section_display);
57+
}

‎objdiff-core/tests/data/x86/basenode.obj

7.08 KB
Binary file not shown.

‎objdiff-core/tests/snapshots/arch_x86__display_section_ordering.snap

Lines changed: 210 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,210 @@
1+
---
2+
source: objdiff-core/tests/arch_x86.rs
3+
expression: section_display
4+
---
5+
[
6+
SectionDisplay {
7+
id: ".text-0",
8+
name: ".text",
9+
size: 47,
10+
match_percent: None,
11+
symbols: [
12+
SectionDisplaySymbol {
13+
symbol: 28,
14+
is_mapping_symbol: false,
15+
},
16+
],
17+
},
18+
SectionDisplay {
19+
id: ".text-1",
20+
name: ".text",
21+
size: 65,
22+
match_percent: None,
23+
symbols: [
24+
SectionDisplaySymbol {
25+
symbol: 29,
26+
is_mapping_symbol: false,
27+
},
28+
],
29+
},
30+
SectionDisplay {
31+
id: ".text-2",
32+
name: ".text",
33+
size: 5,
34+
match_percent: None,
35+
symbols: [
36+
SectionDisplaySymbol {
37+
symbol: 30,
38+
is_mapping_symbol: false,
39+
},
40+
],
41+
},
42+
SectionDisplay {
43+
id: ".text-3",
44+
name: ".text",
45+
size: 141,
46+
match_percent: None,
47+
symbols: [
48+
SectionDisplaySymbol {
49+
symbol: 31,
50+
is_mapping_symbol: false,
51+
},
52+
],
53+
},
54+
SectionDisplay {
55+
id: ".text-4",
56+
name: ".text",
57+
size: 120,
58+
match_percent: None,
59+
symbols: [
60+
SectionDisplaySymbol {
61+
symbol: 34,
62+
is_mapping_symbol: false,
63+
},
64+
],
65+
},
66+
SectionDisplay {
67+
id: ".text-5",
68+
name: ".text",
69+
size: 378,
70+
match_percent: None,
71+
symbols: [
72+
SectionDisplaySymbol {
73+
symbol: 37,
74+
is_mapping_symbol: false,
75+
},
76+
],
77+
},
78+
SectionDisplay {
79+
id: ".text-6",
80+
name: ".text",
81+
size: 130,
82+
match_percent: None,
83+
symbols: [
84+
SectionDisplaySymbol {
85+
symbol: 38,
86+
is_mapping_symbol: false,
87+
},
88+
],
89+
},
90+
SectionDisplay {
91+
id: ".text-7",
92+
name: ".text",
93+
size: 123,
94+
match_percent: None,
95+
symbols: [
96+
SectionDisplaySymbol {
97+
symbol: 39,
98+
is_mapping_symbol: false,
99+
},
100+
],
101+
},
102+
SectionDisplay {
103+
id: ".text-8",
104+
name: ".text",
105+
size: 70,
106+
match_percent: None,
107+
symbols: [
108+
SectionDisplaySymbol {
109+
symbol: 40,
110+
is_mapping_symbol: false,
111+
},
112+
],
113+
},
114+
SectionDisplay {
115+
id: ".text-9",
116+
name: ".text",
117+
size: 90,
118+
match_percent: None,
119+
symbols: [
120+
SectionDisplaySymbol {
121+
symbol: 41,
122+
is_mapping_symbol: false,
123+
},
124+
],
125+
},
126+
SectionDisplay {
127+
id: ".text-10",
128+
name: ".text",
129+
size: 82,
130+
match_percent: None,
131+
symbols: [
132+
SectionDisplaySymbol {
133+
symbol: 42,
134+
is_mapping_symbol: false,
135+
},
136+
],
137+
},
138+
SectionDisplay {
139+
id: ".text-11",
140+
name: ".text",
141+
size: 336,
142+
match_percent: None,
143+
symbols: [
144+
SectionDisplaySymbol {
145+
symbol: 43,
146+
is_mapping_symbol: false,
147+
},
148+
],
149+
},
150+
SectionDisplay {
151+
id: ".text-12",
152+
name: ".text",
153+
size: 193,
154+
match_percent: None,
155+
symbols: [
156+
SectionDisplaySymbol {
157+
symbol: 50,
158+
is_mapping_symbol: false,
159+
},
160+
],
161+
},
162+
SectionDisplay {
163+
id: ".text-13",
164+
name: ".text",
165+
size: 544,
166+
match_percent: None,
167+
symbols: [
168+
SectionDisplaySymbol {
169+
symbol: 53,
170+
is_mapping_symbol: false,
171+
},
172+
],
173+
},
174+
SectionDisplay {
175+
id: ".text-14",
176+
name: ".text",
177+
size: 250,
178+
match_percent: None,
179+
symbols: [
180+
SectionDisplaySymbol {
181+
symbol: 56,
182+
is_mapping_symbol: false,
183+
},
184+
],
185+
},
186+
SectionDisplay {
187+
id: ".text-15",
188+
name: ".text",
189+
size: 89,
190+
match_percent: None,
191+
symbols: [
192+
SectionDisplaySymbol {
193+
symbol: 57,
194+
is_mapping_symbol: false,
195+
},
196+
],
197+
},
198+
SectionDisplay {
199+
id: ".text-16",
200+
name: ".text",
201+
size: 119,
202+
match_percent: None,
203+
symbols: [
204+
SectionDisplaySymbol {
205+
symbol: 60,
206+
is_mapping_symbol: false,
207+
},
208+
],
209+
},
210+
]

0 commit comments

Comments
 (0)