Page MenuHomePhabricator

[LLDB][LLGS Test] Check length of register, only when its available
ClosedPublic

Authored by mohit.bhakkad on Oct 19 2015, 4:45 AM.

Details

Summary

Some machines of an arch may not have all the regs listed for it in RegInfo, and we get 'E15' as error while reading from such register, i.e. its not available.

For example MSA registers listed for MIPS are not present in all MIPS boards.

Diff Detail

Repository
rL LLVM

Event Timeline

mohit.bhakkad retitled this revision from to [LLDB][LLGS Test] Check length of register, only when its available.
mohit.bhakkad updated this object.
mohit.bhakkad added reviewers: clayborg, zturner.
mohit.bhakkad set the repository for this revision to rL LLVM.
mohit.bhakkad added subscribers: jaydeep, bhushan, slthakur and 2 others.
labath added a subscriber: labath.Oct 19 2015, 7:05 AM

If the registers are not present, wouldn't it be better to *not* include them in qRegisterInfo response in the first place?

clayborg requested changes to this revision.Oct 19 2015, 10:42 AM
clayborg edited edge metadata.

We can't rely on any specific error codes since different GDB remote servers will hand back different errors for any packet. Do you have control over the MIPS debugserver? Is this just lldb-server? If so, we should have it not tell LLDB about a register it doesn't have. So I agree with labath that the GDB server should not tell us about this register in the first place.

This revision now requires changes to proceed.Oct 19 2015, 10:42 AM
mohit.bhakkad edited edge metadata.

This patch makes server send an End of register(E45) response if a register present in reginfo is not available on actual machine.

I think this should go in a little bit deeper, i.e., inside the NativeRegisterContextLinux_mips, or whatever is the right class for you. I think this is better for several reasons:

  • if the register is physically not present, the register context should not report it (through GetUserRegisterCount, GetRegisterInfoAtIndex, etc.) - someone else may be iterating through the registers in the context and doing stuff - he will get errors when encountering your registers
  • this can cause us to silently lose some registers if we fail to read them for any reason (due to an unrelated bug somewhere)
clayborg requested changes to this revision.Oct 20 2015, 10:16 AM
clayborg edited edge metadata.

Is there a list of error codes and what they mean for the GDB remote protocol? debugserver returns random errors and doesn't abide by any specific error codes. Quote from some GDB remote protocol docs I found:

The error response returned for some packets includes a two character error number. That number is not well defined.

So we can't rely on specific responses unless you verify that the error is coming from a specific GDB server. In order to do this, we would need to add a qGDBServerInfo packet that could return a name, version info, etc:

$qGDBServerInfo#00
$name:debugserver;version:123.2.3;

The real fix is to fix the GDB server to not have it return this register if it is not available.

This revision now requires changes to proceed.Oct 20 2015, 10:16 AM
mohit.bhakkad edited edge metadata.

Changes in this revision:

  • Fixed GetUserRegisterCount () to get count registers which are actually present.

This will make llgs send E45 when reg_index is of some unavailable reg.

clayborg accepted this revision.Nov 2 2015, 10:50 AM
clayborg edited edge metadata.

Much better.

This revision is now accepted and ready to land.Nov 2 2015, 10:50 AM
This revision was automatically updated to reflect the committed changes.