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

All registers are byteswapped when using lldb #119

Closed
lemmy-64 opened this issue Jan 14, 2023 · 14 comments
Closed

All registers are byteswapped when using lldb #119

lemmy-64 opened this issue Jan 14, 2023 · 14 comments
Labels
lldb-compat LLDB-specific protocol extension

Comments

@lemmy-64
Copy link

lemmy-64 commented Jan 14, 2023

Hi (and sorry for filing so many bugs at once).

I integrated gdbstub into my Nintendo 64 emulator and things mostly work with gdb. However, when I connect with lldb, all registers are byteswapped. I assume this has something to do with the N64 being big-endian and lldb and gdb being too different.

I might be completely wrong here, but I wonder if document explains that: https://opensource.apple.com/source/lldb/lldb-112/docs/lldb-gdb-remote.txt.auto.html (notice that qHostInfo has an endian flag)?

The data that I should be seeing is:

  • PC: FFFFFFFF 800523DC
  • r1: FFFFFFFF 802E2F2C
(lldb) gdb-remote localhost:9001
Process 1 stopped
* thread #1, stop reason = signal SIGTRAP
    frame #0: 0xdc230580ffffffff
error: memory read failed for 0xdc230580fffffe00
Target 0: (n64-systemtest) stopped.
(lldb) register read
general:
        r0 = 0x0000000000000000
        r1 = 0x2c2f2e80ffffffff
        r2 = 0x1c00000000000000
        r3 = 0x00002e80ffffffff
        r4 = 0x102f2e80ffffffff
        r5 = 0x040040a4ffffffff
        r6 = 0x002c010000000000
        r7 = 0x002c010000000000
        r8 = 0xf92b010000000000
        r9 = 0x002c010000000000
       r10 = 0x0000000000000000
       r11 = 0x0000000000000000
       r12 = 0x0100000000000000
       r13 = 0xf4622c80ffffffff
       r14 = 0x2e02000000000000
       r15 = 0x0100000000000000
       r16 = 0xff00000000000000
       r17 = 0x1c00000000000000
       r18 = 0x102f2e80ffffffff
       r19 = 0x00002e80ffffffff
       r20 = 0x0100000000000000
       r21 = 0x0000000000000000
       r22 = 0x3f00000000000000
       r23 = 0x0100000000000000
       r24 = 0x3e00000000000000
       r25 = 0x0900000000000000
       r26 = 0x0000000000000000
       r27 = 0x0000000000000000
       r28 = 0x0900000000000000
       r29 = 0xb0ff7f80ffffffff
       r30 = 0x0000000000000000
       r31 = 0x68210580ffffffff
        lo = 0xc011010000000000
        hi = 0x0000000000000000
        pc = 0xdc230580ffffffff
    status = 0x0000002400000000
  badvaddr = 0x000000f07f000000
     cause = 0x3480008000000000
@daniel5151
Copy link
Owner

daniel5151 commented Jan 14, 2023

Some past issues that might be relevant: #26, #97, and #99

One thing i'll mention right out of the gate is that gdbstub doesn't yet make any guarantees about LLDB compatibility. We've got some LLDB extensions here-and-there, but they've all been added in a pretty ad-hoc manner, and only when they wouldn't interfere with regular GDB operations.

In this case, it may very well be the case that for gdbstub to play nice with lldb on your arch, it'll need to be extended with support for qHostInfo. Thankfully, based on that packet description, it wouldn't be too difficult to add support for it, and I'd be happy to accept a PR that implements it!

I would expect the implementation to follow the same broad strokes as #103 (insofar as it would touch both the Target and Arch traits). The actual implementation should be significantly simpler though, as you wouldn't need to futz with any weird callback stuff (since the data would be fixed-size - you'd just have the new extension method return a regular 'ol struct)

Let me know what you find, and if you'd like to take a stab at that functionality!

P.S: consider first patching gdbstub to respond with a hard-coded qHostInfo response specific to your arch, and see if that fixes things

@lemmy-64
Copy link
Author

lemmy-64 commented Jan 17, 2023

