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

Broken Floating Point Registers in GDB #84

Open
fpedd opened this issue Sep 30, 2021 · 0 comments
Open

Broken Floating Point Registers in GDB #84

fpedd opened this issue Sep 30, 2021 · 0 comments

Comments

@fpedd
Copy link
Collaborator

fpedd commented Sep 30, 2021

Broken Floating Point Registers

Currently, the display of floating point registers is broken. This leads to double-precision values being incorrectly displayed.

Take these to variables:

register double a asm ("f0") = 3.1415;
register float b asm ("f1") = 2.7182;

which are displayed as:

ft0            {float = -4.09600019, double = 6.9528787831812155e-310}	(raw 0x00007ffdc083126f)
ft1            {float = 2.71819997, double = 6.9527724078223043e-310}	(raw 0x00007ffd402df6fd)

in GDB when issuing info all-register. While the single-precision variable is correctly displayed, the double-precision variable is not.

The problem lies in the fact, that the current RISC-V Arch is a RV32IMAFDC. This means, that it supports double-precision instructions and has 64bit floating point registers (FLEN=64). The ETISS GDB server, however, only responds with 4 bytes (instead of 8). This is caused by the FloatRegField_RISCV class in RISCVArchSpecificImp.h beeing setup for single-precision/32bit:

class FloatRegField_RISCV : public BaseField_RISCV<uint32_t>
{
public:
    FloatRegField_RISCV(etiss::VirtualStruct &parent, int id)
        : BaseField_RISCV<uint32_t>(parent, "F" + std::to_string(id), 4, &((RISCV *)parent.structure_)->F[id])
    {
    }
};

Simply adjusting it to double-precision/64bit:

class FloatRegField_RISCV : public BaseField_RISCV<uint64_t>
{
public:
    FloatRegField_RISCV(etiss::VirtualStruct &parent, int id)
        : BaseField_RISCV<uint64_t>(parent, "F" + std::to_string(id), 8, &((RISCV *)parent.structure_)->F[id])
    {
    }
};

fixed the issue:

ft0            {float = -4.09600019, double = 3.1415000000000002}	(raw 0x400921cac083126f)
ft1            {float = 2.71819997, double = -nan(0xfffff402df6fd)}	(raw 0xffffffff402df6fd)

This way the implementation is also conformant to the RISC-V spec (section 9.2), in that it adheres to the "NaN Boxing of Narrower Values" in which "the upper bits of a valid NaN-boxed value must be all 1s".

mappedRegisterCount()

Additionally, I noticed that the mappedRegisterCount() in RISCVGDBCore.h returns 32. This would imply that the implementation would "only" have 32 mapped registers, which is obviously not the case, see RISCV.h.

Actually, this doesn't seem to be that big of an issue, as GDB simply one-by-one requests registers not set to it when it sends a "Read general registers" g request (which only sents 32) using multiple p requests afterwards. This is why I can still see floating-point registers in GDB.

At least including the PC, as is done in the RISCV64GDBCore.h, is probably a good idea nonetheless, i.e. changing it to 33. While one could go all the way up to 64, including the floating-point registers is probably not really necessary and just causes unnecessary overhead. However, I don't know how many registers are actually expected by GDB when it sends a "Read general registers" g request. The onlinedocs don't give any information on that either.

The only reason I am bringing this up in the first place is that the G "Write general registers." will probably always fail the way it is implemented right now, see GDBServer.cpp.

case 'G': // write registers
{
size_t treglen = 0;
for (unsigned i = 0; i < arch_->getGDBCore().mappedRegisterCount(); i++)
{
auto f = plugin_core_->getStruct()->findName(arch_->getGDBCore().mapRegister(i));
if (!f)
{
answer = "EFF";
etiss::log(etiss::ERROR, "Faulty implementation of the GDBCore: Register not found",
arch_->getGDBCore().mapRegister(i), *plugin_core_);
break;
}
treglen += f->width_;
}
if (command.length() == (treglen * 2) + 1)
{
answer = "OK";
size_t off = 1;

The for-loop, which is dependent on mappedRegisterCount(), will create a total size variable treglen and compare it against the incoming command.length(). If they don't match, the request will fail. Thus, it is crucial that mappedRegisterCount() returns the exact number of registers expected by GDBs G request.

I haven't gotten GDB to emit a G request and don't really know how to do so or when it is used. So I am not sure this is actually that relevant, but I just wanted to mention it here.

On another note

@rafzi you were right about the RISC-V vector support in GDB. While the latest GDB 11 release from June this year does have basic support for vector registers, see here and here, my rvv0.8 toolchain (with GDB 8.3.0) does not appear to have any RISC-V vector support. So I will postpone any work on GDB vector support for ETISS.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant