Support qGetTLSAddr

[mkroening]

I noticed that this crate does not support qGetTLSAddr for working with thread-local storage.

Although I do not currently need this myself, I thought it would be good to track this issue.

[daniel5151]

Thanks for popping open the issue - always good to know what protocol features folks are interested in!

A cursory glance through the protocol docs suggests that this should be a pretty trivial addition, and wouldn't require any breaking API changes. i.e: this can be implemented as a standard IDET.

https://sourceware.org/gdb/current/onlinedocs/gdb/General-Query-Packets.html#index-qGetTLSAddr-packet

We'd probably want to stick this function in MultiThreadOps, since I'd imagine this isn't something single threaded targets would care about, correct?

If I had to guess how the user-facing API would look like, I'd wager it'd be something along the lines of:

pub trait GetTlsAddr: MultiThreadOps {
    ...
    
    fn get_tls_addr(
        &self, 
        thread: Tid,
        offset: <Self::Arch as Arch>::Usize,
        load_module: &[u8], // OS/ABI specific
    ) -> TargetResult<<Self::Arch as Arch>::Usize, Self>;
}

The only open design question is whether or not it'd make sense to try and encode the specific load_module representation as part of the Arch defn, and lift some validation / data-marshalling into gdbstub itself? e.g: instead of a raw &[u8], have load_module: <Self::Arch as Arch>::LoadModule.

This would add some complexity to the implementation, and also make adding the feature an API-breaking change. That is to say, I'd be fine with keeping things simple for now, and potentially updating the API at some point down the line.

Also, as you might be able to tell from the commit history, I am planning on cutting a 0.6 release at some point in the near future. I was hoping to do it by end of June, but life suddenly got in the way, and I haven't found the time to sit back down and push the release over the finish line 😅. That said, it'd be a shame to sit on so many unreleased changes, so hopefully I'll push a release out sometime in the next few weeks...

---

Lastly, as with most gdbstub features, the only way to ensure this feature is implemented correctly would be to test it out + validate that it works in an existing project. Unfortunately, I don't personally have a project that uses TLS, so while I'm more than happy to work with you in implementing this feature, you'll have to help out with testing / validation before we could merge anything in.

[mkroening]

Thanks for the quick reply!

I stumbled upon this while reimplementing GDB support in uhyve, but still have to figure out the details and if it makes sense to have this extension.

A TLS extension might be useful for single threaded targets as well, because single threads could still have TLS, could they not?

I should be able to have a more detailed look at this in the next weeks and would be happy to push this forward and test this, if it makes sense for our project.

[daniel5151]

A TLS extension might be useful for single threaded targets as well, because single threads could still have TLS, could they not?

Yes? But also, I'm not sure if the added complexity to the SingleThreadOps would be worth it.

For context, the whole point of SingleThreadOps is to provide an alternative, stripped-down interface for using gdbstub on simple targets that only have a single thread / core (e.g: embedded systems, single-core emulators, etc...). It'd be unlikely that someone would be using SingleThreadOps alongside TLS. Not impossible, but unlikely.

As such, I think we should focus on implementing this for MultiThreadOps, and implement it in SingleThreadOps if/when the need arises.

I should be able to have a more detailed look at this in the next weeks and would be happy to push this forward and test this, if it makes sense for our project.

Yeah, no pressure! Like I said, I'd be happy to review any PRs sent my way 😄

---

I stumbled upon this while reimplementing GDB support in uhyve, but still have to figure out the details and if it makes sense to have this extension.

FYI, I don't think the current dev/0.6 branch will see too many breaking changes before I push out release 0.6, so if you're doing a fresh integration, I'd consider working off of the current dev/0.6 branch rather than 0.5, thereby saving yourself the trouble up updating once I finally get around to cutting 0.6.

Notably, the new state-machine based API makes it much easier to poll for Ctrl-C interrupts (i.e: you don't need to periodically wake up the gdbstub thread to check if an interrupt has arrived).

[mkroening]

As such, I think we should focus on implementing this for MultiThreadOps, and implement it in SingleThreadOps if/when the need arises.

I see. That makes sense. MultiThreadOps it is then. 😄

Notably, the new state-machine based API makes it much easier to poll for Ctrl-C interrupts (i.e: you don't need to periodically wake up the gdbstub thread to check if an interrupt has arrived).

Awesome! I will look into that. Thanks for your awesome crate, by the way! I already have a WIP integration using 0.5 and it works like a charm!

[mchesser]

Outside of embedded, TLS is still fairly common in single threaded programs due to libc requirements, e.g. errno is defined to be a thread-local, so errno is stored in the TLS region.

[daniel5151]

Ahh, this is a fair point.

In that case, we should also include an implementation for single threaded targets as well. We'd probably want to take a similar approach to SingleRegisterAccess, where we make the trait generic over tid kind.