For reference, this is the log when attaching lldb:

 INFO  gdbdebugger::gdb_integration > Debugger connected from [::1]:64691
 TRACE gdbstub::protocol::recv_packet > <-- +
 TRACE gdbstub::protocol::recv_packet > <-- $QStartNoAckMode#b0
 TRACE gdbstub::protocol::response_writer > --> $OK#9a
 TRACE gdbstub::protocol::recv_packet     > <-- +
 TRACE gdbstub::protocol::recv_packet     > <-- $qSupported:xmlRegisters=i386,arm,mips,arc;multiprocess+;fork-events+;vfork-events+#2e
 TRACE gdbstub::protocol::response_writer > --> $PacketSize=1000;vContSupported+;multiprocess+;QStartNoAckMode+;swbreak+;hwbreak+;qXfer:features:read+#47
 TRACE gdbstub::protocol::recv_packet     > <-- $QThreadSuffixSupported#e4
 INFO  gdbstub::stub::core_impl           > Unknown command: Ok("QThreadSuffixSupported")
 TRACE gdbstub::protocol::response_writer > --> $#00
 TRACE gdbstub::protocol::recv_packet     > <-- $QListThreadsInStopReply#21
 INFO  gdbstub::stub::core_impl           > Unknown command: Ok("QListThreadsInStopReply")
 TRACE gdbstub::protocol::response_writer > --> $#00
 TRACE gdbstub::protocol::recv_packet     > <-- $vCont?#49
 TRACE gdbstub::protocol::response_writer > --> $vCont;c;C#26
 TRACE gdbstub::protocol::recv_packet     > <-- $qVAttachOrWaitSupported#38
 INFO  gdbstub::stub::core_impl           > Unknown command: Ok("qVAttachOrWaitSupported")
 TRACE gdbstub::protocol::response_writer > --> $#00
 TRACE gdbstub::protocol::recv_packet     > <-- $QEnableErrorStrings#8c
 INFO  gdbstub::stub::core_impl           > Unknown command: Ok("QEnableErrorStrings")
 TRACE gdbstub::protocol::response_writer > --> $#00
 TRACE gdbstub::protocol::recv_packet     > <-- $qHostInfo#9b
 INFO  gdbstub::stub::core_impl           > Unknown command: Ok("qHostInfo")
 TRACE gdbstub::protocol::response_writer > --> $#00
 TRACE gdbstub::protocol::recv_packet     > <-- $qProcessInfo#dc
 INFO  gdbstub::stub::core_impl           > Unknown command: Ok("qProcessInfo")
 TRACE gdbstub::protocol::response_writer > --> $#00
 TRACE gdbstub::protocol::recv_packet     > <-- $qC#b4
 INFO  gdbstub::stub::core_impl           > Unknown command: Ok("qC")
 TRACE gdbstub::protocol::response_writer > --> $#00
 TRACE gdbstub::protocol::recv_packet     > <-- $qfThreadInfo#bb
 TRACE gdbstub::protocol::response_writer > --> $mp01.01#cd
 TRACE gdbstub::protocol::recv_packet     > <-- $qsThreadInfo#c8
 TRACE gdbstub::protocol::response_writer > --> $l#6c
 TRACE gdbstub::protocol::recv_packet     > <-- $?#3f
 TRACE gdbstub::protocol::response_writer > --> $T05thread:p01.01;#06
 TRACE gdbstub::protocol::recv_packet     > <-- $qProcessInfo#dc
 INFO  gdbstub::stub::core_impl           > Unknown command: Ok("qProcessInfo")
 TRACE gdbstub::protocol::response_writer > --> $#00
 TRACE gdbstub::protocol::recv_packet     > <-- $qProcessInfo#dc
 INFO  gdbstub::stub::core_impl           > Unknown command: Ok("qProcessInfo")
 TRACE gdbstub::protocol::response_writer > --> $#00
 TRACE gdbstub::protocol::recv_packet     > <-- $qProcessInfo#dc
 INFO  gdbstub::stub::core_impl           > Unknown command: Ok("qProcessInfo")
 TRACE gdbstub::protocol::response_writer > --> $#00
 TRACE gdbstub::protocol::recv_packet     > <-- $qXfer:features:read:target.xml:0,fff#7d
 TRACE gdbstub::protocol::response_writer > --> $m
