Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pointer size is affecting array indexing inhibiting proper C output #6805

Open
Wall-AF opened this issue Aug 12, 2024 · 5 comments
Open

Pointer size is affecting array indexing inhibiting proper C output #6805

Wall-AF opened this issue Aug 12, 2024 · 5 comments
Assignees
Labels
Feature: Decompiler Status: Triage Information is being gathered

Comments

@Wall-AF
Copy link

Wall-AF commented Aug 12, 2024

Describe the bug
Using sized pointers, say ScriptHandler *32 in a 16-bit app to establish a far pointer, simple arithmetic used to index the array of those objects fails to use the base object size and consequently doesn't recognise the calculation as the index. Something like

   1028:0cb9 8b c2        0c8           MOV        AX,DX
   1028:0cbb 69 c0 02 04  0c8           IMUL       AX,AX,0x402
   1028:0cbf 26 29 47 16  0c8           SUB        word ptr ES:[BX + 0x16],AX
   1028:0cc3 8b 46 06     0c8           MOV        AX,word ptr SS:[BP + this]

turns into
*(int *)&pThis->lpScrptHldr_0x16 = *(int *)&pThis->lpScrptHldr_0x16 + iVar4 * -0x402;
instead of (if the pointer is sized 16 - the native pointer size)
pThis->lpScrptHldr_0x16 = pThis->lpScrptHldr_0x16 + -iVar4;
admittedly not perfect, but better!

Expected behavior
Array index calculations should only use the pointer size when pointer-to-pointer... (and the index level is NOT at the lowest dereferencing point) is being sought otherwise use its base type size.

Screenshots
See bug description.

Attachments
None.

Environment (please complete the following information):

  • OS: Windows 11
  • Java Version: Temurin-21.0.3+9
  • Ghidra Version: 11.2-DEV
  • Ghidra Origin: locally built (based upon commit 34ba255)

Additional context
None.

@mumbel
Copy link
Contributor

mumbel commented Aug 12, 2024

I recently upgraded a lot of images and I think it's also generally broken right now for a lot of this kind of access. I've noticed a lot of arrays cast as int and then indeed, losing type information (ppc,mips,sparc)

Hopefully fixed by either #6718 or #6722

@Wall-AF
Copy link
Author

Wall-AF commented Aug 12, 2024

I recently upgraded a lot of images and I think it's also generally broken right now for a lot of this kind of access. I've noticed a lot of arrays cast as int and then indeed, losing type information (ppc,mips,sparc)

Hopefully fixed by either #6718 or #6722

I haven't updated to include those yet!

@mumbel
Copy link
Contributor

mumbel commented Aug 12, 2024

They're not merged, I haven't tried either yet in a local build to see if it fixes the issues I've seen, but looks promising

@Wall-AF
Copy link
Author

Wall-AF commented Oct 5, 2024

I recently upgraded a lot of images and I think it's also generally broken right now for a lot of this kind of access. I've noticed a lot of arrays cast as int and then indeed, losing type information (ppc,mips,sparc)
Hopefully fixed by either #6718 or #6722

I haven't updated to include those yet!

No dice! Tried both fixes together and independently to no effect.

I believe the 2 sizes of pointers is what really needs fixing for my issues. E.g. if a structure contains a pointer and that pointer stores the segment and offset that turns it into a 4-byte pointer which then interferes with Ghidra's internals as they cannot cope with 2 sizes! Is there any hope for that to be investigated?

@Wall-AF
Copy link
Author

Wall-AF commented Oct 5, 2024

Maybe the 2 screenshots below illustrate the situation. In the first, pFld0x2->field4_0xa is defined as a pointer of size 32-bits and the second uses the default size (as per cspec (16-bit)).
image
image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature: Decompiler Status: Triage Information is being gathered
Projects
None yet
Development

No branches or pull requests

4 participants