From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail.efficios.com (mail.efficios.com [167.114.26.124]) by sourceware.org (Postfix) with ESMTPS id B3FCB3858D39 for ; Tue, 28 Sep 2021 19:12:08 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org B3FCB3858D39 Received: from localhost (localhost [127.0.0.1]) by mail.efficios.com (Postfix) with ESMTP id 3A7E838B117; Tue, 28 Sep 2021 15:12:08 -0400 (EDT) Received: from mail.efficios.com ([127.0.0.1]) by localhost (mail03.efficios.com [127.0.0.1]) (amavisd-new, port 10032) with ESMTP id iyOhVl843-wS; Tue, 28 Sep 2021 15:12:07 -0400 (EDT) Received: from localhost (localhost [127.0.0.1]) by mail.efficios.com (Postfix) with ESMTP id 9345338AC7C; Tue, 28 Sep 2021 15:12:07 -0400 (EDT) DKIM-Filter: OpenDKIM Filter v2.10.3 mail.efficios.com 9345338AC7C X-Virus-Scanned: amavisd-new at efficios.com Received: from mail.efficios.com ([127.0.0.1]) by localhost (mail03.efficios.com [127.0.0.1]) (amavisd-new, port 10026) with ESMTP id eLo39OVORPlT; Tue, 28 Sep 2021 15:12:07 -0400 (EDT) Received: from [10.0.0.11] (192-222-157-6.qc.cable.ebox.net [192.222.157.6]) by mail.efficios.com (Postfix) with ESMTPSA id 5D22F38ACE6; Tue, 28 Sep 2021 15:12:07 -0400 (EDT) Message-ID: Date: Tue, 28 Sep 2021 15:12:06 -0400 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.1.1 Subject: Re: [PATCH 6/6] gdb: don't share aspace/pspace on fork with "detach-on-fork on" and "follow-fork-mode child" Content-Language: en-US To: Tom de Vries , Simon Marchi , John Baldwin , gdb-patches@sourceware.org References: <20210910205402.3853607-1-simon.marchi@efficios.com> <20210910205402.3853607-6-simon.marchi@efficios.com> <021ba846-e43e-e6ef-c827-7e2b8ff8f5e9@FreeBSD.org> <58ca5b53-9d70-b8e7-e4c9-86b9f080f731@polymtl.ca> <41efe1d2-64fe-b0d1-029f-363bfc567bf5@polymtl.ca> From: Simon Marchi In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-11.3 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, NICE_REPLY_A, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 28 Sep 2021 19:12:11 -0000 On 2021-09-28 11:10, Tom de Vries wrote: > On 9/27/21 9:32 PM, Simon Marchi via Gdb-patches wrote: >> On 2021-09-10 23:16, Simon Marchi via Gdb-patches wrote: >>>>> Note that the problem described above happens today with "detach-on-fork >>>>> off" and "follow-fork-mode child", because we create new spaces for the >>>>> child. This will have to be addressed later. >>>>> >>>>> Test-wise, improve gdb.base/foll-fork.exp to set a breakpoint that is >>>>> expected to have a location in each inferiors. Without the fix, when >>>>> the two inferiors erroneously share a program space, GDB reports a >>>>> single location. >>>> >>>> So I wonder about the case where follow-fork-mode is parent and >>>> detach-on-fork is off? In that case, should the existing aspace/pspace >>>> stay with the parent and the child get clones? That is, using the >>>> follow-fork-mode setting to determine which inferior gets the existing >>>> aspace/pspace and assigning the cloned copies to the !follow-fork-mode >>>> inferior? >>> >>> I think that would work, to address the problem described above. >> >> FYI, I tried doing the above (giving the original program space to the >> child with "follow-fork-mode child" and "detach-on-fork off") and I hit >> some problems. Attached to the program space is the list of shared >> libraries. So GDB would now think the parent has no shared library. I >> added a solib_create_inferior_hook call to force re-discovering the >> shared libraries in the parent. That seemed to work, but then in MI >> there were spurious =library-loaded events that made it look like >> inferior 1 re-loaded shared libraries. I'm also worried about what >> other per-program space is kept, that could cause some kind of trouble >> for the parent. So I will put this off for now, as it's not something >> that should be done in a rush, I think. >> >> In the mean time, I'll re-test this series and push it if all seems >> good. > > I'm seeing: > ... > (gdb) PASS: gdb.base/foll-fork.exp: follow-fork-mode=child: > detach-on-fork=on: cmd=next 2: test_follow_fork: break callee > info breakpoints 2^M > Num Type Disp Enb Address What^M > 2 breakpoint keep y ^M > 2.1 y 0x00000000004005ae in callee at > /home/vries/gdb_versions/devel/src/gdb/testsuite/gdb.base/foll-fork.c:9 > inf 2^M > 2.2 y 0x00000000004005ae in callee at > /home/vries/gdb_versions/devel/src/gdb/testsuite/gdb.base/foll-fork.c:9 > inf 1^M > (gdb) FAIL: gdb.base/foll-fork.exp: follow-fork-mode=child: > detach-on-fork=on: cmd=next 2: test_follow_fork: info breakpoints > ... > > Looks like the test-case expects first inf 1, then inf 2, and this is > the other way around. > > Should the test-case regexp be updated? Huh, I remember that have encountered this, and thought I had changed the regexp to allow both orders. I must have lost that change. But it still bugged me, why did I see one order sometimes and the other at other times. On my home computer (Arch Linux) I see "2, 1" and on my work computer (Ubuntu 20.04) I see "1, 2". So that lets me compare both cases. The order comes from the SALs passed to init_breakpoint_sal. It comes from the compare_symbols function here: https://gitlab.com/gnutools/binutils-gdb/-/blob/3a6a0158ee07ba2f960ae4a898897460382dc5ec/gdb/linespec.c#L3568-3592 It compares the value of program_space pointers to determine the order of symbols, so that can explain the difference between two machines. That then decides the order of the locations in the breakpoint::loc list, since add_location_to_breakpoint only sorts by address. So depending on which location is added first, it will give different orders. I think that the order of locations should be as stable as possible. I started writing a patch for that, but then realized that even then, the order in the test could vary, based on the architecture, whether you use PIE or not, etc. And in the end the order doesn't really matter, what is important is that we have two locations. Having a more stable breakpoint location order would be nice, but not as a way to fix this test failure. So, what do you think of the patch below? The regexp accepts (1|2) for both lines. If you have a simple way to say "either 1 then 2 or 2 then 1" (so that it rejects 1 then 1 or 2 then 2), I would take it, but otherwise I think this is sufficient. >From 38f41f929f46926fede29c70064871fc23e0e215 Mon Sep 17 00:00:00 2001 From: Simon Marchi Date: Tue, 28 Sep 2021 13:22:53 -0400 Subject: [PATCH] gdb.base/foll-fork.exp: accept "info breakpoints" output in any order The test currently requires the "inf 1" breakpoint to be before the "inf 2" breakpoint. This is not always the case: info breakpoints 2 Num Type Disp Enb Address What 2 breakpoint keep y 2.1 y 0x0000555555554730 in callee at /home/simark/src/binutils-gdb/gdb/testsuite/gdb.base/foll-fork.c:9 inf 2 2.2 y 0x0000555555554730 in callee at /home/simark/src/binutils-gdb/gdb/testsuite/gdb.base/foll-fork.c:9 inf 1 (gdb) FAIL: gdb.base/foll-fork.exp: follow-fork-mode=parent: detach-on-fork=off: cmd=next 2: test_follow_fork: info breakpoints Since add_location_to_breakpoint uses only the address as a criterion to sort locations, the order of locations at the same address is not stable: it will depend on the insertion order. Here, the insertion order comes from the order of SALs when creating the breakpoint, which can vary from machine to machine. While it would be more user-friendly to have a more stable order for printed breakpoint locations, it doesn't really matter for this test, and it would be hard to define an order that will be the same everywhere, all the time. So, loosen the regexp to not check the "inf 1" vs "inf 2" order. Change-Id: I5ada2e0c6ad0669e0d161bfb6b767229c0970d16 --- gdb/testsuite/gdb.base/foll-fork.exp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/gdb/testsuite/gdb.base/foll-fork.exp b/gdb/testsuite/gdb.base/foll-fork.exp index 7f9e1cf87c6a..c0beb089d451 100644 --- a/gdb/testsuite/gdb.base/foll-fork.exp +++ b/gdb/testsuite/gdb.base/foll-fork.exp @@ -195,8 +195,8 @@ proc_with_prefix test_follow_fork { follow-fork-mode detach-on-fork cmd } { } gdb_test "info breakpoints $bpnum" \ [multi_line \ - "$bpnum\\.1 .* inf 1" \ - "$bpnum\\.2 .* inf 2"] \ + "$bpnum\\.1 .* inf (1|2)" \ + "$bpnum\\.2 .* inf (1|2)"] \ "info breakpoints" } } -- 2.33.0