M68000 decompiler: Read of volatile region ignored if followed by one or more writes

[philpem]

Describe the bug
When a read of a volatile memory region is followed by a sequence of zero-writes, the read is silently dropped from the decompiled output.

To Reproduce
Steps to reproduce the behavior:

1. Load the code segment included below as M68020 Big Endian code.
2. Define a volatile read-write region in the memory map -- starting address 0x210000, size 32 bytes. Define the whole region as a 32-byte array labelled "NVRAMCON".
3. Disassemble the code.
4. Observe that there is a read from the volatile region immediately prior to the writes which is not included in the disassembly (but should be).

Expected behavior
Decompiler shows a read from volatile space (with the result discarded), followed by a sequence of volatile writes of value zero.

Environment (please complete the following information):

• OS: Ubuntu 18.04
• Java Version: 11
• Ghidra Version: 9.0.2

Additional context

Disassembly extract:

        0000040a 10 28 00 1e     move.b     (offset NVRAMCON[30],A0),D0b                     = null
        0000040e 42 00           clr.b      D0b
        00000410 11 40 00 14     move.b     D0b,(offset NVRAMCON[20],A0)                     = null
        00000414 11 40 00 0a     move.b     D0b,(offset NVRAMCON[10],A0)                     = null
        00000418 11 40 00 14     move.b     D0b,(offset NVRAMCON[20],A0)                     = null
        0000041c 11 40 00 0a     move.b     D0b,(offset NVRAMCON[10],A0)                     = null
        00000420 11 40 00 0a     move.b     D0b,(offset NVRAMCON[10],A0)                     = null

Decompiler output:

  write_volatile_1(NVRAMCON[20],0);
  write_volatile_1(NVRAMCON[10],0);
  write_volatile_1(NVRAMCON[20],0);
  write_volatile_1(NVRAMCON[10],0);
  write_volatile_1(NVRAMCON[10],0);

Note that the byte read from NVRAMCON[30] into D0 is ignored, but the writes are accepted.

[emteere]

Does the read have any side-effect, like priming the NVRAM for write? Looks like the code is loading D0b with the volatile read, and then setting it to zero for the rest of the writes, unless I'm missing something.

I suppose reading a value from volatile memory could have side-effects and support might need to be added, if there isn't a way to do this currently.

[philpem]

@emteere - Yep, the read has side effects. This is part of a block of code which initialises a Dallas DS1234 NVRAM controller.

The NVRAM controller "listens" to the RAM address bus for reads and writes -- a read resets a shift register inside the chip, then the following sixteen RAM writes carry an instruction code for the chip (in this case, it's "enable memory writes and NVRAM battery backup"). The instruction is executed (if valid) when the 16th bit is clocked in.

I'd say that any read or write from volatile space should be shown, even if the result is discarded.

This is a fairly common pattern for memory mapped I/O -- parts like ASICs and I/O ports doing things when a memory address is read or written. The 68682 UART has a few like this (e.g. read from an MMIO register to start the timer counting).

[NeedsMoreFlux]

I've noticed the same problem of volatile reads being dropped in the decompiled output for ARM programs, so it seems like this issue is not specific to the M68000 decompiler.

Memory-mapped FIFO hardware registers are another good example of reads generating side effects, where each read removes an item from the FIFO regardless of whether the value is used later by the program. A specific scenario that I've observed in the ARM decompiler is disassembly along the lines of:

mov r1, #FIFO_ADDR
ldr r0, [r1]
ldr r0, [r1]

being decompiled into a single line like:

dVar1 = read_volatile_4(FIFO_ADDR)

This is quite misleading, as the decompilation gives the impression that dVar1 is being assigned to the value of the first item in the FIFO, whereas the actual effect of the code is to discard the first item and set dVar1 to the value of the second item in the FIFO.

I support @philpem's suggestion of preserving all accesses to volatile memory, even when the results of such accesses are discarded.

[lab313ru]

Is that related: #296 ?

[caheckman]

This should be fixed now in master and in upcoming 9.2. The problem was that the LOAD operation was getting removed as dead code before the decompiler discovered the load address was constant and in volatile memory.