Solo5 / solo5

A sandboxed execution environment for unikernels
ISC License
883 stars 136 forks source link

Fix that GDB HVT bug #567

Closed greydot closed 4 months ago

greydot commented 4 months ago

https://github.com/Solo5/solo5/issues/548

Took me a while to get to this, mostly because GDB wire format is weird.

greydot commented 4 months ago

P.S. While debugging this, I also implemented KVM_GET_FPU code, but it's not really tested in the field (seems to work fine tho) and I'm not sure whether I should include it in this PR.

dinosaure commented 4 months ago

P.S. While debugging this, I also implemented KVM_GET_FPU code, but it's not really tested in the field (seems to work fine tho) and I'm not sure whether I should include it in this PR.

The best is probably to make another PR to review on it :+1:. Otherwise, this PR looks fine to me.

reynir commented 4 months ago

Hi!

It's been about a year since I last looked at this. From reading my comments in #548 it seems we should maybe support the qSupported GDB debug protocol query. If I remember correctly that way we can tell GDB the register layout(?!) but then again I remember it was very difficult to find any documentation on the protocol.

Looking at the XML file it seems this change is correct. Maybe GDB can guess the layout just from the length of the hex string?? I'm in favor of merging this as this is at least less wrong.

greydot commented 4 months ago

It's been about a year since I last looked at this. From reading my comments in #548 it seems we should maybe support the qSupported GDB debug protocol query. If I remember correctly that way we can tell GDB the register layout(?!) but then again I remember it was very difficult to find any documentation on the protocol.

I briefly looked through the feature list that qSupported operates on and it doesn't have anything with registers. I might be wrong here though.

Looking at the XML file it seems this change is correct. Maybe GDB can guess the layout just from the length of the hex string?? I'm in favor of merging this as this is at least less wrong.

Not exactly. It seems that GDB operates either on individual registers or on groups of them (again, see XML). The error in this case was due to EFLAGS being longer than GDB expected, so the following segment registers were shifted down the line. GDB thought there was partial ST0 register data sent and hence threw the error "Truncated register 24 in remote 'g' packet", with "register 24" being ST0 (haha i actually counted).

I'm not sure this GDB server implementation has ever worked correctly. Or at the very least, values for segment registers were always shown wrong in GDB on x86_64.

greydot commented 4 months ago

Sooo... I suppose, this can be merged now? @dinosaure

dinosaure commented 4 months ago

Thanks!