NationalSecurityAgency / ghidra

Ghidra is a software reverse engineering (SRE) framework
https://www.nsa.gov/ghidra
Apache License 2.0
50.91k stars 5.8k forks source link

MSP430 problem #5901

Open gabizro opened 11 months ago

gabizro commented 11 months ago

Ghidra unable to disassemble code Loaded file selected MSP430 16bit LE IVT

RST

Same file on IDA

RST_ida

Ghidra hangs at 0x310C with error Unable to resolve constructor.

mumbel commented 11 months ago

maybe a bad opinion match for elf headers .. reload the elf, but when it chooses MSP430, change to MSP430X and see if that works

gabizro commented 11 months ago

Is a raw binary file MCU is MSP430FG4617 so it is 16 bit Selecting MSP430X get some disassemble but end on error

Cannot invoke "ghidra.program.util.ContextEvaluator.allowAccess(ghidra.program.util.VarnodeContext, ghidra.program.model.address.Address)" because "evaluator" is null

Maybe because CALLA it's a 20-bit instruction with only a 16-bit index some old discussion https://e2e.ti.com/support/microcontrollers/msp-low-power-microcontrollers-group/msp430/f/msp-low-power-microcontroller-forum/180745/crash-with-indexed-calla-instruction-from-flash Same case for MOVA Both are available on Ghidra only on MSP430X, but in 32bit mode all addresses are invalid

GhidorahRex commented 11 months ago

It should definitely be using MSP430X. I think the issue might be with the way the Abs20add sub-constructor is calculated. It writes an immediate into a context register, which it really shouldn't be doing for a sub-constructor.

I don't have a sample binary to test. If you have one, I'd be willing to take a look at it. Otherwise, I would consider changing TI430X.sinc line 39 to:

Abs20add: val is imm_0_4; imm_0_16 [ val=(imm_0_4 << 16) | imm_0_16;] {export *:4 val;}

and line 1127 to: :CALLA "#"^Abs20add is ctx_haveext=0 & (op16_8_8=0x13 & op16_4_4=0xB) ... & Abs20add {

also line 1023 to fix the same issue: :BRA "#"^Abs20add is ctx_haveext=0 & (op16_12_4=0 & insid=0x8 & dest_0_4=0x0) ... & Abs20add {

There's a couple similar issues I've seen where some addressing modes are using context to store part of an immediate. If this fixes this issue, I can work on addressing the rest as well.

gabizro commented 11 months ago

Here is m430fg4617.zip

GhidorahRex commented 11 months ago

Yep, this seems to resolve the issue. EXCEPT... we also need to change line 139 of TI430Common.sinc to: imm_0_4 = (0, 3)

gabizro commented 11 months ago

Tested with modified TI430X.sinc and TI430Common.sinc and selected MSP430X Still some errors. new_err

Also with MSP430X since is 32bit all values that are pointers cannot be defined as pointers new_err2 0x335B

Selecting MSP430 these looks OK 16bit

On MCU user guide https://www.ti.com/lit/pdf/slau056 on page 152 they say that can use only MSP430 instruction with exception for CALLA and RETA So they can use CALLA and MOVA in 16 bit

emteere commented 11 months ago

The null pointer stack trace has been fixed in an upcoming commit GP-3970. That shouldn't ever happen no matter the sleigh code. It may point to an issue with the way the pcode accessing the stack is done as the code wasn't exercised by other processers or I think it would have been seen before.

emteere commented 11 months ago

From your above pointer issues, the pointer size is assumed to be the size of your memory, and if pointers are stored as 16-bit values, the memory size would need to be 16-bit as well. It could be specified in the data organization part of the .cspec if all the uses of those pointers are 16-bit accesses which would mean a new 16-bit variant of the MSP430X. I don't see a data organization section in either MSP430 .cspec files.

<compiler_spec>
  <data_organization>
     ...
     <default_pointer_alignment value="2" />
     <pointer_size value="2" />

For those particular symbols, the vector interrupts. Their size can be set in the .pspec file:

        <symbol name="RESET"   address="RAM:FFFE" entry="true" size="2" type="code_ptr"/>

If the vectors are always 2 bytes in the 430 and 430X this should be done. The 430X might need a .pspec file if not. I'm hopeful the code will see the size and create a pointer of the correct size regardless of the memory size.

If the space is really 32-bit and some pointers are stored as 16-bit values and some as 32-bit values, then you can use a Pointer16 anywhere the pointers are really 16-bit.

Wall-AF commented 11 months ago

If the space is really 32-bit and some pointers are stored as 16-bit values and some as 32-bit values, then you can use a Pointer16 anywhere the pointers are really 16-bit.

@emteere does this comment apply to x86-16.cspec as pointers are set as 16-bit, but, in my case. are really 32-bit with the high 16-bits being referenced from a segment register? If so, what would be the correct changes to make and would any be required in the corresponding pspec file?