RFC: siginfo reading/writing support

Hello,

TL;DR: I'd like to implement at least partial support for
reading/writing siginfo via LLDB. I can't think of a better approach
than copying the GDB's idea of "magical" $_siginfo variable that works
through the expression evaluator. I'd like to know your opinion/ideas.

POSIX defines a siginfo_t structure that is used to pass additional
signal information -- such as more detailed signal code, faulting memory
address in case of SIGSEGV or PID of the child process in case of
SIGCHLD. LLDB already uses ptrace(2) to obtain this information and use
it internally but it doesn't expose it to the user.

The GDB Remote Serial protocol provides the ability to read/write
siginfo via qXfer:siginfo:... packets [1]. GDB exposes this information
to the user via a special $_siginfo variable [2].

A few things to note:

1. Some targets (e.g. Linux, NetBSD) support overwriting siginfo, some
(e.g. FreeBSD) only reading.

2. Siginfo is generally associated with a single thread, so the packets
should be combined with respective thread selection (Hg or Hc?).

3. The exact type of siginfo_t differs per platform (POSIX specifies
a minimal subset).

My rough idea right now is to follow GDB here. While using "$_siginfo"
may seem hacky, it has the nice advantage that it can easily support all
different siginfo_t structures used by various platforms.

The plan would be to:

1. Implement the qXfer:siginfo:... packets in lldb-server, and add tests
to them.

2. Implement support for "$_siginfo" in the client (I suppose this means
hacking on expression evaluator).

3. (Optionally) implement hardcoded siginfo_t definitions for common
platforms to make things work without debug info.

WDYT?

[1]
https://www.sourceware.org/gdb/onlinedocs/gdb/General-Query-Packets.html#qXfer-siginfo-read
[2] https://sourceware.org/gdb/current/onlinedocs/gdb.html#Signals

You should use Hg for this instead of Hc. Hc is used for step/continue, while Hg is used for everything else.

Thanks for the explanation.

I would not do this with the expression parser.

First off, the expression parser doesn’t know how to do anything JIT code that will run directly in the target. So if:

(lldb) expr $signinfo.some_field = 10

doesn’t resolve to some $siginfo structure in real memory with a real type such that clang can calculate the offset of the field “some_field” and write to it to make the change, then this wouldn’t be a natural fit in the current expression parser. I’m guessing this is not the case, since you fetch this field through ptrace calls in the stub.

And the expression parser is enough of a beast already that we don’t want to add complexity to it without good reason.

We also don’t have any other instances of lldb injected $variables that we use for various purposes. I’m not in favor of introducing them as they end up being pretty undiscoverable….

Why not something like:

(lldb) platform signinfo read [-field field_name]

Without the field name it would print the full siginfo, or you can list fields one by one with the —field argument.

And then make the write a raw command like:

(lldb) platform signinfo write -field name expression

The platform is a natural place for this, it is the agent that knows about all the details of the system your target is running on, so it would know what access you have to siginfo for the target system.

Having the argument write use expressions to produce the new value for the field would get you most of the value of introducing a virtual variable into the expression parser, since:

(lldb) pl si w -f some_field <complex expression>

Is the same as you would get with the proposed $siginfo:

(lldb) expr $siginfo.some_field = <complex expression>

You could also implement the write command as a raw command like:

(lldb)platform siginfo write —field some_field <complex expression>

Which has the up side that people wouldn’t need to quote their expressions, but the down side that you could only change one field at a time.

This would also mean “apropos siginfo” would turn up the commands, as would a casual scan through the command tree. So the feature would be pretty discoverable.

The only things this would make inconvenient are if you wanted to pass the value of a signinfo field to some function call or do something like:

$signinfo.some_field += 5

These don’t seem very common operations, and if you needed you could always do this with scripting, since the result from “platform siginfo read -field name” would be the value, so you could write a little script to grab the value and insert it into the desired expression and run that.

One of the advantages of having a command tree is has a few top level nodes and organizes the commands under them is that adding new commands doesn’t clutter up the top level of the command tree, and so you should feel free to add new commands where they fit into the hierarchy rather than trying to slide them into some other command, like using the expression parser.

Jim

This sentence makes no sense, it was a remnant of a previous draft, which included the option to do:

(lldb) platform write —field name —value expression —field other_name —value other_expression

But that would require people to quote their expressions to get them past the command parser, which seems more annoying than having to set fields one by one would.

Jim

I kinda like the cleanliness (of the design, not the implementation) of a $siginfo variable, but you're right that implementing it would be tricky (I guess we'd have to write the struct info the process memory somewhere and then read it back when the expression completes).

I don't expect that users will frequently want to modify the siginfo structure. I think the typical use case would be to inspect the struct fields (maybe in a script -- we have one user wanting to do that) to understand more about the nature of the stop/crash.

