incorrect sequence of integer and FP based calculations

[wothke]

Describe the bug
Ghirda's decompiler output incorrectly changes the sequence of integer and floating point based calculations leading to incorrect garbage C code.

In the below example the generated C code reads:

float10 FUN_0040ad30(double param_1,double param_2)

{
  double dVar1;
  double dVar2;
  
  if (param_2 < DOUBLE_00430038) {
    param_2 = -0.99;
  }
  dVar2 = _fmod_invArgOrder_0041478a(2PI_DOUBLE_00430020,param_1);
  if (dVar2 < DOUBLE_00430018) {
    dVar2 = dVar2 + 2PI_DOUBLE_00430020;
  }

  param_2._0_4_ = 1;	<=== *** ERROR: THIS ASSIGNMENT MUST NOT BE MADE BEFORE THE BELOW CALC! ***
  dVar1 = (param_2 + DOUBLE_0042ff20) * DOUBLE_00430030 * PI_DOUBLE_00430028;
  if ((ushort)((ushort)(dVar2 < dVar1) << 8 | (ushort)(dVar2 == dVar1) << 0xe) != 0) {
    param_2._0_4_ = -1;
  }
  return (float10)param_2._0_4_;
}

Looking at the original ASM code below, the function obviously loads the "param_2" input into the FPU to later perform a sequence of FP additions/multiplications with it. 4 bytes of the then no longer needed stack area originally occupied by "param_2" are then reused as a temporary variable to store the function's 1/-1 result which is finally loaded into ST0.

The decompiled code corrupts the "param_2" while it is still meant to contain the originally input double parameter. This is incorrect and completly invalidates the respective calculation.

                             float10 __stdcall FUN_0040ad30(double param_1, double 
             float10           ST0:10                <RETURN>
             double            Stack[0x4]:8          param_1                                 XREF[1]:     0040ad51(R)  
             double            Stack[0xc]:8          param_2                                 XREF[6,1]:   0040ad30(R), 
                             FUN_0040ad30            
        0040ad30 dd 44 24 0c     FLD                           qword ptr [ESP + param_2]
        0040ad34 dc 1d 38        FCOMP                         qword ptr DOUBLE_00430038]                  <- -0.99
                 00 43 00
        0040ad3a df e0           FNSTSW                        AX
        0040ad3c f6 c4 01        TEST                          AH,0x1
        0040ad3f 74 10           JZ                            LAB_0040ad51
        0040ad41 c7 44 24        MOV                           dword ptr [ESP + param_2],0x7ae147ae
                 0c ae 47 
                 e1 7a
        0040ad49 c7 44 24        MOV                           dword ptr [ESP + param_2+0x4],0xbfefae14
                 10 14 ae 
                 ef bf
                             LAB_0040ad51                                    XREF[1]:     0040ad3f(j)  
        0040ad51 dd 44 24 04     FLD                           qword ptr [ESP + param_1]
        0040ad55 dd 05 20        FLD                           qword ptr 2PI_DOUBLE_00430020]              <- 6.2831853
                 00 43 00
        0040ad5b e8 2a 9a        CALL                          _fmod_invArgOrder_0041478a   
                 00 00
        0040ad60 dc 15 18        FCOM                          qword ptr [DOUBLE_00430018]                 <- 0.0
                 00 43 00
        0040ad66 df e0           FNSTSW                        AX
        0040ad68 f6 c4 01        TEST                          AH,0x1
        0040ad6b 74 06           JZ                            LAB_0040ad73
        0040ad6d dc 05 20        FADD                          qword ptr 2PI_DOUBLE_00430020]              <- 6.2831853
                 00 43 00
                             LAB_0040ad73
							 
							 **************************************************************
							  at this point the original input double "param_2" is loaded
							  into the FPU to be used with the respective FP add/mul ops
							 **************************************************************
							 							 
        0040ad73 dd 44 24 0c     FLD                           qword ptr [ESP + param_2]
        0040ad77 dc 05 20        FADD                          qword ptr DOUBLE_0042ff20]                  <- 1.0
                 ff 42 00

							 **************************************************************
							  from that point the original "param_2" stack space is reused 
							  to hold a temporary integer variable that is finally used 
							  for the function's result: it is here initialized to 1							  
							 **************************************************************

		0040ad7d c7 44 24        MOV                           dword ptr [ESP + param_2],0x1
                 0c 01 00 
                 00 00
        0040ad85 dc 0d 30        FMUL                          qword ptr DOUBLE_00430030]                  <- 0.5
                 00 43 00
        0040ad8b dc 0d 28        FMUL                          qword ptr PI_DOUBLE_00430028]               <- 3.14159265
                 00 43 00
        0040ad91 d9 c9           FXCH
        0040ad93 de d9           FCOMPP
        0040ad95 df e0           FNSTSW                        AX
        0040ad97 f6 c4 41        TEST                          AH,0x41
        0040ad9a 74 08           JZ                            LAB_0040ada4
        0040ad9c c7 44 24        MOV                           dword ptr [ESP + param_2],0xffffffff
                 0c ff ff 
                 ff ff
                             LAB_0040ada4                                    XREF[1]:     0040ad9a(j)  
        0040ada4 db 44 24 0c     FILD                          dword ptr [ESP + param_2]
        0040ada8 c3              RET

Expected behavior
The correct ordering of operations must be preserved when a function uses the same input variable/local storage in FPU and CPU
based operations.

PS: I am surprized by the number of significant decompilation flaws that I've run into in the short time (less than 2 weeks) that I've been playing with this tool (see my other bug reports). How good is Ghidra's coverage of the x86 stuff in general?

Environment (please complete the following information):

• OS: Win10
• Ghidra Version: 10.1.1