Skip to content

Comments

Add higher level commands to RR#1621

Closed
bgirard wants to merge 1 commit intorr-debugger:masterfrom
bgirard:master
Closed

Add higher level commands to RR#1621
bgirard wants to merge 1 commit intorr-debugger:masterfrom
bgirard:master

Conversation

@bgirard
Copy link
Contributor

@bgirard bgirard commented Jan 16, 2016

This pull request is adding more powerful commands to GDB to build more useful command.

Here's a starting proof of concept:
Imgur

This uses forks gdb-dashboard, which already shows a lot of relevant info (source, stack, threads, disassembly).

I've added RR Events which shows the RR event in the past and in the future. It also shows the previous and upcoming thread switch.

I'm not sure if exposing the low level RR Events is useful to users of RR but it's a step towards being able to expose better data when debugging.

@rocallahan
Copy link
Collaborator

Before doing this I'd like to simplify the way we handle target-specific commands in rr/gdb. We can use the maintenance packet command to send an arbitrary packet to rr, which makes it really easy to send whatever data we want without hacks. We can't get a result back from that command, but we can store the result in the special rr memory location and read it from there.

@bgirard
Copy link
Contributor Author

bgirard commented Jan 24, 2016

https://sourceware.org/gdb/onlinedocs/gdb/Maintenance-Commands.html

Do you mean issuing 'maint packet ' from the rr gdbinit? Hopefully GDB doesn't restrict characters and does all the escaping we need. How would we structure 'text'? Something like this (assuming there's no restricted characters by GDB)?

maint packet rr-cmd://
example:
rr-cmd://checkpoint?arg0=1&arg1=--force

We can look for an obvious prefix 'rr-cmd://' when processing the packet.

We can just have a python command in rr's gdbinit to URI encode without much effort. We'd just need to pull in something to decode the URI in the GdbServer.

@rocallahan
Copy link
Collaborator

Do you mean issuing 'maint packet ' from the rr gdbinit?

From the commands defined in rr's gdbinit, yes.

I don't think we need to use URIs here. I don't know what escaping is needed, if any; probably need to experiment since it's underdocumented.

How about sending packets of the form :rr:<command_name>:<arbitrary_per_command_data>?

@rocallahan
Copy link
Collaborator

Actually, just :<command_name>:<arbitrary_per_command_data> should do.

@rocallahan
Copy link
Collaborator

Also I think we should have a generic DREQ_TARGET_COMMAND command; GdbConnection would just extract the command name and per-command data and pass them as a DREQ_TARGET_COMMAND to GdbServer which then handles everything. Want to keep rr-specific features out of GdbConnection.

@bgirard
Copy link
Contributor Author

bgirard commented Jan 24, 2016

Well let's say we want to support a command with multiple string argument. Then we have to pick an argument delimiter that can't appear in the argument or use some kind of length encoding. At this point why not use URI encoding which is well documented and understood.

Also GDB does argument parsing so it's best to let GDB parse the command so there's no inconsistency. For instance: MyRRCmd "this is arg1" "arg2". If we're receiving user commands as a list of arguments we should keep the argument tokenization and not pass arbitrary data and have RR re-parse it.

@rocallahan
Copy link
Collaborator

I don't want to implement URI encoding and decoding unless we really need it.

Commands that need to pass arbitrary strings or binary blobs could use gdb's binary encoding described here: https://sourceware.org/gdb/onlinedocs/gdb/Overview.html#Binary%20Data (starting "The binary data representation ...").

How about this, then: :<command_name>:<param1>:<param2>...:<paramN> where the params (of course) cannot contain :. If you want to pass arbitrary strings or binary data, encode them using the gdb binary encoding and be sure to escape :, ,#,$and}.

@rocallahan
Copy link
Collaborator

GdbConnection's read_binary_data parses that gdb binary data.

@bgirard
Copy link
Contributor Author

bgirard commented Jan 26, 2016

This should match what we discussed. I went with the two bytes encoding because it's simple. I re-implemented the 'when' commands and made sure they're still passing the test. Once this is merged I'll send another PR for history/info checkpoint.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is no longer used

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually I guess it still is used. But we should replace the checkpoint commands with the new infrastructure, later.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I plan on submitting that soon. I just wanted to make sure we were ok with this before I rebased my changes.

@rocallahan
Copy link
Collaborator

OK, I merged this. I made one change, which was to make the parameter to RR_CMD() a string rather than a bare set of tokens. This makes clang-format happy.

@rocallahan rocallahan closed this Jan 26, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants