Skip to content

Commit 1fb5b31

Browse files
ethteckethteck
committed
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
1 parent e6035b0 commit 1fb5b31

11 files changed

+595
-433
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: 12 additions & 9 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_INVALID, RelocationOverride, RelocationOverrideTarget},
1515
diff::{ArmArchVersion, ArmR9Usage, DiffObjConfig, display::InstructionPart},
1616
obj::{
1717
InstructionRef, Relocation, RelocationFlags, ResolvedInstructionRef, ResolvedRelocation,
@@ -48,10 +48,13 @@ impl ArchArm {
4848
&& s.name() == Ok(".ARM.attributes")
4949
}) {
5050
let attr_data = arm_attrs.uncompressed_data()?;
51-
let build_attrs = BuildAttrs::new(&attr_data, match file.endianness() {
52-
object::Endianness::Little => arm_attr::Endian::Little,
53-
object::Endianness::Big => arm_attr::Endian::Big,
54-
})?;
51+
let build_attrs = BuildAttrs::new(
52+
&attr_data,
53+
match file.endianness() {
54+
object::Endianness::Little => arm_attr::Endian::Little,
55+
object::Endianness::Big => arm_attr::Endian::Big,
56+
},
57+
)?;
5558
for subsection in build_attrs.subsections() {
5659
let subsection = subsection?;
5760
if !subsection.is_aeabi() {
@@ -164,7 +167,7 @@ impl ArchArm {
164167
}
165168
_ => bail!("Invalid instruction size {}", ins_ref.size),
166169
};
167-
let (ins, parsed_ins) = if ins_ref.opcode == u16::MAX {
170+
let (ins, parsed_ins) = if ins_ref.opcode == OPCODE_INVALID {
168171
let mut args = args::Arguments::default();
169172
args[0] = args::Argument::UImm(code);
170173
let mnemonic = if ins_ref.size == 4 { ".word" } else { ".hword" };
@@ -238,7 +241,7 @@ impl Arch for ArchArm {
238241
ops.push(InstructionRef {
239242
address: address as u64,
240243
size: data.len() as u8,
241-
opcode: u16::MAX,
244+
opcode: OPCODE_INVALID,
242245
branch_dest: None,
243246
});
244247
break;
@@ -257,7 +260,7 @@ impl Arch for ArchArm {
257260
ops.push(InstructionRef {
258261
address: address as u64,
259262
size: ins_size as u8,
260-
opcode: u16::MAX,
263+
opcode: OPCODE_INVALID,
261264
branch_dest: None,
262265
});
263266
address += ins_size as u32;
@@ -318,7 +321,7 @@ impl Arch for ArchArm {
318321
};
319322
(opcode, branch_dest)
320323
}
321-
unarm::ParseMode::Data => (u16::MAX, None),
324+
unarm::ParseMode::Data => (OPCODE_INVALID, None),
322325
};
323326

324327
ops.push(InstructionRef {

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

Lines changed: 40 additions & 16 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,
@@ -25,7 +25,9 @@ use crate::{
2525
pub struct ArchArm64 {}
2626

2727
impl ArchArm64 {
28-
pub fn new(_file: &object::File) -> Result<Self> { Ok(Self {}) }
28+
pub fn new(_file: &object::File) -> Result<Self> {
29+
Ok(Self {})
30+
}
2931
}
3032

3133
impl Arch for ArchArm64 {
@@ -60,7 +62,7 @@ impl Arch for ArchArm64 {
6062
ops.push(InstructionRef {
6163
address,
6264
size: 4,
63-
opcode: u16::MAX,
65+
opcode: OPCODE_INVALID,
6466
branch_dest: None,
6567
});
6668
continue;
@@ -87,7 +89,7 @@ impl Arch for ArchArm64 {
8789
let decoder = InstDecoder::default();
8890
let mut ins = Instruction::default();
8991
if decoder.decode_into(&mut ins, &mut reader).is_err() {
90-
cb(InstructionPart::opcode("<invalid>", u16::MAX))?;
92+
cb(InstructionPart::opcode("<invalid>", OPCODE_INVALID))?;
9193
return Ok(());
9294
}
9395

@@ -196,7 +198,9 @@ struct DisplayCtx<'a> {
196198
// Reworked for more structured output. The library only gives us a Display impl, and no way to
197199
// capture any of this information, so it needs to be reimplemented here.
198200
fn display_instruction<Cb>(args: &mut Cb, ins: &Instruction, ctx: &mut DisplayCtx) -> &'static str
199-
where Cb: FnMut(InstructionPart<'static>) {
201+
where
202+
Cb: FnMut(InstructionPart<'static>),
203+
{
200204
let mnemonic = match ins.opcode {
201205
Opcode::Invalid => return "<invalid>",
202206
Opcode::UDF => "udf",
@@ -2011,13 +2015,17 @@ fn condition_code(cond: u8) -> &'static str {
20112015

20122016
#[inline]
20132017
fn push_register<Cb>(args: &mut Cb, size: SizeCode, reg: u16, sp: bool)
2014-
where Cb: FnMut(InstructionPart<'static>) {
2018+
where
2019+
Cb: FnMut(InstructionPart<'static>),
2020+
{
20152021
push_opaque(args, reg_name(size, reg, sp));
20162022
}
20172023

20182024
#[inline]
20192025
fn push_shift<Cb>(args: &mut Cb, style: ShiftStyle, amount: u8)
2020-
where Cb: FnMut(InstructionPart<'static>) {
2026+
where
2027+
Cb: FnMut(InstructionPart<'static>),
2028+
{
20212029
push_opaque(args, shift_style(style));
20222030
if amount != 0 {
20232031
push_plain(args, " ");
@@ -2027,12 +2035,16 @@ where Cb: FnMut(InstructionPart<'static>) {
20272035

20282036
#[inline]
20292037
fn push_condition_code<Cb>(args: &mut Cb, cond: u8)
2030-
where Cb: FnMut(InstructionPart<'static>) {
2038+
where
2039+
Cb: FnMut(InstructionPart<'static>),
2040+
{
20312041
push_opaque(args, condition_code(cond));
20322042
}
20332043

20342044
fn push_barrier<Cb>(args: &mut Cb, option: u8)
2035-
where Cb: FnMut(InstructionPart<'static>) {
2045+
where
2046+
Cb: FnMut(InstructionPart<'static>),
2047+
{
20362048
match option {
20372049
0b0001 => push_opaque(args, "oshld"),
20382050
0b0010 => push_opaque(args, "oshst"),
@@ -2052,32 +2064,42 @@ where Cb: FnMut(InstructionPart<'static>) {
20522064

20532065
#[inline]
20542066
fn push_opaque<'a, Cb>(args: &mut Cb, text: &'a str)
2055-
where Cb: FnMut(InstructionPart<'a>) {
2067+
where
2068+
Cb: FnMut(InstructionPart<'a>),
2069+
{
20562070
args(InstructionPart::opaque(text));
20572071
}
20582072

20592073
#[inline]
20602074
fn push_plain<Cb>(args: &mut Cb, text: &'static str)
2061-
where Cb: FnMut(InstructionPart<'static>) {
2075+
where
2076+
Cb: FnMut(InstructionPart<'static>),
2077+
{
20622078
args(InstructionPart::basic(text));
20632079
}
20642080

20652081
#[inline]
20662082
fn push_separator<Cb>(args: &mut Cb)
2067-
where Cb: FnMut(InstructionPart<'static>) {
2083+
where
2084+
Cb: FnMut(InstructionPart<'static>),
2085+
{
20682086
args(InstructionPart::separator());
20692087
}
20702088

20712089
#[inline]
20722090
fn push_unsigned<Cb>(args: &mut Cb, v: u64)
2073-
where Cb: FnMut(InstructionPart<'static>) {
2091+
where
2092+
Cb: FnMut(InstructionPart<'static>),
2093+
{
20742094
push_plain(args, "#");
20752095
args(InstructionPart::unsigned(v));
20762096
}
20772097

20782098
#[inline]
20792099
fn push_signed<Cb>(args: &mut Cb, v: i64)
2080-
where Cb: FnMut(InstructionPart<'static>) {
2100+
where
2101+
Cb: FnMut(InstructionPart<'static>),
2102+
{
20812103
push_plain(args, "#");
20822104
args(InstructionPart::signed(v));
20832105
}
@@ -2117,7 +2139,9 @@ fn is_reg_index_reloc(resolved: Option<ResolvedRelocation>) -> bool {
21172139
}
21182140

21192141
fn push_operand<Cb>(args: &mut Cb, o: &Operand, ctx: &mut DisplayCtx)
2120-
where Cb: FnMut(InstructionPart<'static>) {
2142+
where
2143+
Cb: FnMut(InstructionPart<'static>),
2144+
{
21212145
match o {
21222146
Operand::Nothing => unreachable!(),
21232147
Operand::PCOffset(off) => {
@@ -2295,7 +2319,7 @@ where Cb: FnMut(InstructionPart<'static>) {
22952319
// Opcode is #[repr(u16)], but the tuple variants negate that, so we have to do this instead.
22962320
const fn opcode_to_u16(opcode: Opcode) -> u16 {
22972321
match opcode {
2298-
Opcode::Invalid => u16::MAX,
2322+
Opcode::Invalid => OPCODE_INVALID,
22992323
Opcode::UDF => 0,
23002324
Opcode::MOVN => 1,
23012325
Opcode::MOVK => 2,

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

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

44+
pub const OPCODE_INVALID: u16 = u16::MAX - 1;
45+
4446
/// Represents the type of data associated with an instruction
4547
#[derive(PartialEq)]
4648
pub enum DataType {
@@ -140,20 +142,26 @@ impl DataType {
140142
DataType::Float => {
141143
let bytes: [u8; 4] = bytes.try_into().unwrap();
142144
strs.push((
143-
format!("{:?}f", match endian {
144-
object::Endianness::Little => f32::from_le_bytes(bytes),
145-
object::Endianness::Big => f32::from_be_bytes(bytes),
146-
}),
145+
format!(
146+
"{:?}f",
147+
match endian {
148+
object::Endianness::Little => f32::from_le_bytes(bytes),
149+
object::Endianness::Big => f32::from_be_bytes(bytes),
150+
}
151+
),
147152
None,
148153
));
149154
}
150155
DataType::Double => {
151156
let bytes: [u8; 8] = bytes.try_into().unwrap();
152157
strs.push((
153-
format!("{:?}", match endian {
154-
object::Endianness::Little => f64::from_le_bytes(bytes),
155-
object::Endianness::Big => f64::from_be_bytes(bytes),
156-
}),
158+
format!(
159+
"{:?}",
160+
match endian {
161+
object::Endianness::Little => f64::from_le_bytes(bytes),
162+
object::Endianness::Big => f64::from_be_bytes(bytes),
163+
}
164+
),
157165
None,
158166
));
159167
}
@@ -368,13 +376,19 @@ pub trait Arch: Any + Debug + Send + Sync {
368376
Ok(None)
369377
}
370378

371-
fn demangle(&self, _name: &str) -> Option<String> { None }
379+
fn demangle(&self, _name: &str) -> Option<String> {
380+
None
381+
}
372382

373-
fn reloc_name(&self, _flags: RelocationFlags) -> Option<&'static str> { None }
383+
fn reloc_name(&self, _flags: RelocationFlags) -> Option<&'static str> {
384+
None
385+
}
374386

375387
fn data_reloc_size(&self, flags: RelocationFlags) -> usize;
376388

377-
fn symbol_address(&self, address: u64, _kind: SymbolKind) -> u64 { address }
389+
fn symbol_address(&self, address: u64, _kind: SymbolKind) -> u64 {
390+
address
391+
}
378392

379393
fn extra_symbol_flags(&self, _symbol: &object::Symbol) -> SymbolFlagSet {
380394
SymbolFlagSet::default()
@@ -388,9 +402,13 @@ pub trait Arch: Any + Debug + Send + Sync {
388402
None
389403
}
390404

391-
fn symbol_hover(&self, _obj: &Object, _symbol_index: usize) -> Vec<HoverItem> { Vec::new() }
405+
fn symbol_hover(&self, _obj: &Object, _symbol_index: usize) -> Vec<HoverItem> {
406+
Vec::new()
407+
}
392408

393-
fn symbol_context(&self, _obj: &Object, _symbol_index: usize) -> Vec<ContextItem> { Vec::new() }
409+
fn symbol_context(&self, _obj: &Object, _symbol_index: usize) -> Vec<ContextItem> {
410+
Vec::new()
411+
}
394412

395413
fn instruction_hover(
396414
&self,
@@ -448,7 +466,9 @@ pub fn new_arch(object: &object::File, diff_side: DiffSide) -> Result<Box<dyn Ar
448466
pub struct ArchDummy {}
449467

450468
impl ArchDummy {
451-
pub fn new() -> Box<Self> { Box::new(Self {}) }
469+
pub fn new() -> Box<Self> {
470+
Box::new(Self {})
471+
}
452472
}
453473

454474
impl Arch for ArchDummy {
@@ -472,7 +492,9 @@ impl Arch for ArchDummy {
472492
Ok(())
473493
}
474494

475-
fn data_reloc_size(&self, _flags: RelocationFlags) -> usize { 0 }
495+
fn data_reloc_size(&self, _flags: RelocationFlags) -> usize {
496+
0
497+
}
476498
}
477499

478500
#[derive(Debug, Clone, Copy, PartialEq, Eq)]

0 commit comments

Comments
 (0)