From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 85511 invoked by alias); 1 Feb 2020 12:58:24 -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 85503 invoked by uid 89); 1 Feb 2020 12:58:24 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-4.3 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_LOW autolearn=ham version=3.3.1 spammy= X-HELO: pb-smtp1.pobox.com Received: from pb-smtp1.pobox.com (HELO pb-smtp1.pobox.com) (64.147.108.70) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Sat, 01 Feb 2020 12:58:21 +0000 Received: from pb-smtp1.pobox.com (unknown [127.0.0.1]) by pb-smtp1.pobox.com (Postfix) with ESMTP id BF21841D8B for ; Sat, 1 Feb 2020 07:58:17 -0500 (EST) (envelope-from jonah@kichwacoders.com) DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=pobox.com; h=mime-version :references:in-reply-to:from:date:message-id:subject:to:cc :content-type:content-transfer-encoding; s=sasl; bh=5WtueOUuqxvz wzTY6CAwFWIXlBA=; b=ge22zVpF2Eg4UMbJsVwTsz2Mn1vyfzM35iXLH/iKRrYJ YrRUC3hifIBhJQjgvEXtCNzICugs+j0oBfNhVs6PJgrmiWs9xXvMtkC/gsIGlMkX hSz6KDlq4y81CQUHhMi5nH5pLrrwVS//ZXBsudcjR6uuewW6v90zljlrWwiylV0= Received: from pb-smtp1.nyi.icgroup.com (unknown [127.0.0.1]) by pb-smtp1.pobox.com (Postfix) with ESMTP id B6B0441D8A for ; Sat, 1 Feb 2020 07:58:17 -0500 (EST) (envelope-from jonah@kichwacoders.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed; d=kichwacoders.com; h=mime-version:references:in-reply-to:from:date:message-id:subject:to:cc:content-type:content-transfer-encoding; s=mesmtp; bh=UzgNsHiX1UbOV/k1Rz3Xvzsdzk1u2BFy7soR/eIedjU=; b=Ta1rFeaCcKTVq2O/SYRVjvrQsALuDmlB3vdasqvVb5Yz+rzKJHQd4qRlcTh4RfQ+g9TgLRtbTmxrD8QALVklneoQ9moImTUVFbu260zTcKtScpjwspum/E+t5QJj4U/wAdCkTdDVsiWJOWYGk5N8YYJbvNad7d8vY95oN3yPa1g= Received: from mail-qt1-f174.google.com (unknown [209.85.160.174]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by pb-smtp1.pobox.com (Postfix) with ESMTPSA id 38CCA41D88 for ; Sat, 1 Feb 2020 07:58:17 -0500 (EST) (envelope-from jonah@kichwacoders.com) Received: by mail-qt1-f174.google.com with SMTP id j5so7687760qtq.9 for ; Sat, 01 Feb 2020 04:58:17 -0800 (PST) MIME-Version: 1.0 References: <20200126114033.GA20733@adacore.com> <20200201121720.GA30012@adacore.com> In-Reply-To: <20200201121720.GA30012@adacore.com> From: Jonah Graham Date: Sat, 01 Feb 2020 12:58:00 -0000 Message-ID: Subject: Re: Propose we release GDB 9.1 next weekend (Feb 01-02) To: Joel Brobecker Cc: Simon Marchi , Tom Tromey , gdb-patches@sourceware.org, Pedro Alves , Iain Buclaw , Nick Alcock , Eli Zaretskii Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Pobox-Relay-ID: 83D029B4-44F2-11EA-A2B2-C28CBED8090B-18936988!pb-smtp1.pobox.com X-IsSubscribed: yes X-SW-Source: 2020-02/txt/msg00016.txt.bz2 ~~~ Jonah Graham Kichwa Coders www.kichwacoders.com On Sat, 1 Feb 2020 at 07:17, Joel Brobecker wrote: > > Hi Jonah, > > Thank you for reporting... In time before we created the release! > This is the kind of bug that I think would have forced us to > issue an emergency fixup release. Hi Joel, Thank you for all the hard work. I would like to help in testing a fix - I am mostly not around this weekend so can't do anything useful until next week. > > Hi Simon, > > Thanks very much for looking into it. I second that! > > On Sat, Feb 01, 2020 at 03:00:05AM -0500, Simon Marchi wrote: > > On 2020-01-31 10:20 p.m., Jonah Graham wrote: > > > On Sun, 26 Jan 2020 at 06:40, Joel Brobecker = wrote: > > >> > > >> Pending comments, I will work on the GDB 9.1 release during > > >> the weekend of Feb 01-02. > > >> > > >> > > > > > > Hi folks, > > > > > > Sorry to be coming to the party a little late on this - but GDB 9.1 > > > has a serious regression related to path handling. This affects > > > Eclipse CDT users because the default build system in CDT does not > > > pass absolute paths to GCC when compiling. Thanks to some efforts of > > > users who have been exposed early to this thanks to fc31 pushing > > > pre-releases of 9.1 out we have detected it before release, but just > > > barely :-) > > > > > > The Bug is https://sourceware.org/bugzilla/show_bug.cgi?id=3D24915 and > > > it has been bouncing around for a while, with it seemingly fixed for a > > > while, but it turns out that there are lots of cases that it is not > > > fixed. The patch that causes the problem is > > > https://sourceware.org/git/gitweb.cgi?p=3Dbinutils-gdb.git;h=3Da0c1ff= edcf1988bc13fc5b6d57d3b74a17b60299 > > > > > > The summary is that GDB is often failing to insert breakpoints when > > > the absolute path to the file is passed to GDB if it was compiled with > > > a different path, such as a path with .. in the compilation path, or > > > symlinks. This would require IDEs to know the compilation path of a > > > file it has open when a breakpoint is inserted, rather than only > > > having to know the absolute path to the file that is actually open. > > > > > > Setting basenames-may-differ to on is a workaround as the new code in > > > the commit is guarded by that. However GDB is failing even when > > > basenames don't differ. > > > > > > I have added two short traces in Comment 17 with all the commands > > > (https://sourceware.org/bugzilla/show_bug.cgi?id=3D24915#c17), but I > > > want to show you the part of the trace that indicates this is a bug. > > > The breakpoint fails to insert on the first instance - but then works > > > on the second try with the intervening break with no path. > > > > > > $ /scratch/gdb/build-gdb-9/gdb/gdb --quiet main > > > Reading symbols from main... > > > (gdb) b /scratch/gdb/test/src/main2.c:1 ## Breakpoint fails here > > > No source file named /scratch/gdb/test/src/main2.c. > > > Make breakpoint pending on future shared library load? (y or [n]) n > > > (gdb) b main2.c:1 > > > Breakpoint 1 at 0x609: file ../src/main2.c, line 2. > > > (gdb) b /scratch/gdb/test/src/main2.c:1 ## same command works here > > > Note: breakpoint 1 also set at pc 0x609. > > > Breakpoint 2 at 0x609: file ../src/main2.c, line 2. > > > (gdb) quit > > > $ > > > > > > Sorry for bringing this up so late in the dev cycle - I thought this > > > was fixed, but it was only fixed for a limited number of cases. > > > > Thanks for reporting, I think it's a quite serious problem that we shou= ld > > address before the release. I looked into it a bit, here's my take on = it. > > > > The commit that introduced the regression has an impact when we reach t= his > > point: > > > > #0 find_and_open_source (filename=3D0x6210000c6dc0 "../src/main2.c", d= irname=3D0x60b000062c4e "/tmp/test/build", fullname=3D0x7ffc78d15d50) at /h= ome/simark/src/binutils-gdb/gdb/source.c:1034 > > #1 0x0000560b5146e58a in psymtab_to_fullname (ps=3D0x60b000062e30) at = /home/simark/src/binutils-gdb/gdb/psymtab.c:1149 > > #2 0x0000560b51468b0a in psym_map_symtabs_matching_filename(objfile *,= const char *, const char *, gdb::function_view) (objfile=3D= 0x614000007640, name=3D0x6030000412c0 "/tmp/test/src/main2.c", real_path=3D= 0x621000102d00 "/tmp/test/src/main2.c", callback=3D...) at /home/simark/src= /binutils-gdb/gdb/psymtab.c:182 > > #3 0x0000560b5188b4ea in iterate_over_symtabs(char const*, gdb::functi= on_view) (name=3D0x6030000412c0 "/tmp/test/src/main2.c", ca= llback=3D...) at /home/simark/src/binutils-gdb/gdb/symtab.c:558 > > #4 0x0000560b510c1351 in collect_symtabs_from_filename (file=3D0x60300= 00412c0 "/tmp/test/src/main2.c", search_pspace=3D0x0) at /home/simark/src/b= inutils-gdb/gdb/linespec.c:3798 > > #5 0x0000560b510c15e2 in symtabs_from_filename (filename=3D0x603000041= 2c0 "/tmp/test/src/main2.c", search_pspace=3D0x0) at /home/simark/src/binut= ils-gdb/gdb/linespec.c:3818 > > #6 0x0000560b510b7df5 in parse_linespec (parser=3D0x7ffc78d16b50, arg= =3D0x603000041290 "/tmp/test/src/main2.c:2", match_type=3Dsymbol_name_match= _type::WILD) at /home/simark/src/binutils-gdb/gdb/linespec.c:2615 > > #7 0x0000560b510bbdcd in event_location_to_sals (parser=3D0x7ffc78d16b= 50, location=3D0x60600007fdc0) at /home/simark/src/binutils-gdb/gdb/linespe= c.c:3152 > > #8 0x0000560b510bc655 in decode_line_full (location=3D0x60600007fdc0, = flags=3D1, search_pspace=3D0x0, default_symtab=3D0x0, default_line=3D0, can= onical=3D0x7ffc78d17390, select_mode=3D0x0, filter=3D0x0) at /home/simark/s= rc/binutils-gdb/gdb/linespec.c:3232 > > > > In psym_map_symtabs_matching_filename, we go through all partial symtab= s and > > try to find those that match the given search name in various ways. Th= e function > > psymtab_to_fullname gives us the "full name" of the partial symtab (it = gets it from > > find_and_open_source), but it's not really clear whether that means jus= t any absolute > > path that resolves to that file, or if it means the real/canonical path. > > > > Before the mentioned commit, we always returned the real/canonical path= , after the > > commit, we return an absolute path that may not be the canonical path. = So in my case, > > we end up calling compare_filenames_for_search with "/tmp/test/src/main= 2.c" as the > > search name and "/tmp/test/build/../src/main2.c" as the psymtab's "full= name", and it > > returns false. > > > > My understanding is that Tom's patch meant to change the way the symtab= 's paths are > > displayed, showing the version with symlinks and possibly some ".." in = it, rather than > > the real path. That represents "how the user compiled the file". But = doing so, it also > > changed the symtab's full name that is expected to be a real/canonical= path, for search > > purposes. > > > > [The change of the way the symtab's path is displayed probably explains= why you now see > > non-real/canonical paths in the MI output, as mentioned in the bug.] > > > > If we want to keep both behaviors, I think we need to separate that "fu= llname" field/concept > > in absolute_fullname and realpath_fullname. The former being what Tom'= s patch wants, where we > > keep symlinks and "..", and the latter being the canonical path, which = the search wants. Doing > > so would hopefully make things clearer. I think that just "fullname" i= s confusing: we are left > > wondering if that field contains a canonical path or not. Can fullname field (at least in MI output) continue to be realpath_fullname concept and if needed in MI a new field added for absolute_fullname? In the CLI output users see just file (../src/main2.c), but that is not generally useful in the IDE case as it is display only. > > > > I have a patch that does that, I'll start a test run and see the result= s tomorrow. It's perhaps > > a bit too invasive for the stable branch though. Here it is in the mea= n time: > > > > https://github.com/simark/binutils-gdb/tree/symtab-realpath > > I haven't reviewed the patch super carefully, but I like the idea! > > For the release, it's indeed a bit on the fence. Given the nature of > the patch, With a bit of careful review, we should be able to convince > ourselves that this patch is sufficiently safe, particularly for a .1. > On the other hand, I think the patch that triggered the regression is > a fairly minor enhancement (name of the file printed by annotations). > So perhaps the best way forward is to revert the triggering patch > from the gdb-9-branch. > > I've added Tom to way in on this... Tom If you decide to revert (which would be my preference) I will happily work with you to get this back in. I previously wrote a testcase for the simple case that did already get fixed, I can work on a testcase for the new case too for the suite if it helps. https://sourceware.org/ml/gdb-patches/2019-08/msg00421.html > -- > Joel