[Lldb-commits] [PATCH] fix debugging on linux

[dawn at burble.org]

> Hopefully CanLaunchProcessStoppedAtEntry() makes sense?

Yes, that works.  And "DebugProcess" becomes
"LaunchProcessStoppedAtEntry"?  I like this better than DebugProcess,
but I'm still not crazy about it :).  Linux can launch a process
with it stopped at the entry point, it just doesn't want to "attach"
to it afterwards.

I should backup to the original problem I was trying to solve...  DebugProcess
is in Target/Platform.cpp - a common module shared by allplatforms, yet it
calls "Attach".  When the Linux code launches the process (via fork and exec
and a ptrace call of PTRACE_TRACEME), lldb will already be debugging the
process.  Calling "Attach" at this point will do a ptrace call of PTRACE_ATTACH
on the process that is already being debugged - not good.  

How would you suggest we fix this?  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.

-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