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

SubfloatFlow causes incorrect decompilation with large constants #4586

Open
RootCubed opened this issue Sep 9, 2022 · 1 comment
Open

SubfloatFlow causes incorrect decompilation with large constants #4586

RootCubed opened this issue Sep 9, 2022 · 1 comment
Assignees
Labels
Feature: Decompiler Status: Internal This is being tracked internally by the Ghidra team

Comments

@RootCubed
Copy link
Contributor

RootCubed commented Sep 9, 2022

Describe the bug
The SubfloatFlow rule traverses the operation tree of float2float PcodeOps backwards and converts floating-point constants to the same precision as the float2float output, if I understood correctly. This can cause issues when, for instance, two large floating point numbers are subtracted from each other, resulting in a (relatively) small floating point number which may get rounded down to 0 if the two large numbers are rounded to a smaller floating-point precision format.

Note that I am not asking for this simplification rule to be entirely removed, as it does a good job at keeping the output clean while only slightly reducing the accuracy of the output. But it would be better if this rule were not applied to operations which have only constants as input (since those will be eventually simplified by RuleCollapseConstants, and accuracy can be vital there to having the correct result, see demonstration program)

This issue was first noticed in an instance where the PowerPC int-to-float conversion sequence (https://www.warthman.com/images/IBM_PPC_Compiler_Writer's_Guide-cwg.pdf#G9.303610) was used on a constant value. A simpler example to demonstrate the problem is provided below in the "Additional context" section.

To Reproduce
Steps to reproduce the behavior:

  1. Assemble the provided demonstration program.
  2. Import it into Ghidra and analyse with default settings.
  3. Observe that the decompiler outputs the following:
void demo_float2float_propagate(float *param_1)

{
  *param_1 = 0.0;
  return;
}

Expected behavior
The expected output of the decompiler is:

void demo_float2float_propagate(float *param_1)

{
  *param_1 = 16962.0;
  return;
}

Environment (please complete the following information):

  • OS: Windows 11
  • Java Version: 17.0
  • Ghidra Version: 10.2-DEV
  • Ghidra Origin: locally built from master

Additional context

_Z26demo_float2float_propagatePf:
    lis r4, fconst1@h
    ori r4, r4, fconst1@l
    lis r5, fconst2@h
    ori r5, r5, fconst2@l

    # load the two large constants and subtract them
    lfd f1, 0(r4)
    lfd f2, 0(r5)
    fsub f1, f1, f2

    # convert result to 32-bit floating point and store into 0(r3)
    stfs f1, 0(r3)
    blr

fconst1:
# 4503601774871106.0
.long 0x43300000, 0x80004242
fconst2:
# 4503601774854144.0
.long 0x43300000, 0x80000000

demo_compiled.zip

Edit: In order to conform with the floating point conventions of the PowerPC architecture, there should actually be an frsp f1, f1 before the stfs instruction, but since the decompiler output is the same with and without that line, this is irrelevant for this issue.

@ryanmkurtz ryanmkurtz added Feature: Decompiler Status: Triage Information is being gathered labels Sep 9, 2022
@caheckman caheckman added Status: Prioritize This is currently being prioritized Status: Internal This is being tracked internally by the Ghidra team and removed Status: Triage Information is being gathered Status: Prioritize This is currently being prioritized labels Sep 13, 2022
@RootCubed
Copy link
Contributor Author

@caheckman This ticket has been on "Internal" status for over a year now - has there been any progress towards a fix for this? Would be greatly appreciated, this bug keeps coming up occasionally "in the wild", messing up the decompiler output.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature: Decompiler Status: Internal This is being tracked internally by the Ghidra team
Projects
None yet
Development

No branches or pull requests

3 participants