Comments inlined below. On Oct 17, 2013, at 6:11 AM, Abid, Hafiz <[email protected]> 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:[email protected]] >> Sent: 16 October 2013 19:21 >> To: Abid, Hafiz >> Cc: [email protected] >> 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 <[email protected]> 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:[email protected]] >>>> Sent: 15 October 2013 17:58 >>>> To: Abid, Hafiz >>>> Cc: [email protected] >>>> 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 <[email protected]> >> wrote: >>>> >>>>> Ping. There are 2 patches in this series waiting review. >>>>> >>>>> Thanks, >>>>> Abid >>>>> >>>>>> -----Original Message----- >>>>>> From: [email protected] >>>>>> [mailto:[email protected]] >>>>>> On Behalf Of Abid, Hafiz >>>>>> Sent: 09 October 2013 17:33 >>>>>> To: [email protected] >>>>>> 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: [email protected] >>>>>>> [mailto:[email protected]] >>>>>>> On Behalf Of Abid, Hafiz >>>>>>> Sent: 04 October 2013 17:45 >>>>>>> To: Greg Clayton >>>>>>> Cc: [email protected] >>>>>>> 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:[email protected]] >>>>>>>> Sent: 05 September 2013 19:26 >>>>>>>> To: Abid, Hafiz >>>>>>>> Cc: [email protected] >>>>>>>> Subject: Re: [lldb-dev] LLDB and gdbserver >>>>>>>> >>>>>>>> >>>>>>>> On Sep 5, 2013, at 9:16 AM, Abid, Hafiz <[email protected]> >>>>>> 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 >>>>> [email protected] >>>>> http://lists.cs.uiuc.edu/mailman/listinfo/lldb-dev >>> >>> <pc_adjust.patch> > > <pc_adjust.patch> _______________________________________________ lldb-dev mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/lldb-dev
