Skip to content

Commit e2c7034

Browse files
ethteckethteck
authored
Standardize the values for invalid and data opcodes (#261)
* Standardize the value for an invalid opcode > > This makes it so that all arches share the same value for an invalid opcode, so platform-specific logic isn't needed for checking whether instructions are valid. Also updated dependencies * OPCODE_DATA too
1 parent e6035b0 commit e2c7034

11 files changed

+439
-338
lines changed

‎Cargo.lock‎

Lines changed: 398 additions & 298 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎objdiff-core/src/arch/arm.rs‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ use object::{Endian as _, Object as _, ObjectSection as _, ObjectSymbol as _, el
1111
use unarm::{args, arm, thumb};
1212

1313
use crate::{
14-
arch::{Arch, RelocationOverride, RelocationOverrideTarget},
14+
arch::{Arch, OPCODE_DATA, OPCODE_INVALID, RelocationOverride, RelocationOverrideTarget},
1515
diff::{ArmArchVersion, ArmR9Usage, DiffObjConfig, display::InstructionPart},
1616
obj::{
1717
InstructionRef, Relocation, RelocationFlags, ResolvedInstructionRef, ResolvedRelocation,
@@ -164,7 +164,7 @@ impl ArchArm {
164164
}
165165
_ => bail!("Invalid instruction size {}", ins_ref.size),
166166
};
167-
let (ins, parsed_ins) = if ins_ref.opcode == u16::MAX {
167+
let (ins, parsed_ins) = if ins_ref.opcode == OPCODE_DATA {
168168
let mut args = args::Arguments::default();
169169
args[0] = args::Argument::UImm(code);
170170
let mnemonic = if ins_ref.size == 4 { ".word" } else { ".hword" };
@@ -238,7 +238,7 @@ impl Arch for ArchArm {
238238
ops.push(InstructionRef {
239239
address: address as u64,
240240
size: data.len() as u8,
241-
opcode: u16::MAX,
241+
opcode: OPCODE_DATA,
242242
branch_dest: None,
243243
});
244244
break;
@@ -257,7 +257,7 @@ impl Arch for ArchArm {
257257
ops.push(InstructionRef {
258258
address: address as u64,
259259
size: ins_size as u8,
260-
opcode: u16::MAX,
260+
opcode: OPCODE_INVALID,
261261
branch_dest: None,
262262
});
263263
address += ins_size as u32;
@@ -318,7 +318,7 @@ impl Arch for ArchArm {
318318
};
319319
(opcode, branch_dest)
320320
}
321-
unarm::ParseMode::Data => (u16::MAX, None),
321+
unarm::ParseMode::Data => (OPCODE_DATA, None),
322322
};
323323

324324
ops.push(InstructionRef {

‎objdiff-core/src/arch/arm64.rs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ use yaxpeax_arm::armv8::a64::{
1414
};
1515

1616
use crate::{
17-
arch::Arch,
17+
arch::{Arch, OPCODE_INVALID},
1818
diff::{DiffObjConfig, display::InstructionPart},
1919
obj::{
2020
InstructionRef, Relocation, RelocationFlags, ResolvedInstructionRef, ResolvedRelocation,
@@ -60,7 +60,7 @@ impl Arch for ArchArm64 {
6060
ops.push(InstructionRef {
6161
address,
6262
size: 4,
63-
opcode: u16::MAX,
63+
opcode: OPCODE_INVALID,
6464
branch_dest: None,
6565
});
6666
continue;
@@ -87,7 +87,7 @@ impl Arch for ArchArm64 {
8787
let decoder = InstDecoder::default();
8888
let mut ins = Instruction::default();
8989
if decoder.decode_into(&mut ins, &mut reader).is_err() {
90-
cb(InstructionPart::opcode("<invalid>", u16::MAX))?;
90+
cb(InstructionPart::opcode("<invalid>", OPCODE_INVALID))?;
9191
return Ok(());
9292
}
9393

@@ -2295,7 +2295,7 @@ where Cb: FnMut(InstructionPart<'static>) {
22952295
// Opcode is #[repr(u16)], but the tuple variants negate that, so we have to do this instead.
22962296
const fn opcode_to_u16(opcode: Opcode) -> u16 {
22972297
match opcode {
2298-
Opcode::Invalid => u16::MAX,
2298+
Opcode::Invalid => OPCODE_INVALID,
22992299
Opcode::UDF => 0,
23002300
Opcode::MOVN => 1,
23012301
Opcode::MOVK => 2,

‎objdiff-core/src/arch/mod.rs‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,9 @@ pub mod superh;
4141
#[cfg(feature = "x86")]
4242
pub mod x86;
4343

44+
pub const OPCODE_INVALID: u16 = u16::MAX;
45+
pub const OPCODE_DATA: u16 = u16::MAX - 1;
46+
4447
/// Represents the type of data associated with an instruction
4548
#[derive(PartialEq)]
4649
pub enum DataType {

‎objdiff-core/src/arch/x86.rs‎

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ use iced_x86::{
99
use object::{Endian as _, Object as _, ObjectSection as _, elf, pe};
1010

1111
use crate::{
12-
arch::{Arch, RelocationOverride, RelocationOverrideTarget},
12+
arch::{Arch, OPCODE_DATA, RelocationOverride, RelocationOverrideTarget},
1313
diff::{DiffObjConfig, X86Formatter, display::InstructionPart},
1414
obj::{InstructionRef, Relocation, RelocationFlags, ResolvedInstructionRef, Section, Symbol},
1515
};
@@ -89,8 +89,6 @@ impl ArchX86 {
8989
}
9090
}
9191

92-
const DATA_OPCODE: u16 = u16::MAX - 1;
93-
9492
impl Arch for ArchX86 {
9593
fn scan_instructions_internal(
9694
&self,
@@ -121,7 +119,7 @@ impl Arch for ArchX86 {
121119
out.push(InstructionRef {
122120
address,
123121
size: size as u8,
124-
opcode: DATA_OPCODE,
122+
opcode: OPCODE_DATA,
125123
branch_dest: None,
126124
});
127125

@@ -148,7 +146,7 @@ impl Arch for ArchX86 {
148146
out.push(InstructionRef {
149147
address: indirect_array_address + i as u64,
150148
size: 1,
151-
opcode: DATA_OPCODE,
149+
opcode: OPCODE_DATA,
152150
branch_dest: None,
153151
});
154152
}
@@ -187,14 +185,14 @@ impl Arch for ArchX86 {
187185
diff_config: &DiffObjConfig,
188186
cb: &mut dyn FnMut(InstructionPart) -> Result<()>,
189187
) -> Result<()> {
190-
if resolved.ins_ref.opcode == DATA_OPCODE {
188+
if resolved.ins_ref.opcode == OPCODE_DATA {
191189
let (mnemonic, imm) = match resolved.ins_ref.size {
192190
1 => (".byte", resolved.code[0] as u64),
193191
2 => (".word", self.endianness.read_u16_bytes(resolved.code.try_into()?) as u64),
194192
4 => (".dword", self.endianness.read_u32_bytes(resolved.code.try_into()?) as u64),
195193
_ => bail!("Unsupported x86 inline data size {}", resolved.ins_ref.size),
196194
};
197-
cb(InstructionPart::opcode(mnemonic, DATA_OPCODE))?;
195+
cb(InstructionPart::opcode(mnemonic, OPCODE_DATA))?;
198196
if resolved.relocation.is_some() {
199197
cb(InstructionPart::reloc())?;
200198
} else {
@@ -836,7 +834,7 @@ mod test {
836834
ins_ref: InstructionRef {
837835
address: 0x1234,
838836
size: 1,
839-
opcode: DATA_OPCODE,
837+
opcode: OPCODE_DATA,
840838
branch_dest: None,
841839
},
842840
code: &code,
@@ -850,7 +848,7 @@ mod test {
850848
)
851849
.unwrap();
852850
assert_eq!(parts, &[
853-
InstructionPart::opcode(".byte", DATA_OPCODE),
851+
InstructionPart::opcode(".byte", OPCODE_DATA),
854852
InstructionPart::unsigned(0xABu64),
855853
]);
856854
}

‎objdiff-core/tests/snapshots/arch_arm__combine_text_sections-2.snap‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,4 +4,4 @@ expression: output
44
---
55
[(Line(90), Dim, 5), (Address(0), Dim, 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)]
66
[(Line(90), Dim, 5), (Address(4), Dim, 5), (Spacing(4), Normal, 0), (Opcode("bx", 32779), Normal, 10), (Argument(Opaque("r12")), Normal, 0), (Eol, Normal, 0)]
7-
[(Line(90), Dim, 5), (Address(8), Dim, 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)]
7+
[(Line(90), Dim, 5), (Address(8), Dim, 5), (Spacing(4), Normal, 0), (Opcode(".word", 65534), 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)]

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ expression: diff.instruction_rows
3636
InstructionRef {
3737
address: 84,
3838
size: 4,
39-
opcode: 65535,
39+
opcode: 65534,
4040
branch_dest: None,
4141
},
4242
),

‎objdiff-core/tests/snapshots/arch_arm__read_arm-2.snap‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1829,7 +1829,7 @@ expression: diff.instruction_rows
18291829
InstructionRef {
18301830
address: 460,
18311831
size: 4,
1832-
opcode: 65535,
1832+
opcode: 65534,
18331833
branch_dest: None,
18341834
},
18351835
),
@@ -1843,7 +1843,7 @@ expression: diff.instruction_rows
18431843
InstructionRef {
18441844
address: 464,
18451845
size: 4,
1846-
opcode: 65535,
1846+
opcode: 65534,
18471847
branch_dest: None,
18481848
},
18491849
),
@@ -1864,7 +1864,7 @@ expression: diff.instruction_rows
18641864
InstructionRef {
18651865
address: 468,
18661866
size: 4,
1867-
opcode: 65535,
1867+
opcode: 65534,
18681868
branch_dest: None,
18691869
},
18701870
),

‎objdiff-core/tests/snapshots/arch_arm__read_arm-3.snap‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,6 @@ expression: output
107107
[(Address(408), Dim, 5), (Basic(" ~> "), Rotating(16), 0), (Opcode("mov", 32818), Normal, 10), (Argument(Opaque("r0")), Normal, 0), (Basic(", "), Normal, 0), (Basic("#"), Normal, 0), (Argument(Unsigned(0)), Normal, 0), (Eol, Normal, 0)]
108108
[(Address(412), Dim, 5), (Spacing(4), Normal, 0), (Opcode("strb", 32899), Normal, 10), (Argument(Opaque("r0")), Normal, 0), (Basic(", "), Normal, 0), (Basic("["), Normal, 0), (Argument(Opaque("r5")), Normal, 0), (Basic(", "), Normal, 0), (Basic("#"), Normal, 0), (Argument(Signed(38)), Normal, 0), (Basic("]"), Normal, 0), (Eol, Normal, 0)]
109109
[(Address(416), Dim, 5), (Spacing(4), Normal, 0), (Opcode("ldmia", 32793), Normal, 10), (Argument(Opaque("sp")), Normal, 0), (Argument(Opaque("!")), Normal, 0), (Basic(", "), Normal, 0), (Basic("{"), Normal, 0), (Argument(Opaque("r4")), Normal, 0), (Basic(", "), Normal, 0), (Argument(Opaque("r5")), Normal, 0), (Basic(", "), Normal, 0), (Argument(Opaque("r6")), Normal, 0), (Basic(", "), Normal, 0), (Argument(Opaque("pc")), Normal, 0), (Basic("}"), Normal, 0), (Eol, Normal, 0)]
110-
[(Address(420), Dim, 5), (Spacing(4), Normal, 0), (Opcode(".word", 65535), Normal, 10), (Symbol(Symbol { name: "data_027e103c", demangled_name: None, address: 0, size: 0, kind: Unknown, section: None, flags: FlagSet(Global | Weak), align: None, virtual_address: None }), Bright, 0), (Eol, Normal, 0)]
111-
[(Address(424), Dim, 5), (Basic(" ~> "), Rotating(8), 0), (Opcode(".word", 65535), Normal, 10), (Symbol(Symbol { name: "data_027e1098", demangled_name: None, address: 0, size: 0, kind: Unknown, section: None, flags: FlagSet(Global | Weak), align: None, virtual_address: None }), Bright, 0), (Eol, Normal, 0)]
112-
[(Address(428), Dim, 5), (Spacing(4), Normal, 0), (Opcode(".word", 65535), Normal, 10), (Symbol(Symbol { name: "gPlayerControl", demangled_name: None, address: 0, size: 0, kind: Unknown, section: None, flags: FlagSet(Global | Weak), align: None, virtual_address: None }), Bright, 0), (Eol, Normal, 0)]
110+
[(Address(420), Dim, 5), (Spacing(4), Normal, 0), (Opcode(".word", 65534), Normal, 10), (Symbol(Symbol { name: "data_027e103c", demangled_name: None, address: 0, size: 0, kind: Unknown, section: None, flags: FlagSet(Global | Weak), align: None, virtual_address: None }), Bright, 0), (Eol, Normal, 0)]
111+
[(Address(424), Dim, 5), (Basic(" ~> "), Rotating(8), 0), (Opcode(".word", 65534), Normal, 10), (Symbol(Symbol { name: "data_027e1098", demangled_name: None, address: 0, size: 0, kind: Unknown, section: None, flags: FlagSet(Global | Weak), align: None, virtual_address: None }), Bright, 0), (Eol, Normal, 0)]
112+
[(Address(428), Dim, 5), (Spacing(4), Normal, 0), (Opcode(".word", 65534), Normal, 10), (Symbol(Symbol { name: "gPlayerControl", demangled_name: None, address: 0, size: 0, kind: Unknown, section: None, flags: FlagSet(Global | Weak), align: None, virtual_address: None }), Bright, 0), (Eol, Normal, 0)]

‎objdiff-core/tests/snapshots/arch_arm__read_thumb-2.snap‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1512,7 +1512,7 @@ expression: diff.instruction_rows
15121512
InstructionRef {
15131513
address: 216,
15141514
size: 4,
1515-
opcode: 65535,
1515+
opcode: 65534,
15161516
branch_dest: None,
15171517
},
15181518
),
@@ -1526,7 +1526,7 @@ expression: diff.instruction_rows
15261526
InstructionRef {
15271527
address: 220,
15281528
size: 4,
1529-
opcode: 65535,
1529+
opcode: 65534,
15301530
branch_dest: None,
15311531
},
15321532
),
@@ -1540,7 +1540,7 @@ expression: diff.instruction_rows
15401540
InstructionRef {
15411541
address: 224,
15421542
size: 4,
1543-
opcode: 65535,
1543+
opcode: 65534,
15441544
branch_dest: None,
15451545
},
15461546
),
@@ -1554,7 +1554,7 @@ expression: diff.instruction_rows
15541554
InstructionRef {
15551555
address: 228,
15561556
size: 4,
1557-
opcode: 65535,
1557+
opcode: 65534,
15581558
branch_dest: None,
15591559
},
15601560
),
@@ -1568,7 +1568,7 @@ expression: diff.instruction_rows
15681568
InstructionRef {
15691569
address: 232,
15701570
size: 4,
1571-
opcode: 65535,
1571+
opcode: 65534,
15721572
branch_dest: None,
15731573
},
15741574
),
@@ -1582,7 +1582,7 @@ expression: diff.instruction_rows
15821582
InstructionRef {
15831583
address: 236,
15841584
size: 4,
1585-
opcode: 65535,
1585+
opcode: 65534,
15861586
branch_dest: None,
15871587
},
15881588
),
@@ -1596,7 +1596,7 @@ expression: diff.instruction_rows
15961596
InstructionRef {
15971597
address: 240,
15981598
size: 4,
1599-
opcode: 65535,
1599+
opcode: 65534,
16001600
branch_dest: None,
16011601
},
16021602
),

0 commit comments

Comments
 (0)