From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mout.gmx.net (mout.gmx.net [212.227.17.22]) by sourceware.org (Postfix) with ESMTPS id D5A4438930CB for ; Wed, 29 Apr 2020 09:12:16 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org D5A4438930CB Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=gmx.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=n54@gmx.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=gmx.net; s=badeba3b8450; t=1588151532; bh=0y9HGlCxLT08eQkFlTzkv1wq0EINWMolRsES06wA6zc=; h=X-UI-Sender-Class:From:To:Cc:Subject:Date:In-Reply-To:References; b=SXLDCdJ5HvmvBGMerLmpcf04gG2iZw6TvSYnYRxFVvWb+6A+4oYIdjRAmHRQXMfQl PqLOt313qMQRrUDKMzEioEbjYYQiz/uxWxVZYpJTcL0Leh7dcITeIDG+CS25zcohc3 akKghin2jadauOF4ETOeH4FMI8m6RBucVLpHQQuc= X-UI-Sender-Class: 01bb95c1-4bf8-414a-932a-4f6e2808ef9c Received: from [188.146.226.206] ([188.146.226.206]) by web-mail.gmx.net (3c-app-mailcom-bs07.server.lan [172.19.170.175]) (via HTTP); Wed, 29 Apr 2020 11:12:12 +0200 MIME-Version: 1.0 Message-ID: From: "Kamil Rytarowski" To: "Simon Marchi" Cc: gdb-patches@sourceware.org Subject: Re: [PATCH] Add basic event handling in the NetBSD target Content-Type: text/plain; charset=UTF-8 Date: Wed, 29 Apr 2020 11:12:12 +0200 Importance: normal Sensitivity: Normal In-Reply-To: References: <20200417144508.6366-1-n54@gmx.com> <2eaea1b8-215c-c628-bedc-4b70900c18f9@simark.ca> X-UI-Message-Type: mail X-Priority: 3 X-Provags-ID: V03:K1:Hc6wuouHNk6HVwW8C3Z4lvHAtMaV3xoG4p4QzJqIgup72KZQkeeEOeF1tP/lAk0hNGpi/ AsL7/5HMqRwapqbqnEXaoGWE73ar1Wihugw0w3u7diGGFDOY09DYKB5g/nKkJfHV6pBMAVLmm83f vhWxrPiTH7D5sRJBaiqFVjkNslK579BN88oT+a0zgpDJidKwmHdw+MLvQxTxQb04tX32W88P5G/y /miPRi1QNwY+GyNzxE4yzlpgkdsHzg48WFNkzXktZ4gmCNtmj04HzOGW3mhflhpX8u9iDh4XOLxW rU= X-UI-Out-Filterresults: notjunk:1;V03:K0:yoa2myMnk30=:wONvToNu5gT3rUbT9E6zga ZhVR02PzYzJQecQ7C7CaS+czOCEiMV2f7AYueaZdfBXXQFFvJ5vt2jsVw9ITn+6r7wL6DZ0WR gWB6Z9+ZR3+icYUWXi8sixaYPXJch9QOdIRb6/NzX8ihrru1U439Z8tZ9gHCi59GRHhabXD8t B3NzJOZv63XbS7Rv1KKRSzcVRpDtwFcVWSO3vRCyUrFqGlFV0ML1dmqnjxx74giCaWg2IKgek xbsh6zHmYMHqr5S0BpYykDQnep0aGBCh4rXwfMgJedK+qSB5yDFHBGozbQ6npy9XarUOefgUh V+mX0WujrqRuC1RU+EFTSm1qANWMKlH5Spk9Fd2/E49g2cQkvl5318WyZ2cQ9ow/rEfJuCP5D a1yT9uDNYF3X8UcvCrIoMbklg6HoM/B12Y8/H7Vhs9+vwzGxf2gVIzUQKWwVgdtiLC/w0DAeo 0dR+hu/vfOR3E0MbpPfMVgM7cdRHxG0wBrhgQkBIMvgA1kirmeFG2UcvzogUaoMkFTMP/Ey34 3+Z/0t9zoHP1Ag9uhuPB+DbKzn4Q3FfphZUVKD3W91k6gHT0OeZBTLkqOdKPk7rCRIFdpZwSn Ukb2agws5inIA4EQ57mKScR07mJIredJvRFRJK80uhuCd+fbqVzQU1amfrw6BRcG3SvqjCP/V yzwmwRxSJxjTRK6Yy3fy7WmEgAOBGTRBMe/Ej0VVJEKEnDiagKa80wt6hQ97DJ2HHODQ= Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-5.8 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, FREEMAIL_ENVFROM_END_DIGIT, FREEMAIL_FROM, RCVD_IN_BARRACUDACENTRAL, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_BL, RCVD_IN_MSPIKE_L3, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=no autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) 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: Wed, 29 Apr 2020 09:12:19 -0000 > Sent: Sunday, April 26, 2020 at 11:14 PM > From: "Simon Marchi" > To: "Kamil Rytarowski" , gdb-patches@sourceware.org > Subject: Re: [PATCH] Add basic event handling in the NetBSD target > > On 2020-04-24 6:09 a.m., Kamil Rytarowski wrote: > >> I know that you'll say it's because FreeBSD does it this way, but sti= ll it surprised > >> me. If the core of GDB asks to resume one specific thread, it doesn'= t mean you should > >> suspend the other threads, they should just be left in whatever state= they are. Is there > >> a reason to stop the other threads here? > >> > > > > In Linux there can be non-stop mode and threads are managed (stopped, > > started, etc) individually. On NetBSD we can start and stop the whole > > process, regardless of the number of threads in it. > > > > We need to set the state of all threads before PT_CONTINUE (or assume > > unchanged state of them). > > > > If a thread is not expected to be suspended, we shall set its flag to > > resume. The same for single-step (PT_STEP). This approach simplifies t= he > > implementation as I don't need to track internally which thread has wh= at > > status and merely save a few syscalls. > > Ok. > > >>> + } > >>> + else > >>> + { > >>> + /* If ptid is a wildcard, resume all matching threads (they w= on't run > >>> + until the process is continued however). */ > >>> + for (thread_info *tp : all_non_exited_threads (this, ptid)) > >>> + if (ptrace (PT_RESUME, tp->ptid.pid (), NULL, tp->ptid.lwp = ()) =3D=3D -1) > >>> + perror_with_name (("ptrace")); > >>> + ptid =3D inferior_ptid; > >> > >> Can you explain this? Ideally the resume method should not rely on i= nferior_ptid. We > >> are working on reducing the dependencies on this global, in favor of = passing the context > >> by parameters. Here, the `ptid` parameter should be enough. > >> > > > > The caller can pass minus_one_ptid, which makes no sense for NetBSD an= d > > certainly in the multi-target support. > > > > Scenarios, as I can see them on NetBSD: > > > > - passing ptid_t (pid, 0, 0) -> resume the whole process with all the > > threads > > - passing ptid_t (pid, lwp, 0) -> resume pid::lwp with all the other > > threads suspended in pid (`set scheduler-lock') > > - passing ptid_t (-1, ?, ?) -> confusion to me so I try to find any > > fallback > > > > If we can abandon the -1 case, I can drop the inferior_ptid usage. If > > the core no longer passes -1 (it could be), we shall cleanup existing > > code that handles this as it could happen. If a refactoring can happen= , > > this shall be followed by addition of gdb_assert(ptid !=3D minus_one_p= tid). > > Or a 4th one: resume all threads of all processes that this target manag= es. > OK. I will try to propose this solution. > I don't know off-hand if the core of GDB today can pass minus_one_ptid. = You could > add such an assert, run all your tests, and if everything works, conclud= e it's not > needed. If the assert is hit, you'll know why. > I've checked that the GDB core tries to resume minus_one_ptid. #2 0x0000000000693e08 in dump_core () at utils.c:203 #3 0x00000000006984ba in internal_vproblem(internal_problem *, const char= *, int, const char *, typedef __va_list_tag __va_list_tag *) (problem=3Dp= roblem@entry=3D0xd24840 , file=3D, line=3D553, fmt=3D, ap=3Dap@ent= ry=3D0x7f7fff3266a8) at utils.c:413 #4 0x0000000000698622 in internal_verror (file=3D, line=3D= , fmt=3D, ap=3Dap@entry=3D0x7f7fff3266a8) at= utils.c:438 #5 0x0000000000780497 in internal_error (file=3Dfile@entry=3D0x8cdc9c "nb= sd-nat.c", line=3Dline@entry=3D553, fmt=3D) at errors.cc:55 #6 0x00000000005b18f4 in nbsd_nat_target::resume (this=3D0xd1f9a0 , ptid=3D..., step=3D0, signal=3DGDB_SIGNAL_0) at nbsd= -nat.c:553 #7 0x000000000064ae0d in target_resume (ptid=3D..., step=3Dstep@entry=3D0= , signal=3Dsignal@entry=3DGDB_SIGNAL_0) at target.c:2121 #8 0x0000000000657d29 in target_continue_no_signal (ptid=3D...) at target= .c:3416 #9 0x00000000005aea79 in startup_inferior (proc_target=3Dproc_target@entr= y=3D0xd1f9a0 , pid=3Dpid@entry=3D718, ntraps=3D= ntraps@entry=3D1, last_waitstatus=3Dlast_waitstatus@entry=3D0x0, last_ptid=3Dlast_ptid@e= ntry=3D0x0) at nat/fork-inferior.c:569 #10 0x0000000000520023 in gdb_startup_inferior (pid=3Dpid@entry=3D718, num= _traps=3Dnum_traps@entry=3D1) at fork-child.c:134 #11 0x0000000000555403 in inf_ptrace_target::create_inferior (During symbo= l reading: Duplicate PC 0x559029 for DW_TAG_call_site DIE 0x1999451 [in mo= dule /public/binutils-gdb/gdb/gdb] this=3D0xd1f9a0 , exec_file=3D, = allargs=3D..., env=3D0x7626ff79ae00, from_tty=3D) at inf-ptrace.c:119 #12 0x000000000055b112 in run_command_1 (args=3D, from_tty= =3D1, run_how=3DRUN_NORMAL) at infcmd.c:640 #13 0x0000000000481c96 in cmd_func (cmd=3D, args=3D, from_tty=3D) at cli/cli-decode.c:2004 #14 0x00000000006617b2 in execute_command (p=3D, p@entry=3D= 0x7626ffb1c020 "", from_tty=3D1) at top.c:655 #15 0x0000000000512d5d in command_handler (command=3D0x7626ffb1c020 "") at= event-top.c:588 #16 0x000000000051303a in command_line_handler (rl=3D...) at event-top.c:7= 73 #17 0x0000000000513576 in gdb_rl_callback_handler (rl=3D0x7626ffb1c1e0 "r"= ) at event-top.c:219 #18 0x00000000006cae86 in rl_callback_read_char () at callback.c:281 #19 0x0000000000512317 in gdb_rl_callback_read_char_wrapper_noexcept () at= event-top.c:177 #20 0x00000000005133a7 in gdb_rl_callback_read_char_wrapper (client_data= =3D) at event-top.c:194 #21 0x00000000005121d0 in stdin_event_handler (error=3D, cl= ient_data=3D0x7626ffafe0c0) at event-top.c:516 #22 0x0000000000780b4b in gdb_wait_for_event (block=3Dblock@entry=3D1) at = event-loop.cc:673 #23 0x0000000000780d3d in gdb_do_one_event () at event-loop.cc:215 #24 0x0000000000589ed0 in start_event_loop () at main.c:356 #25 captured_command_loop () at main.c:416 #26 0x000000000058bc5f in captured_main (data=3D0x7f7fff326eb0) at main.c:= 1254 #27 gdb_main (args=3Dargs@entry=3D0x7f7fff326ed0) at main.c:1269 #28 0x00000000007bcf1d in main (argc=3D, argv=3D) at gdb.c:32 > >>> + /* The common code passes WNOHANG that leads to crashes, over= write it. */ > >>> + pid =3D waitpid (ptid.pid (), &status, 0); > >> > >> What is crashing exactly? > >> > > > > The core code asks to waitpid() with WNOHANG, receives pid=3D0 (nobody= ) > > and crashes. > > > > It's a generic bug that shall be fixed... but it looks like nobody > > really obeys the core and the 3rd argument is overwritten (compare: > > rs6000-nat.c, obsd-nat.c, inf-ptrace.c or Linux...). > > > > I've landed into more similar generic nuances during my effort on GDB, > > that handling events promptly can crash the core and I need to change > > the behavior to emulate other kernels. > > > > It would be better to fix the core, but it is not NetBSD's fault and n= ot > > a blocker for NetBSD support. > > > > If there is interest in fixing it, I can file a bug report > > independently. I don't pledge to work on it myself (at least as long a= s > > I can merely focus on one kernel support). > > Ok, I'm fine with doing this and documenting it somewhere. > > Simon > >