<!DOCTYPE feature SYSTEM "gdb-target.dtd">
<target version="1.0">
<architecture>mips:4300</architecture>
<osabi>none</osabi>
<endian>big</endian>
<feature name="org.gnu.gdb.mips.cpu">
    <reg name="r0" bitsize="64" regnum="0"/>
    <reg name="r1" bitsize="64"/>
    <reg name="r2" bitsize="64"/>
    <reg name="r3" bitsize="64"/>
    <reg name="r4" bitsize="64"/>
    <reg name="r5" bitsize="64"/>
    <reg name="r6" bitsize="64"/>
    <reg name="r7" bitsize="64"/>
    <reg name="r8" bitsize="64"/>
    <reg name="r9" bitsize="64"/>
    <reg name="r10" bitsize="64"/>
    <reg name="r11" bitsize="64"/>
    <reg name="r12" bitsize="64"/>
    <reg name="r13" bitsize="64"/>
    <reg name="r14" bitsize="64"/>
    <reg name="r15" bitsize="64"/>
    <reg name="r16" bitsize="64"/>
    <reg name="r17" bitsize="64"/>
    <reg name="r18" bitsize="64"/>
    <reg name="r19" bitsize="64"/>
    <reg name="r20" bitsize="64"/>
    <reg name="r21" bitsize="64"/>
    <reg name="r22" bitsize="64"/>
    <reg name="r23" bitsize="64"/>
    <reg name="r24" bitsize="64"/>
    <reg name="r25" bitsize="64"/>
    <reg name="r26" bitsize="64"/>
    <reg name="r27" bitsize="64"/>
    <reg name="r28" bitsize="64"/>
    <reg name="r29" bitsize="64"/>
    <reg name="r30" bitsize="64"/>
    <reg name="r31" bitsize="64"/>
    <reg name="lo" bitsize="64" regnum="33"/>
    <reg name="hi" bitsize="64" regnum="34"/>
    <reg name="pc" bitsize="64" regnum="37"/>
</feature>
<feature name="org.gnu.gdb.mips.cp0">
    <reg name="status" bitsize="64" regnum="32"/>
    <reg name="badvaddr" bitsize="64" regnum="35"/>
    <reg name="cause" bitsize="64" regnum="36"/>
</feature>
<feature name="org.gnu.gdb.mips.fpu">
    <reg name="f0" bitsize="64" type="ieee_double" regnum="38"/>
    <reg name="f1" bitsize="64" type="ieee_double"/>
    <reg name="f2" bitsize="64" type="ieee_double"/>
    <reg name="f3" bitsize="64" type="ieee_double"/>
    <reg name="f4" bitsize="64" type="ieee_double"/>
    <reg name="f5" bitsize="64" type="ieee_double"/>
    <reg name="f6" bitsize="64" type="ieee_double"/>
    <reg name="f7" bitsize="64" type="ieee_double"/>
    <reg name="f8" bitsize="64" type="ieee_double"/>
    <reg name="f9" bitsize="64" type="ieee_double"/>
    <reg name="f10" bitsize="64" type="ieee_double"/>
    <reg name="f11" bitsize="64" type="ieee_double"/>
    <reg name="f12" bitsize="64" type="ieee_double"/>
    <reg name="f13" bitsize="64" type="ieee_double"/>
    <reg name="f14" bitsize="64" type="ieee_double"/>
    <reg name="f15" bitsize="64" type="ieee_double"/>
    <reg name="f16" bitsize="64" type="ieee_double"/>
    <reg name="f17" bitsize="64" type="ieee_double"/>
    <reg name="f18" bitsize="64" type="ieee_double"/>
    <reg name="f19" bitsize="64" type="ieee_double"/>
    <reg name="f20" bitsize="64" type="ieee_double"/>
    <reg name="f21" bitsize="64" type="ieee_double"/>
    <reg name="f22" bitsize="64" type="ieee_double"/>
    <reg name="f23" bitsize="64" type="ieee_double"/>
    <reg name="f24" bitsize="64" type="ieee_double"/>
    <reg name="f25" bitsize="64" type="ieee_double"/>
    <reg name="f26" bitsize="64" type="ieee_double"/>
    <reg name="f27" bitsize="64" type="ieee_double"/>
    <reg name="f28" bitsize="64" type="ieee_double"/>
    <reg name="f29" bitsize="64" type="ieee_double"/>
    <reg name="f30" bitsize="64" type="ieee_double"/>
    <reg name="f31" bitsize="64" type="ieee_double"/>
    <reg name="fcsr" bitsize="64" group="float"/>
    <reg name="fir" bitsize="64" group="float"/>
