PowerPC VLE se_cmpli immediate value wrong

[revofrank]

Description:

When decoding an se_cmpli instruction the OIM5 field value is used instead of the OIMM.

Ghidra example from a PowerPC:BE:64:VLE-32addr file: 00613e12 22 33 se_cmpli r3,0x3

Whereas Ida displays the expected value: .blk31:00613E12 se_cmpli r3, 4

The fix seems to be changing the ppc_vle.sinc file se_cmpli "OIM5_VLE" references to "OIMM".

Environment: Ghidra 9.04 on Win10 with Java 11.0.2

[revofrank]

Thank you for that :se_cmpli patch in ppc_vle.sinc.

I have just one final concern on line 594 which now is: tmpB:4 = OIMM:4 & 0x1f;

OIMM will be in the 0x01 through 0x20 range inclusive. Should OIMM happen to be 0x20 then tmpB would be incorrectly set to 0? And thus incorrectly updating cr0?

Should line 594 perhaps be: tmpB:4 = OIMM:4 & 0x3f; or just: tmpB:4 = OIMM:4;

I wouldn't know which one of the above would be more appropriate.

I probably should have included the se_cmpli description from the Book E Programmer's reference manual:

If se_cmpli, the contents of GPR(rX) are compared with the value of the zero-extended offset value of the OIM5 field (a final value in the range 1–32), treating the operands as unsigned integers. The result of the comparison is placed into CR field 0.

[mumbel]

sorry, somehow missed this in my emails (but noticed the PR getting assigned today). I really forget my logic for the 0x1F now, and you're right it may be incorrect. I think the reasoning was OIMM is OIM5+1 and the 0x1F would mask OIM5 correctly, but should really be applied to OIM5 and not OIMM. Ill try to get around to taking a second look soon, certainly since devs are looking at the PR now.

[mumbel]

I pushed a fix to the PR (no masking now). thanks for catching it

[revofrank]

Fix looks good to me, so I'll go ahead and close the bug report.