Input / output parameters should be 'extended' for it transfered through registers

[esaulenka]

Describe the bug

It is a some continiuation of conversation #5757
Some architectures (like ARM, Power, V850, Tricore, ...) transfers function inputs and return value through registers.

If these values are smaller than register size, decompiler makes some additional noise.

To Reproduce

For clarify, lets talk about Tricore.

Callee:

                             byte __stdcall FUN_800f8538(void)
                               assume p0 = 0x8004d4a4d0009600
             byte              d2:1           <RETURN>
                             FUN_800f8538                                    XREF[6]: ...
        800f8538 05 d2 1e 05     ld.bu      d2, BYTE_d000101e                                = ??
        800f853c 00 90           ret

decompiled as

byte FUN_800f8538(void)
{
  p0 = 0x8004d4a4d0009600;
  return BYTE_d000101e;
}

Caller:

                             bool __stdcall FUN_800f7b86(void)
                               assume p0 = 0x8004d4a4d0009600
             bool              d2:1           <RETURN>
                             FUN_800f7b86                                    XREF[1]:     FUN_800f7fa4:800f7fdc(c)  
        800f7b86 82 0f           mov        d15, #0x0
        800f7b88 6d 00 d8 04     call       FUN_800f8538                                     byte FUN_800f8538(void)
        800f7b8c df 22 08 80     jne        d2, #0x2, LAB_800f7b9c
        800f7b90 19 00 e8 08     ld.w       d0, [a0]-0x73d8=>UINT_d0002228                   = ??
        800f7b94 19 0f 94 e8     ld.w       d15, [a0]-0x746c=>UINT_d0002194                  = ??
        800f7b98 0b 0f 30 f1     lt.u       d15, d15, d0
                             LAB_800f7b9c                                    XREF[1]:     800f7b8c(j)  
        800f7b9c 02 f2           mov        d2, d15
        800f7b9e 00 90           ret

decompiled as

bool FUN_800f7b86(void)
{
  byte bVar1;
  undefined3 extraout_var;
  bool bVar2;
  
  p0 = 0x8004d4a4d0009600;
  bVar2 = false;
  bVar1 = FUN_800f8538();
  if (CONCAT31(extraout_var,bVar1) == 2) {  // this 'extraout' always zero!
    bVar2 = *(uint *)((int)p0 + -0x746c) < *(uint *)((int)p0 + -0x73d8);     // and this one an another bug
  }
  return bVar2;
}

Similar behavior can be found in the input parameters:

                             bool __stdcall FUN_800f7bc4(byte param_1)
                               assume p0 = 0x8004d4a4d0009600
             bool              d2:1           <RETURN>
             byte              d4:1           param_1
                             FUN_800f7bc4                                    XREF[1]:     FUN_800f7bce:800f7bf4(c)  
        800f7bc4 05 d2 19 05     ld.bu      d2, BYTE_d0001019                                = ??
        800f7bc8 0b 42 00 21     eq         d2, d2, param_1
        800f7bcc 00 90           ret

bool FUN_800f7bc4(byte param_1)
{
  undefined3 in_register_0000ff11;     // this one also contains zeros!
  
  p0 = 0x8004d4a4d0009600;
  return (uint)BYTE_d0001019 == CONCAT31(in_register_0000ff11,param_1);
}

Expected behavior

I don't understand why @emteere said (#5757 (comment)) that such extension is not needed.
If declare all input/output pentries in cspec file with attribute extension="inttype" all these extraout / in_registerXX is gone.

Yes, I know workaroud 'always use native-wide variables'. But I think will be better to use byte, if you sure that this variable always contains values less than 255.

Environment (please complete the following information):

• Ghidra Version: b070f86
• Ghidra Origin: actual github master

[emteere]

My apologies. I should have read more closely.

@esaulenka my mistake in removing the extension="inttype". I had mistakenly removed it thinking it was metatype="inttype".
I'll put it back in. metatype is being deprecated, extension is not.

I'll add that into a tricore patch I'm about to commit.

[esaulenka]

@ryanmkurtz, why you marked it as closed?
There are a few others architectures that can be improved in the same way.

I can provide samples for it, but decompiled code will be exactly the same.

[ryanmkurtz]

Oh, it was auto-closed due to our merge process and an misunderstanding of how our internal work linked to this issue.

[emteere]

The PPC has extension="inttype" on the return value, but not on the parameters. Based on pulling up a pcode unit test PPC binary, it appears it needs it on the parameters as well.

char pcode_i1_complexLogic
               (char param_1,char param_2,char param_3,char param_4,char param_5,char param_6)
{
  char cVar1;
  char cVar2;
  undefined3 in_register_0000000c;
  uint uVar3;
  undefined3 in_register_00000010;
  uint uVar4;
  undefined3 in_register_00000014;
  uint uVar5;
  undefined3 in_register_00000018;
  uint uVar6;
  undefined3 in_register_0000001c;
  uint uVar7;
  undefined3 in_register_00000020;
  uint uVar8;
  byte bVar9;
  
  uVar8 = CONCAT31(in_register_00000020,param_6);
  uVar7 = CONCAT31(in_register_0000001c,param_5);
  uVar6 = CONCAT31(in_register_00000018,param_4);
  uVar5 = CONCAT31(in_register_00000014,param_3);
  uVar4 = CONCAT31(in_register_00000010,param_2);
  uVar3 = CONCAT31(in_register_0000000c,param_1);

The need for the extension tag would depend on the compiler and how it treats passing/returning smaller data types than the full register size. The mips needs extension="sign". The ABI specifies explicitly what is assumed for smaller data types in params and returns.

We've been considering adding an automated test for this in the pcodeunit test binaries. This does depend on having a compiler that can cross-compile a test binary.

We can do a zero based review with the test binaries we have with known small datatype parameters and return values.