ghidra: Improper struct parameter/return storage, unset parameter storage, etc etc

Describe the bug The storage for structs returned or passed by value is incorrect. A struct should always be returned as __return_storage_ptr__. After a certain amount of parameters their storage is not set. Also seen is the Windows x86 PE Exception Handling analyzer not functioning?

To Reproduce Steps to reproduce the behavior:

Load the attached program/pdb/source. It’s a disaster all around and much easier to just provide it.

Expected behavior All structs should be returned as __return_storage_ptr__ even if they will fit in a register. Structs passed by value should be passed as pointer to the stack location and not all it’s components CONCATed together. The TEB * and exception handling function should either be marked up or hidden instead of in_FS_OFFSET and an uncreated function. Comments like the gcc exception handler analyzer produces would be appreciated.

Attachments

4051.zip issue number is one off.

Environment (please complete the following information):

Java Version: 11
Ghidra Version: 10.1.2 and 1996cdacd1d1bd7442c97cf694b6f27efb2b4fdb

Additional context This monstrosity was inspired by real programs out in the wild I’ve have had the pleasure of reversing.

Disclaimer: I am not responsible for any further hair loss that may come as a result of looking into this issue or the source code which produced it.

About this issue

Original URL
State: open
Created 2 years ago
Comments: 15 (12 by maintainers)

Most upvoted comments

Disclaimer: I am not responsible for any further hair loss that may come as a result of looking into this issue or the source code which produced it.

Strong hair required for deep source code spelunking.

dragonmacher on Mar 4, 2022

I did some experimentation, and this monstrosity seems to work for x86-64:
        <pentry minsize="1" maxsize="8">
          <register name="RDI"/>
        </pentry>
        <pentry minsize="1" maxsize="8">
          <register name="RSI"/>
        </pentry>
        <pentry minsize="9" maxsize="16">
          <addr space="join" piece1="RSI" piece2="RDI"/>
        </pentry>
        <pentry minsize="1" maxsize="8">
          <register name="RDX"/>
        </pentry>
        <pentry minsize="9" maxsize="16">
          <addr space="join" piece1="RDX" piece2="RSI"/>
        </pentry>
        <pentry minsize="1" maxsize="8">
          <register name="RCX"/>
        </pentry>
        <pentry minsize="9" maxsize="16">
          <addr space="join" piece1="RCX" piece2="RDX"/>
        </pentry>
        <pentry minsize="1" maxsize="8">
          <register name="R8"/>
        </pentry>
        <pentry minsize="9" maxsize="16">
          <addr space="join" piece1="R8" piece2="RCX"/>
        </pentry>
        <pentry minsize="1" maxsize="8">
          <register name="R9"/>
        </pentry>
        <pentry minsize="9" maxsize="16">
          <addr space="join" piece1="R9" piece2="R8"/>
        </pentry>
        <pentry minsize="1" maxsize="500" align="8">
          <addr offset="8" space="stack"/>
        </pentry>
One minor issue is that it doesn’t handle the (Rust-specific) case of splitting a struct between the last register and the first QWORD of the stack. (In theory, joining with the piece stack:8:8 should work based on my read of AddressXML, but I did not get it to work.) Admittedly, this is an uncommon occurrence, since it requires a two-qword-argument to be exactly the 6th and 7th qword. Otherwise, it works for a few samples I threw at it, and it is at least better than the alternative of having to fix the signatures on every struct-passing function.

@astrelsky The example cspec XML above uses non-contiguous registers to construct larger arguments. Maybe an approach like this would work for you?

I tried to do something similar yesterday for use with go and unfortunately the xml specification file only allows up to join4 even though I think the decompiler has no such limitation. By that point it was about as gross as you would expect. There has to be a better way.

Even though it did work I ultimately still faced part of the same problem as the original issue which was the unnecessary CONCAT to join the structure all over the place in the function call.

astrelsky on May 2, 2022

I did some experimentation, and this monstrosity seems to work for x86-64:

        <pentry minsize="1" maxsize="8">
          <register name="RDI"/>
        </pentry>
        <pentry minsize="1" maxsize="8">
          <register name="RSI"/>
        </pentry>
        <pentry minsize="9" maxsize="16">
          <addr space="join" piece1="RSI" piece2="RDI"/>
        </pentry>
        <pentry minsize="1" maxsize="8">
          <register name="RDX"/>
        </pentry>
        <pentry minsize="9" maxsize="16">
          <addr space="join" piece1="RDX" piece2="RSI"/>
        </pentry>
        <pentry minsize="1" maxsize="8">
          <register name="RCX"/>
        </pentry>
        <pentry minsize="9" maxsize="16">
          <addr space="join" piece1="RCX" piece2="RDX"/>
        </pentry>
        <pentry minsize="1" maxsize="8">
          <register name="R8"/>
        </pentry>
        <pentry minsize="9" maxsize="16">
          <addr space="join" piece1="R8" piece2="RCX"/>
        </pentry>
        <pentry minsize="1" maxsize="8">
          <register name="R9"/>
        </pentry>
        <pentry minsize="9" maxsize="16">
          <addr space="join" piece1="R9" piece2="R8"/>
        </pentry>
        <pentry minsize="1" maxsize="500" align="8">
          <addr offset="8" space="stack"/>
        </pentry>

One minor issue is that it doesn’t handle the (Rust-specific) case of splitting a struct between the last register and the first QWORD of the stack. (In theory, joining with the piece stack:8:8 should work based on my read of AddressXML, but I did not get it to work.) Admittedly, this is an uncommon occurrence, since it requires a two-qword-argument to be exactly the 6th and 7th qword. Otherwise, it works for a few samples I threw at it, and it is at least better than the alternative of having to fix the signatures on every struct-passing function.

@astrelsky The example cspec XML above uses non-contiguous registers to construct larger arguments. Maybe an approach like this would work for you?

nneonneo on Apr 25, 2022