[Lldb-commits] [PATCH] fix debugging on linux
Greg Clayton
gclayton at apple.com
Mon Nov 28 20:38:04 PST 2011
On Nov 28, 2011, at 7:28 PM, dawn at burble.org wrote:
> On Mon, Nov 28, 2011 at 05:58:05PM -0800, Greg Clayton wrote:
>>
>> Can you have the linux "Host::LaunchProcess()" do the fork, ptrace me, exec, then do the attach? Or must linux have a debug process attached right from the start?
>
> No, both Linux and Windows (and I think BSD as well) will not allow you
> to attach to a process that you're already debugging.
I am not asking for that.
> They want to have a debug process attached right from the start. They do support
> attaching to an existing process that's already running, but that's a
> different problem - here we want to debug the process form the start.
>
> So I think you see the issue now?
I see the issue, but your fix in the platform isn't going to help. It works for local debugging only, but not for remote debugging, which is the whole point of going through the platform in the first place. Your current fix will always try and launch the program using the default process plug-in on the current machine.
What is intended for the platform is:
(lldb) platform select remote-macosx
(lldb) platform connect tcp://some.machine.com:1234
(lldb) target create /sdk/linux2.3/bin/ls
(lldb) run
now when you do the launching, the launching should happen through the platform on the remote host, but in your patch:
lldb::ProcessSP
Platform::DebugProcess (ProcessLaunchInfo &launch_info,
Debugger &debugger,
Target *target, // Can be NULL, if NULL create a new target, else use existing one
Listener &listener,
Error &error)
{
ProcessSP process_sp;
// Make sure we stop at the entry point
launch_info.GetFlags ().Set (eLaunchFlagDebug);
if (!target->GetPlatform()->CanLaunchProcessStoppedAtEntry())
{
const char *plugin_name = launch_info.GetProcessPluginName();
process_sp = target->CreateProcess (listener, plugin_name).get();
error = process_sp->Launch (launch_info);
}
else
....
this will just launch it locally. A fix should actually be happening in the Platform::LaunchProcess:
Error
Platform::LaunchProcess (ProcessLaunchInfo &launch_info)
{
Error error;
// Take care of the host case so that each subclass can just
// call this function to get the host functionality.
if (IsHost())
error = Host::LaunchProcess (launch_info);
else
error.SetErrorString ("base lldb_private::Platform class can't launch remote processes");
return error;
}
But here LaunchProcess isn't returning a ProcessSP of any kind since this function is also used for just spawning processes.
Do you see where I am going with the platform thing here? We can't mix in any specific process class to do the launching.
I know there is work to be done on the platforms still to work out how all of this happens. So for now, I think the code is good
>
>>> How would you suggest we fix this?
>>
>> With my fix that is already checked in (the fix to CommandObjectProcessLaunch that first checks to see if the platform can start a process stopped for debugging and if it can't, it will just use the current process plug-in to do the launch directly and skip the platform launch for debug), the linux process plug-in will be used to do the launching and avoid this issue altogether, right?
>
> Yes, but it doesn't fit in with how the process control code was
> intended to be used - we're skipping the call to DebugProcess just to
> avoid a call to Attach? And other platforms will have to go this route
> as well?
No, the explanation above should clarify why.
>
>>> Move the call to Attach into the MaxOSX
>>> LaunchProcess code? Or have a flag like CanLaunchViaAttach so we'll know to
>>> skip the call to Attach? I went with the latter in my patch - that seemed the
>>> least intrusive yet generic fix.
>>
>> Again, I don't understand the can launch via attach. You should be able to launch a program on linux and then attach to it without a debugger needing to be there from the start, you will just miss many instructions as the process makes progress until you attach.
>
> Exactly - I think you see the problem now :)
So my vote is to leave the code changes as they exist in the top of tree and let the process plug-in do the launching.
>
> -Dawn
>
>
>>> On Mon, Nov 28, 2011 at 01:48:35PM -0800, Greg Clayton wrote:
>>>>
>>>> On Nov 28, 2011, at 11:59 AM, dawn at burble.org wrote:
>>>>
>>>>> I find the name "CanDebugProcess" to be misleading. The issue isn't
>>>>> whether a process can be debugged on Linux (because it obviously can),
>>>>> it's that it can't be spawned in a suspended state and then attached
>>>>> to. AFAIK, Linux doesn't have anything like the MacOSX
>>>>> "POSIX_SPAWN_START_SUSPENDED" flag for posix_spawnattr_setflags() that
>>>>> MacOSX has.
>>>>
>>>> Yes, this flag is darwin only.
>>>>
>>>>> I chose the name "CanLaunchViaAttach" for this, which I think better
>>>>> describes the problem. Ok to rename "CanDebugProcess" to
>>>>> "CanLaunchViaAttach"?
>>>>
>>>> I am not too fond of the CanLaunchViaAttach because launching a program using a platform doesn't imply that you want to debug or attach to it. Also it doesn't make sense how do you launch via attach? First you launch, then you attach. The attach doesn't have anything to do with launching. You might just want to spawn a server program on the remote platform which you don't want to debug. We are asking the platform (not the process plug-in, but the platform) if it can do something, in this case: can the platform launch a process that can be stopped for debugging. Currently the linux platform can't (though I am sure we can make it work somehow), only the native linux _process_ plug-in can. You can't use the ProcessLinux plug-in to implement stuff functionality for the platform, the platform itself should be doing the work. This is preparing for better remote debugging support where we can launch new remote processes without having to involve any of the process plug-ins.
>>>>
>>>> The attach part shouldn't have anything to do with this either because you might want to spawn a new process that is stopped, and then resume it at a controlled time later. The "POSIX_SPAWN_START_SUSPENDED" flag in darwin will have the process halted at the entry point with a SIGSTOP, and it can be resumed later.
>>>>
>>>> How about "bool Platform::CanLaunchProcessStoppedAtEntry()"?
>>>>
>>>> Platforms are far from complete and we can modify them and grow their abilities over time. Again, the vision is that platforms will eventually launch (possibly for debug), attach to processes, list processes, upload/download and more. They should provide a nice connection to a local or remote machine so that debugging to them is as easy as debugging on your local machine. That is the vision at least, and I know we have some work ahead of us to get there.
>>>>
>>>>
>>>> Greg
>>>>
>>>>
>>>>
>>>>> Likewise, since you moved the fixes I had made to
>>>>> DebugProcess() into CommandObjectProcessLaunch(), please also rename
>>>>> "DebugProcess" to "LaunchViaAttach".
>>>>
>>>> Hopefully CanLaunchProcessStoppedAtEntry() makes sense?
>>>>
>>>>> Thank you!
>>>>> -Dawn
>>>>>
>>>>>
>>>>> On Sun, Nov 27, 2011 at 06:03:03PM -0800, Greg Clayton wrote:
>>>>>>
>>>>>> On Nov 23, 2011, at 11:31 PM, dawn at burble.org wrote:
>>>>>>
>>>>>>> This patch was created using clang/llvm rev 144569, lldb rev 144919 but has
>>>>>>> been updated to clang/llvm rev 144982, lldb rev 145118. It gets debugging on
>>>>>>> Linux back to the state it was in in llvm/clang rev 143631, lldb rev 143774.
>>>>>>>
>>>>>>> Debugging on Linux broke when the assumption was made that a process could be
>>>>>>> launched by spawning it in a suspended state and then attaching to it. That
>>>>>>> support does not exist on Linux. To work around this, I added a new method
>>>>>>> "CanLaunchViaAttach" which tests whether the platform supports being launched
>>>>>>> in this way. I wanted to add a flag to ProcessLaunchInfo, but could not find
>>>>>>> an easy way to set it from within the Linux plugins.
>>>>>>>
>>>>>>> Commit message:
>>>>>>>
>>>>>>> Fix debugging on Linux.
>>>>>>> Implement remote vs. host platform code as was done with PlatformDarwin.
>>>>>>> Platform::CanLaunchViaAttach(): new: return true if the process can be launched
>>>>>>> via spawn and attach. Default is true. Override in PlatformLinux to false.
>>>>>>> Platform::DebugProcess(): if (!target->GetPlatform()->CanLaunchViaAttach())
>>>>>>> then launch the process via CreateProcess and Launch.
>>>>>>>
>>>>>>> Please review and commit if acceptable.
>>>>>>
>>>>>> I checked in an alternate fix:
>>>>>>
>>>>>> % svn commit
>>>>>> Sending include/lldb/Core/Module.h
>>>>>> Sending include/lldb/Symbol/SymbolFile.h
>>>>>> Sending include/lldb/Target/Platform.h
>>>>>> Sending source/Commands/CommandObjectProcess.cpp
>>>>>> Sending source/Core/Module.cpp
>>>>>> Sending source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.cpp
>>>>>> Sending source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
>>>>>> Sending source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
>>>>>> Sending source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
>>>>>> Sending source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
>>>>>> Sending source/Symbol/SymbolFile.cpp
>>>>>> Transmitting file data ...........
>>>>>> Committed revision 145219.
>>>>>>
>>>>>> % svn commit
>>>>>> Sending Platform/Linux/PlatformLinux.h
>>>>>> Transmitting file data .
>>>>>> Committed revision 145221.
>>>>>>
>>>>>>
>>>>>> The first one contains the necessary changes to CommandObjectProcess.cpp and to include/lldb/Target/Platform.h that allow a platform to say:
>>>>>>
>>>>>>
>>>>>> virtual bool
>>>>>> CanDebugProcess ()
>>>>>> {
>>>>>> return true;
>>>>>> }
>>>>>>
>>>>>> This keeps the code out of the platform since when a platform launches a process, it shouldn't be tied to a process plug-in as in the patch you submitted.
>>>>>>
>>>>>> Here is the vision of platforms:
>>>>>>
>>>>>> - They should be able to list all of the processes on the platform. This way you can connect to your local machine and list the processes, or connect to a remote platform, and if the remote platform supports it, it can list the processes that are there.
>>>>>> - Platforms should support launching processes. Not always for debugging, but sometimes just to spawn a child process. On the localhost, this means just using the Host::LaunchProcess() functionality. This way, we just need to implement a single copy of the process launching code. When you create a remote platform, you should be able to link against the native linux or darwin launching code and then we don't end up with multiple copies of the launching code like we have now. You make a good point in that not all platforms will be able to support launching with a program stopped for debugging, so we now ask the platform up in the CommandObjectProcessLaunch class before we launch the process. This keeps the platform clean, yet also lets us fall back to just launching the process through the built in process plug-in.
>>>>>> - Platforms should support attaching to a remote process. Combined with being able to launch for debug, this allows debugging as we have it on MacOSX. If we do this while remotely connected to a remote darwin machine, we now have the ability to talk to a remote computer and launch, attach, list and do a lot more
>>>>>> - Platforms should know if there are files locally on the current machine in an SDK or PDK or some over developer kit/cross compiling sysroot, etc to allow locating files in terms of the path you would us on the remote machine. For example when debugging on a remote device, if the are asked to find the file for "/bin/ls", this might locally be located in developer kit directory such as
>>>>>> "/path/to/old/linux/sysroot/bin/ls". This allows you to debug remote machines, or event mount the root file system on a mountpoint so you have access to the files so you can debug.
>>>>>> - Platforms should be able to upload/download files so you could install a new binary and run it, or upload a shared library so we can locally cache it for faster debugging.
>>>>>>
>>>>>> We are in the process of slowly makine these Platforms a reality, so keep the questions coming if anyone has any.
>>>>>>
>>>>>> So in short: I made the fix you requested, but in a different way. If you can rework your patch around what is in top of tree, then we can get the rest of it submitted.
>>>>>>
>>>>>> Greg Clayton
>>>>>>
>>>>>>> Thanks!
>>>>>>> -Dawn
>>>>>>> <llvmR144982_lldbR145118.patch>_______________________________________________
>>>>>>> lldb-commits mailing list
>>>>>>> lldb-commits at cs.uiuc.edu
>>>>>>> http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
More information about the lldb-commits
mailing list