MSP430 and MSP430X fixes

[Sleigh-InSPECtor]

As part of a research project testing the accuracy of the SLEIGH specifications compared to real hardware, we observed unexpected behavior in a number of MSP430 and MSP430X instructions. As the fixes for these instructions where difficult to fully separate from each other this has been submitted as one pull request with each commit fixing a single issue. Where not mentioned the commit does not affect de-compilation to the best of our knowledge. The hardware compared to was an MSP430FR5969 and the documentation used was slau367p. TI's Code Composer Studios (CCS) was also used as a source of truth. To assist in checking these each heading links to the relevant commit.

Fix ADDA/SUBA/CMPA with CG registers

This commit adds support to the address instructions for using immediate from the constant generator and status register. The normal use of this is described in section 4.3.4. We have found that TI has added custom behavior to the normal addressing modes for the SR and CG to use these immediates. The behavior of these has been verified with hardware and the decompiler window of CCS.

Fix carry flag for SUBA

The carry flag for subtraction in msp430 should be set if there is not a borrow. This is opposite to what the documentation says for the SUBA instructions 4.6.4.10, but was found to be what the hardware does. It is also the same as the current behavior in sleigh for the SUB instruction. Currently the SUBA instruction does the opposite of this.

Fix issue with index address when base register is in low memory

When MSP430 instructions use the indexed addressing mode they should behave differently depending on if the base register is located in the upper or lower 64KB of memory. This is described in section 4.4.2. Currently when MSP430 instructions use the indexing mode with the base register in the lower 64KB of memory they do not mask the final address to the lower 64KB of memory.

It looks like ctx_isHi was meant to handle this, but as the value in the base register isn't known during disassembly and varies during execution, this cant be used.

This does show up in de-compilation.

// Before
*(undefined2 *)((ulong)(puVar1 + 9) & 0xffff) = uVar2;
// After
*(undefined2 *)((ulong)(puVar1 + 9) & ((ulong)(((ulong)(puVar1 + 2) & 0xf0000) != 0) * 0xf0000 | 0xffff)) = uVar2;

Fix upper register zeroing for single operand instructions

Single operand MSP430 word instructions should clear the upper 16-19 bits for MSP430X, this is described in 4.3.5. Double operand word instructions as well as single and double operand byte instructions current clear the upper bits, but single operand word instructions has been missed.

Use correct size for jump offset for CPUX mode

For MSP430X, the MSP430 JMP instruction should have the immediate sign extended to 20 bits and then added to the 20 bit PC as described in 4.5.1.3. Currently it is fixed to the MSP430 width of 16 bits.
This is incorrect as Ghidra seems to use the address and ignores the size restriction. This commit has no impact.

Fix handling of addressing modes that involve 20-bit immediates for 'X' suffix instructions

MSP430X instructions which use the extended instruction word encode the upper 4 bits of the immediate in the source field of the non-register mode extension word. This is described in section 4.5.2.2. Currently these bits are ignored for the immediate, label, and indexed address modes for these instructions.

Add handling for the RRUX instruction

The current note about the RRUX instruction is partially correct. CCS does replace RRUX.W and RRUX.A with RRUM.W #1, .. and RRUM.A #1, .. as it is a 1 word instead of a 2 word instruction. But it is incorrect for RRUX.B which it inserts a RRUX.B instructions for. The encoding for RRUX instructions is the RRCX instruction with the zero carry bit set, but is considered by the manual as its own instruction (4.5.2.4).

Reset overflow bit in RRCX

The RRCX instruction should reset the overflow bit (4.6.3.28), currently it does not.

Even align PC

The program counter should always have the least significant bit set to 0 (4.3.1). Currently there are some instructions which can write odd values to this registers. This fix could affect de-compilation of some of the MSP430X instructions which did not have masks already, as existing masks already show in de-compilation.

The postIncrementStore constructor is currently very repetitive and can be simplified. The ctx_al=1 condition for the address size is also the wrong way around, this prevents MSP430X instructions which use this table and write to the PC from jumping to above 64KB in memory. It also includes a redundant zero extend (zeroExtend) only for byte instructions.

Use signed immediate in symbolic mode

The symbolic addressing mode should add the signed immediate to the program counter (4.4.3). Currently it adds the unsigned immediate.

Word align word and address memory accesses

We have found that the hardware ignores the lower bit of word and address memory accesses to forcefully align them to 16 bits. While this is not mentioned in the documentation it has been mentioned as the correct behavior on TI's support forum Accessing unaligned memory.

Stop POPM from writing to SP and CG

POPM currently writes values to the stack pointer and constant generator when restoring registers from the stack. From testing with hardware we have found that the value in memory which is meant to be restored to the stack pointer is ignored. This value is meant to be the current SP value which is likely why this is the case. It is also incorrect to write any value to the constant generator register as it does not exist as a register.

