[lldb-dev] LLDB and gdbserver

[Abid, Hafiz]

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

> -----Original Message-----
> From: Greg Clayton [mailto:gclayton at apple.com]
> Sent: 17 October 2013 17:59
> To: Abid, Hafiz
> Cc: lldb-dev at cs.uiuc.edu
> Subject: Re: [lldb-dev] LLDB and gdbserver
> 
> Comments inlined below.
> 
> On Oct 17, 2013, at 6:11 AM, Abid, Hafiz <Hafiz_Abid at mentor.com> wrote:
> 
> > 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?
> 
> >
> > Regards,
> > Abid
> >
> >> -----Original Message-----
> >> From: Greg Clayton [mailto:gclayton at apple.com]
> >> Sent: 16 October 2013 19:21
> >> To: Abid, Hafiz
> >> Cc: lldb-dev at cs.uiuc.edu
> >> Subject: Re: [lldb-dev] LLDB and gdbserver
> >>
> >> The question is: do we need it? You could just add a key to the
> >> target definition file.
> >>
> >> On Oct 16, 2013, at 10:10 AM, Abid, Hafiz <Hafiz_Abid at mentor.com>
> wrote:
> >>
> >>> 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
> >>>
> >>>
> >>>> -----Original Message-----
> >>>> From: Greg Clayton [mailto:gclayton at apple.com]
> >>>> Sent: 15 October 2013 17:58
> >>>> To: Abid, Hafiz
> >>>> Cc: lldb-dev at cs.uiuc.edu
> >>>> Subject: Re: [lldb-dev] LLDB and gdbserver
> >>>>
> >>>> 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
> >>>> Date: Mon Oct 14 19:14:28 2013
> >>>> 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
> >>>>
> >>>>
> >>>> On Oct 11, 2013, at 6:52 AM, Abid, Hafiz <Hafiz_Abid at mentor.com>
> >> wrote:
> >>>>
> >>>>> Ping. There are 2 patches in this series waiting review.
> >>>>>
> >>>>> Thanks,
> >>>>> Abid
> >>>>>
> >>>>>> -----Original Message-----
> >>>>>> From: lldb-dev-bounces at cs.uiuc.edu
> >>>>>> [mailto:lldb-dev-bounces at cs.uiuc.edu]
> >>>>>> On Behalf Of Abid, Hafiz
> >>>>>> Sent: 09 October 2013 17:33
> >>>>>> To: lldb-dev at cs.uiuc.edu
> >>>>>> Subject: Re: [lldb-dev] LLDB and gdbserver
> >>>>>>
> >>>>>> 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
> >>>>>>
> >>>>>>
> >>>>>>> -----Original Message-----
> >>>>>>> From: lldb-dev-bounces at cs.uiuc.edu
> >>>>>>> [mailto:lldb-dev-bounces at cs.uiuc.edu]
> >>>>>>> On Behalf Of Abid, Hafiz
> >>>>>>> Sent: 04 October 2013 17:45
> >>>>>>> To: Greg Clayton
> >>>>>>> Cc: lldb-dev at cs.uiuc.edu
> >>>>>>> Subject: Re: [lldb-dev] LLDB and gdbserver
> >>>>>>>
> >>>>>>> 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
> >>>>>>>
> >>>>>>>> -----Original Message-----
> >>>>>>>> From: Greg Clayton [mailto:gclayton at apple.com]
> >>>>>>>> Sent: 05 September 2013 19:26
> >>>>>>>> To: Abid, Hafiz
> >>>>>>>> Cc: lldb-dev at cs.uiuc.edu
> >>>>>>>> Subject: Re: [lldb-dev] LLDB and gdbserver
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> On Sep 5, 2013, at 9:16 AM, Abid, Hafiz
> <Hafiz_Abid at mentor.com>
> >>>>>> wrote:
> >>>>>>>>
> >>>>>>>>> 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
> >>>>>
> >>>>> _______________________________________________
> >>>>> lldb-dev mailing list
> >>>>> lldb-dev at cs.uiuc.edu
> >>>>> http://lists.cs.uiuc.edu/mailman/listinfo/lldb-dev
> >>>
> >>> <pc_adjust.patch>
> >
> > <pc_adjust.patch>