From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id 97D143858438 for ; Thu, 22 Jun 2023 13:17:42 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 97D143858438 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1687439862; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=qD3h85yOc28kXXbSbpZFA/DABx2nvUwci4wpUNDMVRI=; b=bFtdwXrAO+nCJhdeizuUw3GaoVxrSZsvgW6LBoYJJ6XHUqThLLFvsmqr5tvBiCR6K3Ekzi A6AvaKWZfXVp8Yt59uaC7bKN0f6IjKwwPFQMa5iJCWoRbMg1xziTKVCS0EhMeABfrn/wC5 H7MBuh0WtEGYPSSBqYVYH4vfFOsZ2N0= Received: from mail-wm1-f70.google.com (mail-wm1-f70.google.com [209.85.128.70]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-315-Zt7GYHqrOQSff8zxvrYeLQ-1; Thu, 22 Jun 2023 09:17:41 -0400 X-MC-Unique: Zt7GYHqrOQSff8zxvrYeLQ-1 Received: by mail-wm1-f70.google.com with SMTP id 5b1f17b1804b1-3f9b12b55cfso24987905e9.2 for ; Thu, 22 Jun 2023 06:17:40 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1687439859; x=1690031859; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=qD3h85yOc28kXXbSbpZFA/DABx2nvUwci4wpUNDMVRI=; b=P63xQrU5PnnvzuyOLEJhxgje1vWpcVOO6lt8oz9nhj0rCMoTfSP/c2tpkKd78hCW15 FhqiaqkXVzAl9o4k2YFVljFNryH3oUGWNkLb//OcCyYcz0XYHeE44cWu/KX2ZNt6pbRZ BbwLIgzkmEyRKnPs0k0cfkm0GNlY3svcommujH78rhvV6D3n5W8Ah2FW4eMERi+1RKqX Ig4WR5vcyo7g31wC/c8r+DcOvVhRzhE/Axl/ILPKOPaK4q6sOApeMAG1wW6DvnM3SXmy Q7GO1yRAAkzzfJM/q4+tal23e8PNQzBID2SqDDlWGhG+WGceqzjNNv9ohXFnI8PqA+AT hxsQ== X-Gm-Message-State: AC+VfDwUVpXsfkh76RYrk1SdhAq87UFf8/KrHo0t1uchaGYhTc5G00Hp yn0dZ5O6vxxCRfrVko7NE9rD+Fq3ZQ/LS5iVyCw7UpItUnux4O2xwNbLFww6xEb243SbPaVmmu5 i48HTcBcze7WAHTaVtBpGz28UQbkB74sDAMGtm/z9GSyfOFQ1qOxyGn3ATx3bo7pgSX2EXUjWe+ LmqFaD/Q== X-Received: by 2002:a05:600c:2041:b0:3f9:871:c2da with SMTP id p1-20020a05600c204100b003f90871c2damr11840067wmg.40.1687439859559; Thu, 22 Jun 2023 06:17:39 -0700 (PDT) X-Google-Smtp-Source: ACHHUZ484dwKUJv3KEw2IaasKO6h3KWslFXWgZNTS8wM9Evd+Y5FnUCKfBTMUgsFaZLWUyZrmtupXg== X-Received: by 2002:a05:600c:2041:b0:3f9:871:c2da with SMTP id p1-20020a05600c204100b003f90871c2damr11840049wmg.40.1687439859151; Thu, 22 Jun 2023 06:17:39 -0700 (PDT) Received: from localhost (2.72.115.87.dyn.plus.net. [87.115.72.2]) by smtp.gmail.com with ESMTPSA id q6-20020a7bce86000000b003f735ba7736sm7666143wmj.46.2023.06.22.06.17.36 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 22 Jun 2023 06:17:37 -0700 (PDT) From: Andrew Burgess To: gdb-patches@sourceware.org Cc: Andrew Burgess Subject: [PATCH 3/8] gdb: fix an issue with vfork in non-stop mode Date: Thu, 22 Jun 2023 14:17:23 +0100 Message-Id: <286af487595cc488a31377c2f0d31cdf6893190f.1687438786.git.aburgess@redhat.com> X-Mailer: git-send-email 2.25.4 In-Reply-To: References: MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Transfer-Encoding: 8bit Content-Type: text/plain; charset="US-ASCII"; x-default=true X-Spam-Status: No, score=-11.8 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H5,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_NONE,TXREP,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: This commit fixes a bug introduced by this commit: commit d8bbae6ea080249c05ca90b1f8640fde48a18301 Date: Fri Jan 14 15:40:59 2022 -0500 gdb: fix handling of vfork by multi-threaded program (follow-fork-mode=parent, detach-on-fork=on) The problem can be seen in this GDB session: $ gdb -q (gdb) set non-stop on (gdb) file ./gdb/testsuite/outputs/gdb.base/foll-vfork/foll-vfork Reading symbols from ./gdb/testsuite/outputs/gdb.base/foll-vfork/foll-vfork... (gdb) tcatch vfork Catchpoint 1 (vfork) (gdb) run Starting program: /tmp/gdb/testsuite/outputs/gdb.base/foll-vfork/foll-vfork Temporary catchpoint 1 (vforked process 1375914), 0x00007ffff7d5043c in vfork () from /lib64/libc.so.6 (gdb) bt #0 0x00007ffff7d5043c in vfork () from /lib64/libc.so.6 #1 0x00000000004011af in main (argc=1, argv=0x7fffffffad88) at .../gdb/testsuite/gdb.base/foll-vfork.c:32 (gdb) finish Run till exit from #0 0x00007ffff7d5043c in vfork () from /lib64/libc.so.6 [Detaching after vfork from child process 1375914] No unwaited-for children left. (gdb) Notice the "No unwaited-for children left." error. This is incorrect, given where we are stopped there's no reason why we shouldn't be able to use "finish" to return to the main frame. When the inferior is stopped as a result of the 'tcatch vfork', the inferior is in the process of performing the vfork, that is, GDB has seen the VFORKED event, but has not yet attached to the new child process, nor has the child process been resumed. However, GDB has seen the VFORKED, and, as we are going to follow the parent process, the inferior for the vfork parent will have its thread_waiting_for_vfork_done member variable set, this will point to the one and only thread that makes up the vfork parent process. When the "finish" command is used GDB eventually ends up in the proceed function (in infrun.c), in here we pass through all the function until we eventually encounter this 'else if' condition: else if (!cur_thr->resumed () && !thread_is_in_step_over_chain (cur_thr) /* In non-stop, forbid resuming a thread if some other thread of that inferior is waiting for a vfork-done event (this means breakpoints are out for this inferior). */ && !(non_stop && cur_thr->inf->thread_waiting_for_vfork_done != nullptr)) { The first two of these conditions will both be true, the thread is not already resumed, and is not in the step-over chain, however, the third condition, this one: && !(non_stop && cur_thr->inf->thread_waiting_for_vfork_done != nullptr)) is false, and this prevents the thread we are trying to finish from being resumed. This condition is false because (a) non_stop is true, and (b) cur_thr->inf->thread_waiting_for_vfork_done is not nullptr (see above for why). Now, if we check the comment embedded within the condition it says: /* In non-stop, forbid resuming a thread if some other thread of that inferior is waiting for a vfork-done event (this means breakpoints are out for this inferior). */ And this makes sense, if we have a vfork parent with two thread, and one thread has performed a vfork, then we shouldn't try to resume the second thread. However, if we are trying to resume the thread that actually performed a vfork, then this is fine. If we never resume the vfork parent then we'll never get a VFORK_DONE event, and so the vfork will never complete. Thus, the condition should actually be: && !(non_stop && cur_thr->inf->thread_waiting_for_vfork_done != nullptr && cur_thr->inf->thread_waiting_for_vfork_done != cur_thr)) This extra check will allow the vfork parent thread to resume, but prevent any other thread in the vfork parent process from resuming. This is the same condition that already exists in the all-stop on a non-stop-target block earlier in the proceed function. My actual fix is slightly different to the above, first, I've chosen to use a nested 'if' check instead of extending the original 'else if' check, this makes it easier to write a longer comment explaining what's going on, and second, instead of checking 'non_stop' I've switched to checking 'target_is_non_stop_p'. In this context this is effectively the same thing, a previous 'else if' block in proceed already handles '!non_stop && target_is_non_stop_p ()', so by the time we get here, if 'target_is_non_stop_p ()' then we must be running in non_stop mode. Both of these tweaks will make the next patch easier, which is a refactor to merge two parts of the proceed function, so this nested 'if' block is not going to exist for long. For testing, there is no test included with this commit. The test was exposed when using a modified version of the gdb.base/foll-vfork.exp test script, however, there are other bugs that are exposed when using the modified test script. These bugs will be addressed in subsequent commits, and then I'll add the updated gdb.base/foll-vfork.exp. If you wish to reproduce this failure then grab the updates to gdb.base/foll-vfork.exp from the later commit and run this test, the failure is always reproducible. --- gdb/infrun.c | 34 ++++++++++++++++++++++------------ 1 file changed, 22 insertions(+), 12 deletions(-) diff --git a/gdb/infrun.c b/gdb/infrun.c index 58da1cef29e..5b0257076f0 100644 --- a/gdb/infrun.c +++ b/gdb/infrun.c @@ -3503,19 +3503,29 @@ proceed (CORE_ADDR addr, enum gdb_signal siggnal) } } else if (!cur_thr->resumed () - && !thread_is_in_step_over_chain (cur_thr) - /* In non-stop, forbid resuming a thread if some other thread of - that inferior is waiting for a vfork-done event (this means - breakpoints are out for this inferior). */ - && !(non_stop - && cur_thr->inf->thread_waiting_for_vfork_done != nullptr)) + && !thread_is_in_step_over_chain (cur_thr)) { - /* The thread wasn't started, and isn't queued, run it now. */ - execution_control_state ecs (cur_thr); - switch_to_thread (cur_thr); - keep_going_pass_signal (&ecs); - if (!ecs.wait_some_more) - error (_("Command aborted.")); + /* In non-stop mode, if a there exists a thread waiting for a vfork + then only allow that thread to resume (breakpoints are removed + from an inferior when handling a vfork). + + We check target_is_non_stop_p here rather than just checking the + non-stop flag, though these are equivalent (all-stop on a + non-stop target was handled in a previous block, so at this + point, if target_is_non_stop_p then GDB must be running on + non-stop mode). By using target_is_non_stop_p it will be easier + to merge this block with the previous in a later commit. */ + if (!(target_is_non_stop_p () + && cur_thr->inf->thread_waiting_for_vfork_done != nullptr + && cur_thr->inf->thread_waiting_for_vfork_done != cur_thr)) + { + /* The thread wasn't started, and isn't queued, run it now. */ + execution_control_state ecs (cur_thr); + switch_to_thread (cur_thr); + keep_going_pass_signal (&ecs); + if (!ecs.wait_some_more) + error (_("Command aborted.")); + } } disable_commit_resumed.reset_and_commit (); -- 2.25.4