ctx_al should default to 1

The ctx_al context field represents the A/L bit of the extension word. This bit is set to 1 for byte and word instructions and 0 for address instructions. To allow postRegIncrement and other constructors shared between the MSP430 and MSP430X instructions to work this field should default to 1.

Replaced dest_0_4 with new DST20_0_4

This commit replaces most occurrences of the dest_0_4 reg in the MSP430X.sinc file with a new constructor DEST20_0_4. This constructor sets the value of the PC to the start of the next instruction before exporting it. Currently the PC holds the address of the start of the current instruction which causes register accesses to read the wrong value.

Signed 20 bit values are not properly handled

As sleigh only supports byte sized registers, 20 bit values are stored in a larger register, minimum 3 bytes. Due to this signed operations like s<, s>>, and sext() do not work as the 20th bit is never the sign bit.

Mask address register accesses to 20 bits

MSP430X registers should hold at most 20 bits as that is the register width. Some instructions which read a value from memory or sign extend a smaller value to the full width allow writing larger values. These mask appear in de-compilation.

Added temporary for src in adda and suba macro

When the source and destination registers are the same the source get overwritten when writing the destination. This then produces wrong values for the flags.

POP.W POP.B need to delay building the destination reg

We have found that the POP instruction first reads the value from the stack, increments the stack pointer, and then moves the value found to the current destination. This is different to the current behavior of the POP instruction which first builds the destination. This fixes cases such as POP 0x0(SP) as they now write to memory at the address of the SP after incrementing.

MOV, MOVX, MOVA, and BRA @rn+ do not correctly order opperations

This is similar to the above fix as POP is an emulated instruction, therefore POP 0x0(SP) is actually MOV @SP+,0x0(SP). BRA is also an emulated instruction making BRA @rn+ really MOVA @rn+, PC. We have found that in general MOV instructions fetch the source, auto-increment, fetch the destination, and move what the source value was to the destination.

Fixed ordering of SP decrementing for CALLA

We have found that for CALLA SP instructions the initial value of the stack pointer is used as the jump destination. The current behavior of this instructions is to jump to the value of the stack pointer minus 4.

POPX fixes

The POPX instructions currently does not jump when the PC is used as the destination register. POPX also had the same ordering issues fixed previously with the POP instructions.

Check correct sign bit for RRAM.W and RLAM.W

Currently the RRAM.W and RLAM.W instructions check the 20th bit for the sign value. This is incorrect as they are word instructions and clear the upper bits.

SXT sign extend through 20 bits

For MSP430X, the MSP430 SXT instruction should sign extend through the full register (4.3.5). Currently it only sign extends up to a word.

Add address support to postRegIncrement

Currently the postRegIncrement table doesn't handle 20 bit sizes, but is used in single operand address sized instructions. This causes instructions which access address sized memory with the indirect auto-increment addressing mode (4.4) to increment by 1 instead of 4. It also causes jumps from using the PC as the destination register to be masked to a byte.

Fix ordering for PUSHX instructions

The correct ordering for a push instruction is to fetch the source value, decrement the SP, and write the value to the memory at that location. Figure 4-8 shows he result after a PUSH SP instruction. Currently the PUSHX instruction does not do this and instead writes the value of the new SP to the memory the SP addresses.

RRCX.A uses wrong value for address mask

In the RRCX.A instruction the register is shifted to the right one, and then the upper 19 bits should be masked for the main result. Currently the value 0x0xEFFFF is used as the mask instead of 0x7FFFF.

SXTX, RRAX, and RRCX overlaps for rpt

Currently the SXTX, RRAX, and RRCX instructions overlap as the 7th bit is not checked in the disassembly. The RRAX and RRCX instructions use the extended single-operand format shown in figure 4-31.

Address instructions use wrong value for pc with indexed mode

We found that when using the program counter as the base register for the indexed accessing mode for address instructions, the address of the start of the immediate field should be used. As the address instructions take up 2 bytes this is the instruction start + 2.

Add postStorePC to rpt RLAX instructions

The non-register repeat variant of the RLAX instructions should jump when changing the program counter, currently they do not.

rpt must use register addressing mode

The repeat instructions are only valid when using the register addressing mode. This is shown in 4.5.2. Currently other addressing modes are allow and where possible Ghidra will prefer the repeat format over the regular instruction.

PUSHM SP and PC fixes

We found that the PUSHM instructions push the address of the next instruction when pushing the program counter, currently it is pushing the start address of the current instructions. The PUSHM instructions where also found to push the value of the stack pointer before decrementing instead of the value after as is currently done.

Use postStorePC for address instructions instead of duplicating

By using the postStorePC constructor, special cases of the address instructions with the PC as the destination can be removed. This commit does not fix any behavioural issues.