[PIC16F] Wrong decompilation of conditional branches (btfsc, btfss)

[beketata]

Describe the bug

Below is a part of the listing obtained from the attached HEX file.

Actually the PIC model is PIC16F18855. That's why some registers are shown here as DAT_DATA_xxxx instead of their real names:
DAT_DATA_0716 instead of PIE0,
DAT_DATA_070c instead of PIR0 and so on.
But it doesn't matter for the explanation of the current issue.

                             **************************************************************
                             *                          FUNCTION                          *
                             **************************************************************
                             undefined Interrupt()
             undefined         W:1            <RETURN>
                             Interrupt                                       XREF[1]:     Entry Point(*)  `
       CODE:0004 7e 14           BSF        DAT_DATA_007e,#0x0                               = ??

       CODE:0005 80 31           MOVLP      #0x0
       CODE:0006 2e 00           MOVLB      #0xe
       CODE:0007 96 1a           BTFSC      offset DAT_DATA_0716,#0x5                        = ??
       CODE:0008 8c 1e           BTFSS      offset DAT_DATA_070c,#0x5                        = ??
       CODE:0009 0e 28           GOTO       LAB_CODE_000e
       CODE:000a 91 31           MOVLP      #0x11
       CODE:000b 61 21           CALL       offset FUN_CODE_1161                             undefined FUN_CODE_1161()
       CODE:000c 80 31           MOVLP      #0x0
       CODE:000d 1c 28           GOTO       LAB_CODE_001c
             assume BSR = 0xe
                             LAB_CODE_000e                                   XREF[1]:     Interrupt:0009(j)  
       CODE:000e 16 1a           BTFSC      offset DAT_DATA_0716,#0x4                        = ??
...

Expected behavior

As can be seen from the disassembly listing, FUN_CODE_1161 in the line CODE:000b should only be called if both checking on the line CODE:0007 and CODE:0008 are met like this:

  if( (DAT_DATA_0716 & 0x20) != 0 && (DAT_DATA_070c & 0x20) != 0 ) {
    FUN_CODE_1161();
  }

or much better (so long awaited ):
Decompiler not detecting bitfields in structures #2462
Bitfields don't seem to decompile very well #1059

  if( PIE0bits.TMR0IE && PIR0bits.TMR0IF ) {
    FUN_CODE_1161();
  }

And then follows the corresponding decompilation view:

void Interrupt(void)
{
  byte bVar1;
  bool bVar2;
  
  DAT_DATA_007e = DAT_DATA_007e | 1;

  bVar2 = (DAT_DATA_0716 & 0x20) == 0;  // There is a checking of TMR0IE bit of PIE0 register
  if (!bVar2) {
    bVar2 = (DAT_DATA_070c & 0x20) != 0;  // There is a checking of TMR0IF bit of PIR0 register
  }
  if (bVar2) {
    FUN_CODE_1161();
  }
  else {
...  

In this variant calling of FUN_CODE_1161 will occur when (DAT_DATA_0716 & 0x20) == 0 regardless of the condition of the second check (DAT_DATA_070c & 0x20) != 0

To Reproduce

1. Unpack attached PIC16F18855.zip
2. Create new project and select language PIC16F
3. Open unpacked PIC16F18855.hex
4. Go to the CODE:0004 and see the issue

Attachments

PIC16F18855.zip

Environment

• OS: Windows 10 Pro 22H2
• Java Version: 23.0.1
• Ghidra Version: 11.2.1
• Ghidra Origin: official GitHub distro

[beketata]

My investigation of this issue led me to the next topic Skip instruction implementation #4241
Therefore it is necessary to modify all skip instructions in the corresponding processor file with inst_next2 instead of inst_next.
For example:

:BTFSS srcREG, bit		is op4=0x7 & bit & srcREG
{
	#  --01 11bb bfff ffff
	#  0001 1110 0000 0000	->	BTFSS INDF, #0x4
	#  0001 1110 0010 0000	->	BTFSS 0x20, #0x4
	local bitmask = 1 << bit;
	local tmp = srcREG & bitmask;
	if( tmp != 0 ) goto inst_next2;
}

After this modification the decompilation view looks more or less acceptable, but most importantly - without violating the logic of execution.