Z80 instructions brokenly reordered in decompilation

[cm68]

the order of EI and DI instructions is important, and moving them exposes races that do not exist in the original code

the code:

                             **************************************************************
                             *                          FUNCTION                          *
                             **************************************************************
                             void _enable(void)
             void              <VOID>         <RETURN>
                             _enable                                    
                0                   ram:6b27 af              XOR        A
                0                   ram:6b28 32 36 6b        LD         (dicount),A
                0                   ram:6b2b ed 46           IM
                0                   ram:6b2d fb              EI
                0                   ram:6b2e c9              RET

decompiles to:

void _enable(void)
{
  setInterruptMode(0);
  enableMaskableInterrupts();
  dicount = 0;
  return;
}

the original code uses dicount as a lock, and moving the clearing of the allows an interrupt handler to see a non-zero value.

ghidra version 9.2-DEV

[agatti]

Maybe it is the same as #1208? Marking dicount as volatile might do the trick I guess...

[cm68]

how would I mark a variable as volatile? as far as I know, this is only a memory region thing.

On Tue, Sep 15, 2020 at 12:08 PM Alessandro Gatti notifications@github.com wrote:

Maybe it is the same as #1208 https://github.com/NationalSecurityAgency/ghidra/issues/1208? Marking dicount as volatile might do the trick I guess...

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/NationalSecurityAgency/ghidra/issues/2257#issuecomment-692919426, or unsubscribe https://github.com/notifications/unsubscribe-auth/AJJUTKZJUDSK23BCR4VOQGDSF63TTANCNFSM4RCWCOEA .

-- I refine, they debug, he/she patches, they kludge.

[agatti]

No idea, really. Maybe you can create a region that's as big as that variable... Still, this may or may be not a bug in the decompiler, I guess the Ghidra folks could take a look at that.

[emteere]

You can create a db or other data type at dicount, and change the data settings for mutability to volatile. That does indeed re-order the "dicount=0" to before the Interrupt enable pcode userops.

You really shouldn't need to do this, and the order preserved, but it does work. I'll check into why.

[neuromancer]

Is there any updates on this issue? I was wondering if it will possible to add generic IO memory marked as volatile for different devices (e.g. ZX Spectrum, Amstrad CPC, etc), in order to workaround this issue and also to offer better decompilation