Invalid branch offset

[rva3]

On the ARMv7 in the Thumb mode, unconditional BranchThumbOffset uses relative offset to the current instruction, however the conditional branches do +1 for some reason:

yaxpeax-arm/src/armv7/thumb.rs

Lines 4182 to 4186 in 5803a74

	inst.opcode = Opcode::B;
	let imm = instr2[0..8].load::<u8>() as i8 as i32;
	inst.condition = ConditionCode::build(opcode);
	inst.operands = [
	Operand::BranchThumbOffset(imm + 1),

Is there a reason for this? The workaround is simple

let fixup = if code.instruction.condition == ConditionCode::AL {
    4
} else {
    2
};
let pc = code.offset + fixup;

but I don't think it matches the ARM spec (where the PC + 4 is a general rule for the PC-relative stuff).

[iximeow]

the answer is really that i wildly misunderstood A32 when i'd initially written the decoder, and no one has told me how confusing Branch{Thumb}Offset is. BranchThumbOffset is worse than just off by one, it's the encoded (not-shifted) form of the offset, so for Thumb the useful value is BranchOffsetThumb(i) => i << 1. this was/is fixed up in the Display impl (???) and not really documented. sorry! i'm really not sure what i was thinking five years ago...

i also entirely did not understand that pc is architecturally two instructions ahead, so i'm fairly sure i'd misunderstood why the textual disassembly i was comparing against was the way it was.

i've been using the armv7 code myself over the last few weeks and found several additional issues mostly related to Thumb in addition to this. i hadn't actually pushed those patches anywhere yet (or cut a new release) because i think there's some cleanup i've got to do with the display impl.. but maybe i should do that sooner than later!

(where the PC + 4 is a general rule for the PC-relative stuff).

i assume you mean in Thumb specifically? i read your workaround as computing the fixup difference between the value in BranchThumbOffset and however much pc should be offset for a branch. in that case i think a reasonable expectation (which yaxpeax-arm does not currently meet!) would be that you could write more or less:

if let Operand::BranchThumbOffset(i) = inst.operands[0] && branch_taken(&inst ,..) {
    pc = pc.wrapping_add(i)
} else {
    pc = pc.wrapping_add(inst.len())
}

and similar for relative branches in A32, though that may be less useful for you.

[rva3]

i assume you mean in Thumb specifically

Yeah, for the Thumb mode it's + 4, for ARM it's + 8. I'm using pc + fixup + (i << 1), though the original workaround was using instruction length, guess I forgot there's wide instructions in Thumb.

found several additional issues mostly related to Thumb

The diff looks good, feel free to close this in case of a new release :)

and similar for relative branches in A32

I haven't played with A32 decoder yet, mostly because bare-metal is usually Thumb.