From: Simon Marchi via Gdb-patches <gdb-patches@sourceware.org>
To: Pedro Alves <pedro@palves.net>,
Simon Marchi <simon.marchi@efficios.com>,
gdb-patches@sourceware.org
Subject: Re: [PATCH v2] gdb: make "start" breakpoint inferior-specific
Date: Thu, 10 Nov 2022 12:33:28 -0500 [thread overview]
Message-ID: <0ebeeb02-db83-53ac-2bcb-05dc92b96316@simark.ca> (raw)
In-Reply-To: <47b696c4-6584-8165-0799-5d742132361a@palves.net>
On 11/10/22 11:45, Pedro Alves wrote:
> On 2022-11-08 9:20 p.m., Simon Marchi via Gdb-patches wrote:
>> New in v2:
>>
>> - Change the test so it doesn't call the main function
>>
>> I saw this failure on a CI:
>>
>> (gdb) add-inferior
>> [New inferior 2]
>> Added inferior 2
>> (gdb) PASS: gdb.threads/vfork-multi-inferior.exp: method=non-stop: add-inferior
>> inferior 2
>> [Switching to inferior 2 [<null>] (<noexec>)]
>> (gdb) PASS: gdb.threads/vfork-multi-inferior.exp: method=non-stop: inferior 2
>> kill
>> The program is not being run.
>> (gdb) file /home/jenkins/workspace/binutils-gdb_master_linuxbuild/platform/jammy-amd64/target_board/unix/tmp/tmp.GYATAXR8Ku/gdb/testsuite/outputs/gdb.threads/vfork-multi-inferior/vfork-multi-inferior-sleep
>> Reading symbols from /home/jenkins/workspace/binutils-gdb_master_linuxbuild/platform/jammy-amd64/target_board/unix/tmp/tmp.GYATAXR8Ku/gdb/testsuite/outputs/gdb.threads/vfork-multi-inferior/vfork-multi-inferior-sleep...
>> (gdb) run &
>> Starting program: /home/jenkins/workspace/binutils-gdb_master_linuxbuild/platform/jammy-amd64/target_board/unix/tmp/tmp.GYATAXR8Ku/gdb/testsuite/outputs/gdb.threads/vfork-multi-inferior/vfork-multi-inferior-sleep
>> (gdb) PASS: gdb.threads/vfork-multi-inferior.exp: method=non-stop: run inferior 2
>> inferior 1
>> [Switching to inferior 1 [<null>] (<noexec>)]
>> (gdb) PASS: gdb.threads/vfork-multi-inferior.exp: method=non-stop: inferior 1
>> kill
>> The program is not being run.
>> (gdb) file /home/jenkins/workspace/binutils-gdb_master_linuxbuild/platform/jammy-amd64/target_board/unix/tmp/tmp.GYATAXR8Ku/gdb/testsuite/outputs/gdb.threads/vfork-multi-inferior/vfork-multi-inferior
>> Reading symbols from /home/jenkins/workspace/binutils-gdb_master_linuxbuild/platform/jammy-amd64/target_board/unix/tmp/tmp.GYATAXR8Ku/gdb/testsuite/outputs/gdb.threads/vfork-multi-inferior/vfork-multi-inferior...
>> (gdb) break should_break_here
>> Breakpoint 1 at 0x11b1: file /home/jenkins/workspace/binutils-gdb_master_linuxbuild/platform/jammy-amd64/target_board/unix/src/binutils-gdb/gdb/testsuite/gdb.threads/vfork-multi-inferior.c, line 25.
>> (gdb) PASS: gdb.threads/vfork-multi-inferior.exp: method=non-stop: break should_break_here
>> [Thread debugging using libthread_db enabled]
>> Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".
>> start
>> Temporary breakpoint 2 at 0x11c0: -qualified main. (2 locations)
>> Starting program: /home/jenkins/workspace/binutils-gdb_master_linuxbuild/platform/jammy-amd64/target_board/unix/tmp/tmp.GYATAXR8Ku/gdb/testsuite/outputs/gdb.threads/vfork-multi-inferior/vfork-multi-inferior
>> [Thread debugging using libthread_db enabled]
>> Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".
>>
>> Thread 2.1 "vfork-multi-inf" hit Temporary breakpoint 2, main () at /home/jenkins/workspace/binutils-gdb_master_linuxbuild/platform/jammy-amd64/target_board/unix/src/binutils-gdb/gdb/testsuite/gdb.threads/vfork-multi-inferior-sleep.c:23
>> 23 sleep (30);
>> (gdb) FAIL: gdb.threads/vfork-multi-inferior.exp: method=non-stop: start inferior 1
>>
>> What happens is:
>>
>> 1. We start inferior 2 with "run&", it runs very slowly, takes time to
>> get to main
>> 2. We switch to inferior 1, and run "start"
>> 3. The temporary breakpoint inserted by "start" applies to all inferiors
>> 4. Inferior 2 hits that breakpoint and GDB reports that hit
>>
>> To avoid this, breakpoints inserted by "start" should be
>> inferior-specific. However, we don't have a nice way to make
>> inferior-specific breakpoints yet. It's possible to make
>> pspace-specific breakpoints (for example how the internal_breakpoint
>> constructor does) by creating a symtab_and_line manually. However,
>> inferiors can share program spaces (usually on particular embedded
>> targets), so we could have a situation where two inferiors run the same
>> code in the same program space. In that case, it would just not be
>> possible to insert a breakpoint in one inferior but not the other.
>>
>> A simple solution that should work all the time is to add a condition to
>> the breakpoint inserted by "start", to check the inferior reporting the
>> hit is the expected one. This is what this patch implements.
>>
>
> Even though this does work, it still sets the breakpoint on all the pspaces
> unnecessarily. It would be nice if the breakpoint was pspace specific, in
> addition to inferior specific like you have (or some other way). But, what you
> have is fine with me as is, as it is better than what we have today.
> Maybe just add a little comment suggesting that it would be even better
> to make the breakpoint apply to the current pspace only?
Will do.
>> Add a test that does:
>>
>> - start in background inferior 1 that sleeps 3 seconds before reaching
>> its main function (using a sleep in a global C++ object constructor)
>> - start inferior 2, which also sleeps 3 seconds before reaching its m
>> ain function, with the "start" command
>> - validate that we hit the breakpoint in inferior 2
>>
>> Without the fix, we hit the breakpoint in inferior 1 pretty much all the
>> time. There could be some unfortunate scheduling causing the test not
>> to catch the bug, for instance if the scheduler decides not to schedule
>> inferior 1 for a long time, but it would be really rare. If the bug is
>> re-introduced, the test will catch it much more often than not, so it
>> will be noticed.
>
> My thinking when I saw that both inferiors wait 3 seconds before reaching
> main (before reading the commit log), was, "???, I don't understand this."
>
> So this is assuming that because the first inferior was started first, that
> its 3 seconds always finish before the second inferior's 3 seconds? That
> seems a bit risky. gdb and the other inferiors may all be running on
> different cores, and on a fast machine, gdb may be fast enough to spawn
> both processes roughtly at the same time, and then inferior 2 may end up
> reporting the hit first.
>
> Why not make it so that inferior 3 takes like 3 seconds to reach main,
> but inferior 2 takes 4 or 5 seconds? Or 2 vs 4. Something like that.
> I.e., make sure that inferior 2's time is larger than inferior 1's.
> That could be done by tweaking the sleep calls in the programs, or
> adding a sleep call in the .exp file between "run&" and starting the
> second inferior.
>
> But maybe I'm missing something?
Hmm no, you're right, there's no reason for the two sleep times to be
equal. I'll do 2 seconds for inferior 1 and 4 seconds for inferior 2.
>> +proc do_test {} {
>> + # With remote, to be able to start an inferior while another one is
>> + # running, we need to use the non-stop variant of the protocol.
>> + save_vars { ::GDBFLAGS } {
>> + if { [target_info gdb_protocol] == "extended-remote"} {
>> + append ::GDBFLAGS " -ex \"maintenance set target-non-stop on\""
>> + }
>> +
>> + clean_restart ${::binfile_other}
>> + }
>> +
>> + gdb_test "run&" "Starting program: .*" "start background inferior"
>
> I was going to point out that if the inferior prints something, this can
> timeout, as that output would appear after the prompt. I then looked around
> the tree for "run&" uses, to confirm we are using gdb_test_multiple with that,
> and found that you just recently added "gdb_test -no-prompt-anchor", for exactly
> this scenario. :-) I think that should be used here.
Even if, in this case, we know the inferior won't print anything?
Simon
next prev parent reply other threads:[~2022-11-10 17:34 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-08-04 17:40 [PATCH] " Simon Marchi via Gdb-patches
2022-08-17 17:56 ` Simon Marchi via Gdb-patches
2022-08-31 14:03 ` Bruno Larsen via Gdb-patches
2022-11-04 16:52 ` Simon Marchi via Gdb-patches
2022-11-07 8:14 ` Bruno Larsen via Gdb-patches
2022-11-08 17:24 ` Tom Tromey
2022-09-01 10:42 ` Andrew Burgess via Gdb-patches
2022-11-04 17:24 ` Simon Marchi via Gdb-patches
[not found] ` <8735asb7cj.fsf@redhat.com>
2022-11-09 13:19 ` Simon Marchi via Gdb-patches
2022-11-08 19:43 ` Pedro Alves
2022-11-08 20:14 ` Simon Marchi via Gdb-patches
2022-11-08 21:09 ` Pedro Alves
2022-11-08 21:20 ` [PATCH v2] " Simon Marchi via Gdb-patches
2022-11-10 16:45 ` Pedro Alves
2022-11-10 17:33 ` Simon Marchi via Gdb-patches [this message]
2022-11-10 17:36 ` Simon Marchi via Gdb-patches
2022-11-10 17:47 ` Pedro Alves
2022-11-10 17:53 ` Simon Marchi via Gdb-patches
2022-11-11 12:37 ` Tom de Vries via Gdb-patches
2022-11-11 13:53 ` Simon Marchi via Gdb-patches
2022-11-11 15:21 ` Tom de Vries via Gdb-patches
2022-11-11 19:03 ` Simon Marchi via Gdb-patches
2022-11-12 10:43 ` Tom de Vries via Gdb-patches
2022-11-14 11:29 ` Tom de Vries via Gdb-patches
2022-11-14 13:19 ` Simon Marchi via Gdb-patches
2022-11-14 14:18 ` Tom de Vries via Gdb-patches
2022-11-16 16:22 ` Tom Tromey
2022-11-16 16:26 ` Simon Marchi via Gdb-patches
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=0ebeeb02-db83-53ac-2bcb-05dc92b96316@simark.ca \
--to=gdb-patches@sourceware.org \
--cc=pedro@palves.net \
--cc=simark@simark.ca \
--cc=simon.marchi@efficios.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox