Call to stdcall functions through virtual tables causes issues with the stack analysis

[Kerilk]

Describe the bug On 32bit X86 when calling __stdcall functions through virtual tables (Microsoft interfaces for instance) the stack analysis is breaking down.

To Reproduce Steps to reproduce the behavior:

1. compile the test_stdcall.c file (contained in the provided zip archive using gcc: gcc -m32 -fno-pic test_stdcall.c -o test_stdcall -O1
2. import into ghidra
3. create correct function types and correctly set global variables types.
4. the call_func procedure is incorrectly analyzed

Expected behavior As calling conventions have been correctly specified for the function pointers I would expect the stack analysis to be performed correctly.

Screenshots This is the observed behavior: And here is the original function code:

void call_func(int a, int b, int c) {
    (*(*pObj)->f1)(pObj, 0, a);
    (*(*pObj)->f1)(pObj, 1, b);
    (*(*pObj)->f1)(pObj, 2, c);
}

Attachments The attachment contains the complete minimal source file as well as the completed Ghidra zip file. test_stdcall.zip

Environment (please complete the following information):

• OS: Win10
• Java Version: open-jdk 11.0.2
• Ghidra Version: 9.0.2
• gcc Version: (Ubuntu 7.3.0-27ubuntu1~18.04) 7.3.0

Additional context I used Ubuntu for windows for the test case but I know of binaries compiled with visual studio that display this behavior.

[emteere]

The culprit is the gcc .cspec doesn't include the stdcall calling convention currrently. It needs to be added. The automatic detection might be problematic based on the case. It does indeed work if you add the calling convention to the x86gcc.cpsec, and then set the calling convention on the function func_t definition.

void call_func(int a,int b,int c)

{
  (*(*pObj)->f1)(pObj,0,a);
  (*(*pObj)->f1)(pObj,1,b);
  (*(*pObj)->f1)(pObj,2,c);
  return;
}

[Kerilk]

Thank you very much for looking into this. This indeed fixes the gcc binary analysis.

I had hoped this was a minimal way to reproduce this bug I have on visual studio binaries: But unfortunately the issue seems to come from somewhere else. Of course the expected result is:

void __cdecl setRenderStateBlendAlpha(D3DBLEND srcBlend,D3DBLEND dstBlend,D3DBLENDOP blendOp)

{
  (*(*pDevice)->SetRenderState)(pDevice,D3DRS_SRCBLENDALPHA,srcBlend);
  (*(*pDevice)->SetRenderState)(pDevice,D3DRS_DESTBLENDALPHA,dstBlend);
  (*(*pDevice)->SetRenderState)(pDevice,D3DRS_BLENDOPALPHA,blendOp);
  return;
}

The bug is not always triggered, but often enough to be very annoying. Any Idea of how I can reduce it to something more manageable, that I could post here?

[emteere]

You can post the decompiler debug .xml that can be produced from the pull-down arrow in the upper right of the decompiler. It does appear the correct calling convention isn't applied to the SetRenderState() entry. Does the function definition have stdcall convention applied?

[Kerilk]

Yes the __stdcall convention is set. I also checked in the debug XML file: stdcall_win_debug.zip and the flag appears in the function type definition.

Thanks again for looking into it.

[elizagamedev]

I was able to reproduce the erroneous behavior for a visual studio compiled binary, so I thought it might help for me to upload a debug sample too. If the debug XML file isn't sufficient, please let me know and I can provide more detailed context.

broken_vtable_vs.zip

EDIT: I might have posted this on the incorrect bug; it's more closely related to #2051. The behavior here is that the return value of GetProcAddress, while being explicitly correctly typed with a function with the correct calling convention, still confuses the stack analysis.

[arbruijn]

The problem seems to be that the call stack adjustment (ActionExtraPopSetup) is handled before resolving the indirect function type (ActionDeindirect). I came up with the following patch which fixes the problem for my small test case (FUN_0040100d should end with return param_2 + iVar1 + param_1).

fix_indirect_stdcall.patch

decompind.zip

[filip-hejsek]

This is an instance of a more general problem - analysis before resolving the indirect function type (ActionDeindirect) cannot use the information from the function type.

I have encountered a similar problem, but with fastcall parameters. Because the function signature is not considered, the decompiler doesn't know how many parameters the function takes so it doesn't know what values are passed to the function. As a result, values which are passed to indirect calls but not otherwise used are considered unused and writes to them are ignored.

For now, i have found a simple workaround: use Override Signature to save the function signature at the call site. This will cause the signature to be available in all analysis passes and the function will be analyzed correctly.

I think a real fix will be a bit harder. The above patch solves the problem for call stack adjustment by detecting unknown extra pop, however to solve it generally some different approach is needed.

[tonybounty]

This seems to be the same problem here, the push 0x0 is transformed in variable in decompiler view. As written in the previous message, I must override the signature on GetModuleHandleA() with the same prototype.

before override signature :

Override Signature