From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 55694 invoked by alias); 27 Apr 2019 16:16:27 -0000 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 Received: (qmail 55686 invoked by uid 89); 27 Apr 2019 16:16:26 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-18.7 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,SPF_HELO_PASS,SPF_PASS autolearn=ham version=3.3.1 spammy= X-HELO: simark.ca Received: from simark.ca (HELO simark.ca) (158.69.221.121) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Sat, 27 Apr 2019 16:16:25 +0000 Received: from [10.0.0.11] (unknown [192.222.164.54]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id 5E1BB1E7B0; Sat, 27 Apr 2019 12:16:23 -0400 (EDT) Subject: Re: Fix lookup of separate debug file on MS-Windows To: Eli Zaretskii Cc: gdb-patches@sourceware.org References: <83ef5ze84y.fsf@gnu.org> <03da9895-5136-1da6-8c37-c4be0d06b608@gmail.com> <8336mfdumf.fsf@gnu.org> <831s1va7h8.fsf@gnu.org> <2697b965-e108-5e7c-75d3-9baa7493141c@simark.ca> <835zr68kiw.fsf@gnu.org> From: Simon Marchi Message-ID: <491fba49-4be5-ef4e-c75e-8b8845673360@simark.ca> Date: Sat, 27 Apr 2019 16:16:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.6.1 MIME-Version: 1.0 In-Reply-To: <835zr68kiw.fsf@gnu.org> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-SW-Source: 2019-04/txt/msg00603.txt.bz2 On 2019-04-22 5:19 a.m., Eli Zaretskii wrote: >> From: Simon Marchi >> Date: Sun, 21 Apr 2019 08:55:07 -0400 >> >> On 2019-04-21 8:05 a.m., Eli Zaretskii wrote: >>> Ping! >>> >>> Would people please voice their opinions regarding preservation of the >>> drive letter (and removing the colon) vs just dropping the drive >>> letter altogether? I'd like to fix this issue for the upcoming >>> release of GDB 8.3. >> >> I first read your patch (which initiated this thread), and my first reaction was >> also to be surprised that we would lose the drive letter. The risk of clash >> is low, but why take that risk at all when it's easy enough to keep the drive letter >> and just strip the colon? > > OK, so how about the patch below? > >> I don't know the complete history of this, so if you know about distributions that >> place debug files in a path without the drive letter (what your patch implements), >> then I would be fine if GDB looked in both places, starting with the path including >> the drive letter. > > I'm not aware of any Windows distributions that put debug info in the > directory suggested by my previous patch. So I think we can safely > use just the /X/ subdirectory method. > > diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo > index a3a5f3e..533a52c 100644 > --- a/gdb/doc/gdb.texinfo > +++ b/gdb/doc/gdb.texinfo > @@ -19917,7 +19917,11 @@ > the directory of the executable file, then in a subdirectory of that > directory named @file{.debug}, and finally under each one of the global debug > directories, in a subdirectory whose name is identical to the leading > -directories of the executable's absolute file name. > +directories of the executable's absolute file name. (On MS-Windows, > +the drive letter of the executable's leading directories is converted > +to a one-letter subdirectory, i.e.@: @file{d:/usr/bin/} is converted > +to @file{/d/usr/bin/}, because Windows filesystems disallow colons in > +file names.) > > @item > For the ``build ID'' method, @value{GDBN} looks in the > diff --git a/gdb/symfile.c b/gdb/symfile.c > index 5736666..5749c61 100644 > --- a/gdb/symfile.c > +++ b/gdb/symfile.c > @@ -1443,11 +1443,24 @@ find_separate_debug_file (const char *dir, > = dirnames_to_char_ptr_vec (debug_file_directory); > gdb::unique_xmalloc_ptr canon_sysroot = gdb_realpath (gdb_sysroot); > > + /* MS-Windows/MS-DOS don't allow colons in file names; we must > + convert the drive letter into a one-letter directory, so that the > + file name resulting from splicing below will be valid. */ > + std::string drive; > + if (HAS_DRIVE_SPEC (dir_notarget)) I think we should consider the case where we build GDB on GNU/Linux, to remotely debug a Windows program. When building on GNU/Linux, HAS_DRIVE_SPEC always return false, since it's defined as (see include/filenames.h): #define HAS_DRIVE_SPEC(f) (0) Let's suppose DEBUGDIR is "D:/my/debug", DIR is "target:E:/the/directory/" and DEBUGLINK is "program.debug". On GNU/Linux, we would build the path target:D:/my/debug/E:/the/directory/program.debug And I suppose that the "E:" would result in the debug file not being found. So I think we should be using HAS_DRIVE_SPEC_1, which allows us to do the same check. We just need to pass to the macro whether the target filesystem id DOS-based. The only problem is, how do we know whether the target filesystem is DOS-based? We wouldn't want HAS_DRIVE_SPEC_1 to do this stripping erroneously when debugging natively on GNU/Linux... If we can't find a practical solution to this, then I would say it's better to keep using HAS_DRIVE_SPEC, and it will be a known deficiency for the moment that it is wrong when debugging remotely from GNU/Linux to Windows. It wouldn't be a regression, as it is already wrong today). But you will have fixed it for the native Windows case, which is a net gain. > + { > + drive = std::string (1, dir_notarget[0]); This should be equivalent to: drive = dir_notarget[0]; > + dir_notarget = STRIP_DRIVE_SPEC (dir_notarget); > + } > + else > + drive = ""; The else clause is unnecessary, as drive is already the empty string; So you could reduce it to: std::string drive; if (HAS_DRIVE_SPEC (dir_notarget)) { drive = dir_notarget[0]; dir_notarget = STRIP_DRIVE_SPEC(dir_notarget); } Simon