Condition being disregarded leading to wrong output

[Wall-AF]

Describe the bug
A condition based upon the contents of a global byte array (initially within an undefined memory block, but recently joined with its predecessor to make one contiguous area, as the array spanned both areas when loaded into Ghidra) is being treated as having a value of 0 when interpreted by the decompiler, even if the mutability is changed from normal to volatile - normal should work. This is leading to incorrect C code. I.e. instead of

      if (false) {
         aVkStates_2_1001c128[vk] = aVkStates_2_1001c128[vk] | KEYSTATE_Held;
      }

or nothing (if option Eliminate unreachable code is on), we should see

      if (g_aVkStates_1_100197f0[vk] < 0) {
         aVkStates_2_1001c128[vk] = aVkStates_2_1001c128[vk] | KEYSTATE_Held;
      }

(edited the condition to be <)

the assembly that is corresponding to if (false) { is

                             LAB_10007c99                                    XREF[2]:     10007c5f(j), 10007c7c(j)  
    10007c99 8b 45 fc     020           MOV        EAX,dword ptr [EBP + vk]
    10007c9c 33 c9        020           XOR        ECX,ECX
    10007c9e 8a 88 f0     020           MOV        CL,byte ptr [EAX + aVkStates_1_100197f0]
             97 01 10
    10007ca4 85 c9        020           TEST       ECX,ECX
    10007ca6 0f 8d 17     020           JGE        LAB_10007cc3
             00 00 00

To Reproduce
Steps to reproduce the behavior:

1. Load in the enclosed function (from the Decompile:Panels Debug Function Decompilation menu)
2. Search down the decompiled code to find the output above
3. Compare the assembly with the C code and you'll see the condition is missing

Expected behavior
The enclosing guard condition should not be deemed superfluous and should appear in the reproduced C code.

Screenshots
N/A

Attachments
dd25hook_FUN_10007bb7.zip

Environment (please complete the following information):

• OS: Windows 11
• Java Version: 17.0,3.1
• Ghidra Version: 11.1.DEV
• Ghidra Origin: locally built

Additional context
This is a 32-bit DLL compiled with MSVC 4.1/4.2 compiler.

[Wall-AF]

This could be a red herring, but I'm unsure. Because of what the TEST/JGE is guarding, I can't believe it to be unreachable. If it were, surely the compiler would have eliminated it?

[caheckman]

The decompiler's analysis looks valid to me. The TEST / JGE branches if ECX is greater than or equal to zero, which is always true because the XOR ECX,ECX clears the sign bit and the MOV CL,byte ptr [...] doesn't alter it.

[Wall-AF]

@caheckman I came to the same conclusion, but .... I wonder if there's some undocumented thing going on that, back in the day, treated that prticular MOV opcode as a signed one, or that the particular segment definition ensured the TEST just used CL or something else wierd? Because of the assembly and what it's doing, I can't believe the original developers would have missed that, and surely the compiler would've removed that code entirely during optimisation if it was unreachable??? What do you think?

[ryanmkurtz]

Closing because there doesn't seem to be a problem with Ghidra. The conversation can continue in the "closed" state.