From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 3772 invoked by alias); 29 Dec 2007 22:44:43 -0000 Received: (qmail 3764 invoked by uid 22791); 29 Dec 2007 22:44:42 -0000 X-Spam-Check-By: sourceware.org Received: from fk-out-0910.google.com (HELO fk-out-0910.google.com) (209.85.128.187) by sourceware.org (qpsmtpd/0.31) with ESMTP; Sat, 29 Dec 2007 22:44:29 +0000 Received: by fk-out-0910.google.com with SMTP id 26so5249430fkx.8 for ; Sat, 29 Dec 2007 14:44:26 -0800 (PST) Received: by 10.82.155.10 with SMTP id c10mr11929263bue.39.1198968266469; Sat, 29 Dec 2007 14:44:26 -0800 (PST) Received: from ?78.130.87.130? ( [78.130.87.130]) by mx.google.com with ESMTPS id c14sm13421542nfi.6.2007.12.29.14.44.24 (version=TLSv1/SSLv3 cipher=RC4-MD5); Sat, 29 Dec 2007 14:44:26 -0800 (PST) Message-ID: <4776CDA3.90204@portugalmail.pt> Date: Sat, 29 Dec 2007 22:46:00 -0000 From: Pedro Alves User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; pt-BR; rv:1.8.1.9) Gecko/20071031 Thunderbird/2.0.0.9 Mnenhy/0.7.5.0 MIME-Version: 1.0 To: gdb-patches@sourceware.org Subject: Re: PR/2386 [2/2]: MinGW attach to process without an exec file References: <47744F9C.8040604@portugalmail.pt> <20071228013457.GB7602@ednor.casa.cgf.cx> <477579E0.5010809@portugalmail.pt> <20071229035030.GC30002@adacore.com> <47764306.4060903@portugalmail.pt> <20071229180146.GC24999@ednor.casa.cgf.cx> In-Reply-To: <20071229180146.GC24999@ednor.casa.cgf.cx> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2007-12/txt/msg00465.txt.bz2 Christopher Faylor wrote: > > On Fri, 28 Dec 2007 22:34:08 +0000 , Pedro Alves wrote: >> Not useful in this case, as the pid gdb holds is the native Windows pid. >> Hence current usage of cygwin_internal (CW_GETPINFO, ...). > > I do find this less intrusive but I don't understand the above comment. > Did you actually try using /proc and found it wanting? Finding a cygwin > exename should be as simple as just doing a readlink > ("/proc//exe"). I just tried this on a top-level process created > by cygwin and on the pid returned by typing "sleep 300&" in bash. The > sleep process does have two pids associated with it due to the oddities > of the way that cygwin emulates exec but "ls -l /proc//exe" showed > "/bin/sleep.exe" in each case. > From infcmd.c:attach_command : /* * If no exec file is yet known, try to determine it from the * process itself. */ exec_file = (char *) get_exec_file (0); if (!exec_file) { exec_file = target_pid_to_exec_file (PIDGET (inferior_ptid)); if (exec_file) { So, the pid that is passed is what's stored in inferior_ptid. It happens, that the win32-nat.c implementation stuff *thread* ids in ptid_t structs, so PIDGET (inferior_ptid) extracts the current inferior thread id -- not very useful for target_pid_to_exec_file, hence the following comment in the current version of win32_pid_to_exec_file: /* Try to find the process path using the Cygwin internal process list pid isn't a valid pid, unfortunately. Use current_event.dwProcessId instead. */ (That's the I-have-a-patch-to-clean-that-up part. But even then, PIDGET(inferior_ptid) will be the winpid. We have room for it in the ptid_t, though, in the form of a lwp. Note, *that* patch will be invasive :-) ) What's stored in current_event.dwProcessId is a Windows PID, not a Cygwin pid. In that case, /proc//exe isn't helpful, unless we can query Cygwin for the cygpid associated with this pid, or we look though all the /proc/s looking at a matching `cat /proc//winpid`. >sleep 300& [3] 3848 [2] Exit 1 sleep >cat /proc/3848/winpid 1788 Is there a reverse of CW_CYGWIN_PID_TO_WINPID ? Even if the user specifies a cygwin pid to gdb --pid , win32_attach does this: /* Attach to process PID, then initialize for debugging it. */ static void win32_attach (char *args, int from_tty) { BOOL ok; DWORD pid; pid = strtoul (args, 0, 0); /* Windows pid */ ok = DebugActiveProcess (pid); if (!ok) { /* Try fall back to Cygwin pid */ pid = cygwin_internal (CW_CYGWIN_PID_TO_WINPID, pid); if (pid > 0) ok = DebugActiveProcess (pid); } We're calling win32 debug functions, so we internally always care for the Windows pid. Please do note that I'm not writting that part of the patch. That Cygwin specific code is already there. That is not to say that I won't change it if you tell me how to get the winpid from a cygwin pid. > For the rest of the code, have you investigated > "NtQuerySystemInformation"? I don't know if it contains everything > needed but, despite what the Microsoft site says, it seems to have had a > stable interface for years and I don't see how Microsoft could change > the functionality without breaking lots of stuff. I think it would be > better to adopt to probably nonexistent future API changes than it is > to rely on a DLL which may not be there. I've always had a problem with > the psapi.h stuff for just the reasons that you mentioned. > I'm not sure if psapi.dll is that problematic. It should be quite widespread by now. Googling around shows that IE7 comes with it, for instance. In fact, google shows a lot of people with some app that doesn't load, because they have several versions of psapi.dll on the system, and the wrong version is being loaded. Following the lead of a lot of apps, we could *require* it, and instruct the user to get it, if it isn't found. For kickers, I took a quick peek at Wine's implementation of psapi.dll, and it was looking directly at PEB structures and similar stuff -- those probably are even less stable. There's a new API in Vista that gives us what we want, but I forgot it's name, and can't re-find it... argh... In this particular case, I'm now inclined to just go for psapi.dll and be done with it. We could load it in _initialize_win32_nat and warn if we're on NT, and it wasn't found ... > We use NtQuerySystemInformation in cygwin to return the list of > processes. See pinfo.cc. > Not fully useable, I did try it. The ProcessName field in the SYSTEM_PROCESSES structure doesn't have a path, only the filename ... I also looked into NtQueryInformationFile with FileNameInformation, but that returns a file path starting from the root of the drive, that is: \cygwin\bin\cat.exe, instead of \Device\Partition\cygwin\bin\cat.exe. I couldn't find a way to get the drive letter from that info. So, what shall I do ? I've already posted several patches with different combinations, so please, let's decide what to use, then I'll post a patch. 1 - ToolHelp32 - available on 9x, returns full path - available on NT >= 2000, return filename only [1], not a full path. Leaves NT4 out. 2 - PSAPI - Available on NT only, as a seperate dll. I believe that on 2000, it comes with one of the service packs. XP, I believe comes with it. I can't find anywhere specifying for sure which versions of Windows are bundled with it. For all NT versions, it can be downloaded from MSFT. 3 - NtQueryObject(ObjectNameInfo) on the file handle. - Available on NT only, returns full path on XPSP2 at least. - [2] Will only work on the current inferior. That is, if the target_pid_to_exec_file is used for something, else in the future, it will not work. But that is not a problem currently. 4 - NtQueryInformationFile with FileNameInformation on the file handle. - Available on NT only, returns semi-full path relative to the drive base, but - Same as [2] above. 5 - NtQuerySystemInformation - Available on NT only, only returns filename [1], not a full path. 6 - Other ? [1] - Gdb tries to cope with it, by looking for a match in the debug info. Not very perfect, but the best it can be done. In any case, we can put a warning in infcmd.c:attach_command, when the exec file isn't found, instructing the user to use the "file" command. Looking at the options, I'd either go for '3 + 2 + 1' as in the original patch, or just to '2 + 1' as in the latest patch. I'm more inclined into just '2 + 1' like in: http://sourceware.org/ml/gdb-patches/2007-12/msg00456.html Chris, could you please take a peek at that patch ? I think you may have missed it. > For the Windows 9x case, I'd prefer if there was just a straightforward > error which indicated that what was being attempted is not available on > non-NT versions of Windows. > But why? It's not like supporting it in this case is hard. It can be shoved in 30 lines of code, just getting at toolhelp, and iterating over all the processes ... Pretty self contained, doesn't impact anything else. -- Pedro Alves