</feature>
</target>#0c
 TRACE gdbstub::protocol::recv_packet     > <-- $qXfer:features:read:target.xml:dd6,fff#4b
 TRACE gdbstub::protocol::response_writer > --> $l#6c
 TRACE gdbstub::protocol::recv_packet     > <-- $qRegisterInfo0#72
 INFO  gdbstub::stub::core_impl           > Unknown command: Ok("qRegisterInfo0")
 TRACE gdbstub::protocol::response_writer > --> $#00
 TRACE gdbstub::protocol::recv_packet     > <-- $Hg1#e0
 TRACE gdbstub::protocol::response_writer > --> $OK#9a
 TRACE gdbstub::protocol::recv_packet     > <-- $p0#a0
 INFO  gdbstub::stub::core_impl           > Unknown command: Ok("p0")
 TRACE gdbstub::protocol::response_writer > --> $#00
 TRACE gdbstub::protocol::recv_packet     > <-- $qProcessInfo#dc
 INFO  gdbstub::stub::core_impl           > Unknown command: Ok("qProcessInfo")
 TRACE gdbstub::protocol::response_writer > --> $#00
 TRACE gdbstub::protocol::recv_packet     > <-- $qStructuredDataPlugins#02
 INFO  gdbstub::stub::core_impl           > Unknown command: Ok("qStructuredDataPlugins")
 TRACE gdbstub::protocol::response_writer > --> $#00
 TRACE gdbstub::protocol::recv_packet     > <-- $qShlibInfoAddr#6a
 INFO  gdbstub::stub::core_impl           > Unknown command: Ok("qShlibInfoAddr")
 TRACE gdbstub::protocol::response_writer > --> $#00
 TRACE gdbstub::protocol::recv_packet     > <-- $qfThreadInfo#bb
 TRACE gdbstub::protocol::response_writer > --> $mp01.01#cd
 TRACE gdbstub::protocol::recv_packet     > <-- $qsThreadInfo#c8
 TRACE gdbstub::protocol::response_writer > --> $l#6c
 TRACE gdbstub::protocol::recv_packet     > <-- $qfThreadInfo#bb
 TRACE gdbstub::protocol::response_writer > --> $mp01.01#cd
 TRACE gdbstub::protocol::recv_packet     > <-- $qsThreadInfo#c8
 TRACE gdbstub::protocol::response_writer > --> $l#6c
 TRACE gdbstub::protocol::recv_packet     > <-- $jThreadsInfo#c1
 INFO  gdbstub::stub::core_impl           > Unknown command: Ok("jThreadsInfo")
 TRACE gdbstub::protocol::response_writer > --> $#00
 TRACE gdbstub::protocol::recv_packet     > <-- $jThreadExtendedInfo:#b9
 INFO  gdbstub::stub::core_impl           > Unknown command: Ok("jThreadExtendedInfo:")
 TRACE gdbstub::protocol::response_writer > --> $#00

@daniel5151
Copy link
Owner

Yep, there sure are a bunch of unimplemented LLDB packets there 😄
Aside from those though, everything looks about how I'd expect.

I think the next step is definitely to try implementing a basic qHostInfo response, and seeing what happens.


P.S: in the future, post logs in a code block, or else those checksums end up getting formatted as issue-links, which is a little weird lol

@daniel5151 daniel5151 added the lldb-compat LLDB-specific protocol extension label Jan 17, 2023
@lemmy-64
Copy link
Author

lemmy-64 commented Feb 8, 2023

Some updates on this. It turned out that actually lldb was right and gdb was wrong :)

When I filed this bug, my memory reading was essentially pretending to be little endian, which didn't work for anything that isn't 32 bit. So I changed my memory read function to correctly return big endian. As I couldn't find a way to tell gdb that the system is big endian, I now have to manually call "set endian big" in gdb every time I debug.

Additionally, this meant that I now have to swap the registers in read_registers() like this:

Register read:

    fn read_registers(...) {
        regs.pc = pc.swap_bytes();
        let main_registers = *self.nemu.main_registers();
        for i in 0..32 {
            regs.r[i] = main_registers[i].swap_bytes();
        }
        regs.lo = self.nemu.mult_lo().swap_bytes();
        regs.hi = self.nemu.mult_hi().swap_bytes();

With this, both gdb and lldb work fine.

However, this doesn't feel quite right. Two questions about this:
a) Do you know if it's possible to tell gdb that a system is big endian so that "set endian big" doesn't need to be called?
b) Byteswapping in read_registers() doesn't sound like a good API for gdbstub. Maybe it would be worth having a big endian mode so that the byteswapping happens inside of the stub?

