Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RISCV: nanboxing #6541

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Sleigh-InSPECtor
Copy link
Contributor

As part of a research project testing the accuracy of the sleigh specifications compared to real hardware, we observed an unexpected behaviour in the single precision float instructions for RISCV. According to Section 12.2, when multiple floating-point precisions are supported the narrower value must be NaN-boxed by setting all upper bits to 1's. When an incorrectly NaN-boxed value is read the input is treated as NaN. The current behaviour ignores the upper bits and zero extends single precision floats. While this pull request does not cover them this is also an issue for half and double precision floats.

It was also found that the fcvt.s.d instruction did not use the fassignS macro and instead wrote directly to frd.

Correctly handling this cases does bloat disassembly. The or'ing of the value to 0xffffffff00000000 on lines 4 and 7 of "Decompilation with fix" are from the flw instructions as they are NaN-boxed after being read off the stack. The rest of the logic on lines 4-9 is from the fadd.s instruction checking if its inputs are NaN-boxed. The or'ing on line 11 is from the fadd.s instruction NaN-boxing its output. The rest of the logic on lines 11-13 is from the fmv.x.w instruction checking if its input is correctly NaN-boxed (both decompilations includes the patch #6492).

Compiler generated assembly

0001015a 01 11           c.addi     sp,-0x20
	 assume gp = <UNKNOWN>
0001015c 22 ce           c.swsp     s0,0x1c(sp)
0001015e 00 10           c.addi4spn s0,sp,0x20
00010160 23 26 a4 fe     sw         a0,-0x14=>local_14(s0)
00010164 23 24 b4 fe     sw         a1,-0x18=>local_18(s0)
00010168 07 27 c4 fe     flw        fa4,-0x14=>local_14(s0)
0001016c 87 27 84 fe     flw        fa5,-0x18=>local_18(s0)
00010170 d3 77 f7 00     fadd.s     fa5,fa4,fa5,dyn
00010174 53 85 07 e0     fmv.x.w    a0,fa5
00010178 72 44           c.lwsp     s0,0x1c(sp)
0001017a 05 61           c.addi16sp sp,0x20
0001017c 82 80           ret

Decompilation without fix

float float_add(float param_1,float param_2)
{
  gp = &__global_pointer$;
  return param_1 + param_2;
}

Decompilation with fix

float float_add(float param_1,float param_2)
{
  gp = &__global_pointer$;
  if ((longlong)((ulonglong)(uint)param_2 | 0xffffffff00000000) >> 0x20 != -1) {
    param_2 = NAN;
  }
  if ((longlong)((ulonglong)(uint)param_1 | 0xffffffff00000000) >> 0x20 != -1) {
    param_1 = NAN;
  }
  param_1 = param_1 + param_2;
  if ((longlong)((ulonglong)(uint)param_1 | 0xffffffff00000000) >> 0x20 != -1) {
    param_1 = NAN;
  }
  return param_1;
}

It should be possible to include most of the functionality of this fix while also generating cleaner decompilation by changing the condition on lines 91, 98, and 105 to nan(frXXXX) instead of bit shifting and comparing. If you set "NaN operations" under the Analysis tab on the "Options for Code Brower" window to "ignore all", this should cut out all of the if statements but Ghidra seems to get confused and generates the following decompilation.

undefined4 float_add(void)
{
  gp = &__global_pointer$;
  return 0x7fc00000;
}

As this greatly impacts the readability of the current decompilation without adding too much to analysis it has been left as a draft.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature: Processor/RISC-V Status: Triage Information is being gathered
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants