LLDB and gdbserver

I was able to make the LLDB work with gdbserver. Running, stopping, source stepping were working ok. I needed to patch 2 areas to make it work. I would like comments on those changes before I can get them ready for submission.

If dynamic register info is not available then GDBRemoteRegisterContext is relying on hardcoded registers for ARM. I have to added similar hard-coded registers for x86_64. Would it make any sense if we keep GDBRemoteRegisterContext for reading/writing the register packets only. The task of translating those packets should be left to some higher level classes. Perhaps something like GDBRemoteRegisterContext_arm, GDBRemoteRegisterContext_x86_64 etc. Or for the time being, hardcoding is considered ok.

It seems that debugserver decrements the pc after stopping on breakpoint. To find the stop reason, code in ProcessGDBRemote::SetThreadStopInfo() checks for breakpoint on pc. But gdbserver does not decrement the pc in this case. I had to duplicate the above check for (pc - 1) and then decrement the pc accordingly. I was wondering if there is some good way to distinguish if I am connected to debugserver or gdbserver. Can I make use of some of the new packets added by LLDB or perhaps add some option in the gdb-remote command?

Regards,
Abid

Awesome Abib, I can't wait to try this on x86 and then adapt it for arm!

S.

I was able to make the LLDB work with gdbserver. Running, stopping, source stepping were working ok. I needed to patch 2 areas to make it work. I would like comments on those changes before I can get them ready for submission.

If dynamic register info is not available then GDBRemoteRegisterContext is relying on hardcoded registers for ARM. I have to added similar hard-coded registers for x86_64. Would it make any sense if we keep GDBRemoteRegisterContext for reading/writing the register packets only. The task of translating those packets should be left to some higher level classes. Perhaps something like GDBRemoteRegisterContext_arm, GDBRemoteRegisterContext_x86_64 etc. Or for the time being, hardcoding is considered ok.

The only hard coded registers should be for ARM as this was needed for legacy iOS support. Any new registers should use a new plugin setting that supplies an XML file that describes the registers. The setting should be something like:

(lldb) settings set plugin.process.gdb-remote.register-definition-file /path/to/registers.xml

The XML register description should supply the same information as the qRegisterInfo packet for all registers. Then the GDBRemoteDynamicRegisterInfo class will need to be able to set itself up using an XML file. Another way to do this would be to supply a python file. We have Python bridging objects available where you could easily make a new python callback where a python dictionary could be returned. Python might also be more useful because you could create classes that know the common register numbers for certain architectures and ABIs...

So the flow would be:
- debugging starts with debugserver
- we stop for the first time and "qRegisterInfo0" packet is sent, and the unsupported response of "$#00" is returned.
- we grab the value of the "plugin.process.gdb-remote.register-definition-file" setting, and if it is set, we parse that file
- else fallback to hard coded registers.

So I would avoid adding anymore hard coded registers to LLDB otherwise we will end up with a mess in LLDB with all the different gdb servers that we can attach to.

It seems that debugserver decrements the pc after stopping on breakpoint. To find the stop reason, code in ProcessGDBRemote::SetThreadStopInfo() checks for breakpoint on pc. But gdbserver does not decrement the pc in this case. I had to duplicate the above check for (pc - 1) and then decrement the pc accordingly. I was wondering if there is some good way to distinguish if I am connected to debugserver or gdbserver. Can I make use of some of the new packets added by LLDB or perhaps add some option in the gdb-remote command?

I would modify the qHostInfo to return a new key/value pair like: "adjusts_breakpoint_pc:1;" or "adjusts_breakpoint_pc:0;" so we know for certain architectures if we need to manually adjust the PC. Then we use a LazyBool instance variable in GDBRemoteCommunicationClient to detect this setting via the qHostInfo packet. If the "adjusts_breakpoint_pc" key/value isn't specified in the qHostInfo packet, or if the qHostInfo packet isn't supported, we should fall back to a setting:

(lldb) settings set plugin.process.gdb-remote.adjust-breakpoint-pc true
(lldb) settings set plugin.process.gdb-remote.adjust-breakpoint-pc false

So with all settings when using GDB server, we try to detect things dynamically (registers and other settings like the adjust breakpoint PC), and if that fails, we fall back to manual settings.

Greg

Hi Greg,

Is there any reason not to move the currently hard-coded ARM register information into an XML file once that support is in place?

I'd like to see some mechanism where we'll attempt to choose a register definition file based on the target architecture, but of course having the ability to override the selected file with a settings variable would be desirable.

-Andy

Hi Greg,

Is there any reason not to move the currently hard-coded ARM register information into an XML file once that support is in place?

We could totally do that.

I'd like to see some mechanism where we'll attempt to choose a register definition file based on the target architecture, but of course having the ability to override the selected file with a settings variable would be desirable.

Yes, we would need to match up the size of the response to the "g" packet along with the target triple to files in a builtin location within the LLDB.framework, and I am not sure where these files would live on linux, but I am sure you can find a location. So maybe we don't need a setting then, just a central place to look for register definition files.

The data could look something like:

<target_definition>
    <triple>arm*-apple-ios</triple>
    <g_packet_size>168</g_packet_size>
    <registers>
        <sets>
            <set id=0>
                <name>General Purpose Registers</name>
                <short_name>gpr</short_name>
            </set>
            <set id=0>
                <name>Floating Point Registers</name>
                <short_name>fpr</short_name>
            </set>
            <set id=0>
                <name>Exception Registers</name>
                <short_name>exc</short_name>
            </set>
        </sets>
        <register_infos>
            <register id=0>
                <name>rax</name>
                <bitsize>64</bitsize>
                <offset>0</offset>
                <encoding>uint</encoding>
                <format>hex</format>
                <set>0</set>
                <compiler_regnum>0</compiler_regnum>
                <dwarf_regnum>0</dwarf_regnum>
                <invalidate-regs>0,15,25,35,39</invalidate-regs>
            </register>
            ....
            <register id=16>
                <name>rip</name>
                <alt_name>pc</alt_name>
                <bitsize>64</bitsize>
                <offset>128</offset>
                <encoding>uint</encoding>
                <format>hex</format>
                <set>0</set>
                <compiler_regnum>16</compiler_regnum>
                <dwarf_regnum>16</dwarf_regnum>
                <generic>pc</generic>
            </register>
        </register_infos>
    </registers>
</target_definition>

Note the "triple" and "g_packet_size" fields that we could use to do matching. The rest is just the same info from the qRegisterInfo packet..

Hi Greg,
I am attaching a patch that implements the 2nd part as discussed below i.e. to adjust the pc after hitting breakpoint. The response of qHostInfo packet is checked for ' adjusts_breakpoint_pc' key. If found, it gives the value by which the pc should be adjusted. In its absence, a new setting ' plugin.process.gdb-remote .adjust-breakpoint-pc' can be used to provide this information. The default of this settings is 0. If a user is connecting to a stub that needs the debugger to adjust the pc (e.g. gdbserver) then user can adjust the value of this settings accordingly.

I tested the new setting and it is working fine. As I don't have a stub that sends qHostInfo with this new key, so that code is un-tested. OK to commit?

Regards,
Abid

adjust_pc.patch (8.2 KB)

Hi All,
The attached patch implements reading register description of the remote target from the xml file. The path of the file is provided through a settings as a first step. We can make lldb search though xml files and find correct one based on arch and g packet size on top of this patch. After this patch and my previous patch (regarding adjusting pc when breakpoint is hit) go in, we can connect to gdbserver like shown below. I am also attaching an xml file that I used for testing on Ubuntu 12.04 on x86-64.

I was not aware what will be the best way to check for the presence of libXML. I am using an already present macro 'CLANG_HAVE_LIBXML'.

How does it all look?

Regards,
Abid

lldb ~/demos/act
Current executable set to '/home/abidh/demos/act' (x86_64).
(lldb) settings set -- plugin.process.gdb-remote.register-definition-file /home/abidh/work/llvm/src/tools/lldb/xml/x86_64_gdbserver.xml
(lldb) setting set -- plugin.process.gdb-remote.adjust-breakpoint-pc 1
(lldb) gdb-remote 10000

x86_64_gdbserver.xml (11.6 KB)

xml_register_desc.patch (16.2 KB)

Ping. There are 2 patches in this series waiting review.

Thanks,
Abid

The patch looks good. The only thing I worry about is someone setting this adjust PC value to something like -1, and then forgetting about it. Then they attach to a program with a different architecture and the PC gets adjusted. I am wondering if this setting should be a dictionary that takes target triple regular expresssions as the key and the value is the adjustment amount? This would stop random PC adjustments from happening on different architectures.

Greg

Looks good.

Greg

XML patch looks good.

The breakpoint adjust patch, I had these comments:

The patch looks good. The only thing I worry about is someone setting this adjust PC value to something like -1, and then forgetting about it. Then they attach to a program with a different architecture and the PC gets adjusted. I am wondering if this setting should be a dictionary that takes target triple regular expresssions as the key and the value is the adjustment amount? This would stop random PC adjustments from happening on different architectures.

The XML patch is no longer needed. I checked in a similar way to do this in python with revision 192646. The main reason is we already had the ability to pass an python dictionary through the script bridging API and parse the dictionary on the other side. The python is easier to write, it can use code flow to construct the register context programmatically, and it can use the LLDB defines and enumeration values to avoid having to make up string values for formats, encodings and everything else. We also already had code that parsed a register definition from a dictionary that was used in the OperatingSystemPython plug-in.

The details on the checkin are:

Author: gclayton
New Revision: 192646

URL: http://llvm.org/viewvc/llvm-project?rev=192646&view=rev
Log:
<rdar://problem/14972424>

When debugging with the GDB remote in LLDB, LLDB uses special packets to discover the
registers on the remote server. When those packets aren't supported, LLDB doesn't
know what the registers look like. This checkin implements a setting that can be used
to specify a python file that contains the registers definitions. The setting is:

(lldb) settings set plugin.process.gdb-remote.target-definition-file /path/to/module.py

Inside module there should be a function:

def get_dynamic_setting(target, setting_name):

This dynamic setting function is handed the "target" which is a SBTarget, and the
"setting_name", which is the name of the dynamic setting to retrieve. For the GDB
remote target definition the setting name is 'gdb-server-target-definition'. The
return value is a dictionary that follows the same format as the OperatingSystem
plugins follow. I have checked in an example file that implements the x86_64 GDB
register set for people to see:

   examples/python/x86_64_target_definition.py

This allows LLDB to debug to any archticture that is support and allows users to
define the registers contexts when the discovery packets (qRegisterInfo, qHostInfo)
are not supported by the remote GDB server.

A few benefits of doing this in Python:
1 - The dynamic register context was already supported in the OperatingSystem plug-in
2 - Register contexts can use all of the LLDB enumerations and definitions for things
   like lldb::Format, lldb::Encoding, generic register numbers, invalid registers
   numbers, etc.
3 - The code that generates the register context can use the program to calculate the
   register context contents (like offsets, register numbers, and more)
4 - True dynamic detection could be used where variables and types could be read from
   the target program itself in order to determine which registers are available since
   the target is passed into the python function.

This is designed to be used instead of XML since it is more dynamic and code flow and
functions can be used to make the dictionary.

Added:
   lldb/trunk/examples/python/x86_64_target_definition.py
Modified:
   lldb/trunk/include/lldb/Interpreter/ScriptInterpreter.h
   lldb/trunk/include/lldb/Interpreter/ScriptInterpreterPython.h
   lldb/trunk/scripts/Python/python-wrapper.swig
   lldb/trunk/source/Interpreter/ScriptInterpreterPython.cpp
   lldb/trunk/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
   lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
   lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
   lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h
   lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
   lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h

Greg,
Thanks for doing python register stuff. It works great.

I have also modified the patch to adjust the pc after hitting breakpoint. It can now take target triple and pc adjust value as key value pairs. Multiple pairs can be provided separated by space.
setting set -- plugin.process.gdb-remote.adjust-breakpoint-pc x86_64-*-linux:1

Tested by connecting with gdbserver on Linux and it works fine. Ok to commit?

Regards,
Abid

pc_adjust.patch (11.9 KB)

The question is: do we need it? You could just add a key to the target definition file.

My impression was that target definition file was just for describing the target registers. But if we are adding connection specific information in it then it makes sense to add this information there too. The attached patch adds a new target definition file for Linux gdbserver. It is quite similar to macosx file although there are a few differences. Some of them are fixes that I will incorporate to macosx file too. There are related changes in the ProcessGDBRemote.cpp. Tested with gdbserver on Linux and it works fine. Ok to commit?

Regards,
Abid

pc_adjust.patch (22.9 KB)

Comments inlined below.

My impression was that target definition file was just for describing the target registers. But if we are adding connection specific information in it then it makes sense to add this information there too.

Yes, this is why I renamed it to "target-definition-file" instead of "register-definition-file". Any special target wide settings that are required for debugging a target that we would normally use special LLDB packets for (qRegisterInfo, qHostInfo, etc) can be put into this file to make it more useful and self sufficient (don't need a combo of "settings set" variables along with the "target-definition-file" content).

The attached patch adds a new target definition file for Linux gdbserver. It is quite similar to macosx file although there are a few differences. Some of them are fixes that I will incorporate to macosx file too.

What fixes are needed on the macosx one?

There are related changes in the ProcessGDBRemote.cpp. Tested with gdbserver on Linux and it works fine. Ok to commit?

Looks good. The only things I can see that we might want to change are:

- should we rename "adjust-breakpoint-pc" to "breakpoint-pc-offset"?
- Should we make the adjust pc value signed? I don't know of any targets that would need to adjust their PC values forward, but if this value is signed, we might want the value to be set to "-1" for linux?

Thanks for the review.

What fixes are needed on the macosx one?

get_reg_num has a duplicate copy and offset value is incremented for registers which are part of another register. So in the end, we assign a wrong value to 'g-packet-size' key.

Looks good. The only things I can see that we might want to change are:

- should we rename "adjust-breakpoint-pc" to "breakpoint-pc-offset"?
- Should we make the adjust pc value signed? I don't know of any targets
that would need to adjust their PC values forward, but if this value is signed,
we might want the value to be set to "-1" for linux?

I renamed the key. It is also treated as signed now and -1 value is being used for x86_64 on Linux.
Committed after making these changes.

Regards,
Abid