With that in mind, I don't have a problem with a separate command, but I don't think that the "platform" subtree is a good fit for this. I mean, I am sure the (internal) Platform class will be involved in interpreting the data, but all of the platform _commands_ have something to do with the system as a whole (moving files around, listing processes, etc.) and not a specific process. I think this would belong under the "thread" subtree, since the signal is tied to a specific thread.

Due to the scripting use case, I am also interested in being able to inspect the siginfo struct through the SB API -- the expression approach would (kinda) make that possible, while a brand new command doesn't (without extra work). So, I started thinking whether this be exposed there. We already kinda expose the si_signo field via GetStopReasonDataAtIndex(0) (and it even happens to be the first siginfo field), but I don't think we would want to expose all fields in that manner.

This then lead me to SBThread::GetStopReasonExtendedInfoAsJSON. What is this meant to contain? Could we put the signal info there. If yes, then the natural command-line way of retrieving this would be the "thread info" command, and we would need to add any new commands.

This wouldn't solve the problem of writing to the siginfo struct, but I am not sure if this is a use case Michał is actually trying to solve right now (?) If it is then, maybe this could be done through a separate command, as we currently lack the ability to resume a process/thread with a specific signal ("process signal" does something slightly different). It could either be brand new command, or integrated into the existing process/thread continue commands. (thread continue --signal SIGFOO => "continue with SIGFOO"; thread continue --siginfo $47 => continue with siginfo in $47 ???)

pl

I kinda like the cleanliness (of the design, not the implementation) of a $siginfo variable, but you're right that implementing it would be tricky (I guess we'd have to write the struct info the process memory somewhere and then read it back when the expression completes).

I don't expect that users will frequently want to modify the siginfo structure. I think the typical use case would be to inspect the struct fields (maybe in a script -- we have one user wanting to do that) to understand more about the nature of the stop/crash.

With that in mind, I don't have a problem with a separate command, but I don't think that the "platform" subtree is a good fit for this. I mean, I am sure the (internal) Platform class will be involved in interpreting the data, but all of the platform _commands_ have something to do with the system as a whole (moving files around, listing processes, etc.) and not a specific process. I think this would belong under the "thread" subtree, since the signal is tied to a specific thread.

Platform seemed appropriate to me because this is a platform specific feature; some platforms don’t support siginfo at all…. But I’m fine with thread too.

Due to the scripting use case, I am also interested in being able to inspect the siginfo struct through the SB API -- the expression approach would (kinda) make that possible, while a brand new command doesn't (without extra work). So, I started thinking whether this be exposed there. We already kinda expose the si_signo field via GetStopReasonDataAtIndex(0) (and it even happens to be the first siginfo field), but I don't think we would want to expose all fields in that manner.

Why not something like:

SBValue
SBThread::GetSiginfo();

That returns an SBValue with the siginfo type and the data filled in from the gdb-remote packet. If the platform didn’t support this you’d just get an SBValue with the error set saying “not supported” or whatever.

If you have all the types of the members to hand it’s easy to cons up an SBValue from the data you got from the stub. An SBValue is exactly what you’d get back from the expression parser anyway, so from the client’s perspective this would be just as good. And printing the SBValue and doing logic on its members are all well supported.

If we can’t always get our hands on the siginfo type, we will have to cons that type up by hand. But we would have had to do that if we were implementing this feature in the expression parser anyway, and we already hand-make types to hand out in SBValues for a bunch of the synthetic child providers already, so that’s a well trodden path.

You could even make a ValueObjectSiginfo to back the SBValue you hand out which implements “SetValueFromCString” through the gdb-remote protocol interface, so writing back to the siginfo through this interface would be natural.

Jim

Yeah, writing is not very important to me right now. I think it's
rather uncommon for people to override siginfo.

Well, it all makes sense to me. It should also make the implementation
somewhat easier, as I can focus on getting siginfo_t parser with unit
tests first, and then work on the additional commands.

Could you point me to some example I could base my code on? :wink:

You are really going to make a lldb_private::CompilerType, since that’s what backs the Type & ultimately the SBTypes. There’s a self-contained example where we make a CompilerType to represent the pairs in the synthetic child provider for NSDictionaries in the function GetLLDBNSPairType in NSDictionary.cpp. And then you can follow the use of that function to see how that gets turned into a Type.

Also, the whole job of the DWARF parser is to make up CompilerTypes out of information from external sources, so if you need other examples for how to add elements to a CompilerType the DWARF parser is replete with them.

Jim

The other idea would be to allow the Platform subclasses to be able to fill in some fixed variable names when asked.

So if the user typed either:

(lldb) frame variable $platform.siginfo
(lldb) expression $platform.siginfo

We would have the name lookup mechanism check with the current platform if the string starts with "$platform" for the current target and ask it _first_ if it knows anything about this name. If the linux platform recognizes it, it can create one however it wants to and return the variable all filled in. We could use this same mechanism for other things like the uncaught exceptions for C++ (GDB has a way of vending info on these exceptions in certain cases too.