MSP430 Stack variable analysis gets confused about return pointer

[tslater2006]

Describe the bug When marking a function as returning a char and accepting 2 char parameters (strcpy) the stack analysis seems to get confused and decompiler output becomes wildly incorrect.

To Reproduce Steps to reproduce the behavior:

1. Use the adjusted cspec for MSP430 (attached) which provides "gcc" and "__interrupt" conventions
2. Import attached Ghidra Zip export for Jakarta
3. Navigate to the "login" function and observe decompiler output looks like this (screenshot 1)
4. Go to FUN_46f4 function and edit the function
5. Set calling convention to gcc
6. Set return type to char *
7. Add 2 parameters that are char * (screenshot 2)
8. Navigate to login function and observe decompiler output has change (it is showing the call return addresses as uStack38 = return address. (Screenshot 3)

Expected behavior I would not expect this function definition to change the decompiled output of other functions to the point where we see return addresses being assigned to stack variables. See attached "SantaCruz" export where a similar approach was taken and the decompiler works as expected.

Screenshots Screenshot 1:

Screenshot 2:

Screenshot 3:

Attachments Adjusted CSPEC to account for GCC calling convention (and the interrupt convention used by microcorruption CTF) TI_MSP430_CSPEC.zip

Problematic Microcorruption binary (this is a ghidra zip put into a zip so github would allow the attachment): Jakarta.zip

Microcorruption binary that works with strcpy defined properly: SantaCruz.zip

Environment (please complete the following information):

• OS: Windows 10
• Java Version: 11.0
• Ghidra Version: 9.0.1

Additional context

This issue is reproducible using binaries that have been created from the microcorruption CTF (Jakarta is the problematic one, SantaCruz works as expected). Due to Microcorruption leveraging GCC it was necessary to add a new calling convention for gcc which leverages R15-R12 for parameters instead of R12-R15

[emteere]

Is this a standard calling convention for the MSP430, or a special one for CTF?

The most likely culprit is the definition in the .cspec where the stack variables actually start.

        <pentry minsize="1" maxsize="500" align="4">
          <addr offset="0" space="stack"/>
        </pentry>

if the stack parameters start at something other than zero from the current stack pointer you may need to change offset in the .cspec.

[tslater2006]

This is the calling convention used by GCC: http://www.ti.com/lit/an/slaa664/slaa664.pdf

Relevant portions:

In MSPGCC,registers are passed starting with R15 and descendingto R12.

MSPGCC places the return value in R15 (or R15 and consecutive lower registers if the value is larger than a word), while the EABI specifies that the return value is placed in R12.

From what I can tell stack variables start at a 0 offset from the stack pointer.

Of note, the cspec adjustment works on the SantaCruz binary attached to the issue, but fails on the Jakarta one, both from the CTF and using MSPGCC

[tslater2006]

I will try adjusting that stack offset, however I fear that will impact the decompilation of the other functions that aren't affected by whatever is going on.

[emteere]

Your .cspec changes appear to be correct. It appears there is an aliasing issue in the decompiler where the return value is considered to have an effect on the second call to the function in question.

However, you can add the following to the .cspec to cause the decompiler to carve out the local space that doesn't include the return value. The issue will require a little more diagnosis to see if there is more that needs to be done.

    <returnaddress>
           <varnode space="stack" offset="0" size="2"/>
     </returnaddress>
     <default_proto>
           .....
     </unaffected>
     <localrange>
          <range space="stack" first="0x0" last="0xfc"/>
      </localrange>
    </prototype>

(Although, the decompiler isn't too offbase in it's thinking as this is "potentially" an artifact of the CTF)

[emteere]

Also, thanks for the work on the additional calling convention for the MSP430!

[tslater2006]

Glad the cspec changes were correct, and you're welcome. Thanks for looking further into this issue.