@daniel5151
Copy link
Owner

Oh, okay, very interesting! This definitely has to do with #26 then...

Unfortunately, I really don't have any hands-on experience working with big-endian systems, so I don't have answers/opinions to either question off the top of my head. Coming to a conclusion there will require some more research / digging into the GDB source code.

Not sure if/when I'll have a chance to sit down and do that, but if you can do the research into what the behavior should be, I can certainly try and set aside some time to whip up some gdbstub-side code / docs to make things as easy as possible for big-endian systems :)

@daniel5151
Copy link
Owner

Though, poking through the GDB docs a bit:

Some types of processors, such as the MIPS, PowerPC, and Renesas SH, offer the ability to run either big-endian or little-endian byte orders. Usually the executable or symbol will include a bit to designate the endian-ness, and you will not need to worry about which to use. However, you may still find it useful to adjust GDB’s idea of processor endian-ness manually.

https://sourceware.org/gdb/onlinedocs/gdb/Byte-Order.html#Byte-Order

So it might be the case that there's a bit of build-time metadata you're setting incorrectly when building your MIPS executables?

@lemmy-64
Copy link
Author

lemmy-64 commented Feb 8, 2023

The problem for me has been that if I pass the binary as an argument to gdb then gdb will autodetect the arch and get everything wrong (I get incorrect package sizes). It seems to ignore (or at least mostly ignore) target_description_xml in that case and just go with some arch that it thinks is the right one.

The only thing that works for me is:

  • Launch gdb without a binary, but connect to the server
  • set endian big
  • symbol-file path/to/binary
  • continue/single-step

Which is...not a great workflow at all. It also has the downside of gdb trying its hardest to run the program at launch - so if you want to debug something early on it might have already happened.

@daniel5151
Copy link
Owner

I hate to say it, but I'm not convinced this is an issue with gdbstub per-se, and is moreso an issue with client-side GDB ergonomics and/or how you're stuffing OS/ABI/endianess info into the executable you're debugging.

Of course, i'm happy to be convinced otherwise. e.g: if you can find some other n64 emulator with a gdbstub that does the Right Thing here, we could compare what its doing vs. what gdbstub + your impl is doing and go from there?

@lemmy-64
Copy link
Author

lemmy-64 commented Feb 8, 2023

Yeah I don't think this is an issue with gdbstub either

@lemmy-64
Copy link
Author

lemmy-64 commented Feb 8, 2023

The only things I see for gdbstub would be:

  • Byteswap registers in big endian mode
  • (If possible) Try to tell gdb that we're big endian

Anything else is just gdb being annoying

@daniel5151
Copy link
Owner

If that's the case... I wouldn't be opposed to adding some kind of optional auto-byteswap feature into gdbstub as a convenience, though as always, we'd have to take special care to make sure it doesn't unnecessarily bloat gdbstub when its not being used.

I'm not sure when I'd have the time to sit down and take a crack at implementing something like that, but if you'd like to take a crack at it yourself, I'd happily review any PR sent my way :)

@lemmy-64
Copy link
Author

lemmy-64 commented Feb 8, 2023

The workaround is pretty simple - I can byteswap the registers myself. I mostly wanted to confirm with you that this is the right thing to do.

You could also leave this as-is frankly. It's a little weird to provide byteswapped u64, but in the grand picture this is one of the smaller tasks to do and it can easily be handled by the client. Having it documented might be useful however.

@daniel5151
Copy link
Owner

Oh! I just realized: if you're using the built-in MIPS arch implementation from gdbstub_arch, it implements gdb_serialize and gdb_deserialize using to/from_le_bytes!

As such, I think the real solution here is that you should simply re-implement the MIPS Arch impl using to/from_be_bytes, which should be the true solution to the problem. Probably should've called that out sooner 😅

If I'm right, and that makes sense, then I think the appropriate doc update is actually far more simple: "NOTE: gdbstub_arch::mips is implemented for little-endian MIPS systems"

@daniel5151
Copy link
Owner

Given that the only outstanding ask from this issue is a doc update, I think I'm going to close this out for now.

Feel free to send in the doc update if you'd like!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lldb-compat LLDB-specific protocol extension
Projects
None yet
Development

No branches or pull requests

2 participants