From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr1-f68.google.com (mail-wr1-f68.google.com [209.85.221.68]) by sourceware.org (Postfix) with ESMTPS id 717C13858D37 for ; Mon, 6 Jul 2020 19:03:13 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 717C13858D37 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=palves.net Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=alves.ped@gmail.com Received: by mail-wr1-f68.google.com with SMTP id o11so42387035wrv.9 for ; Mon, 06 Jul 2020 12:03:13 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:subject:date:message-id:in-reply-to :references; bh=IvXWlYtFtgyP/6kmnNn3RpeTf8xeGaopurC1+YlRJPc=; b=tTxVdHn7HzGsptCA+J4KGzlio9htR97cB0cLh5rEL7gHPhReC++99gBXch+v6dTqPN Bx071QdqCzR9FPUVahOJhX1D1fAeEVfWD44ILQZWVHo+kWxQ2D2/jZhzqapwC8ImrW9i 5mA66TCOgZcOeCHLh2oxDU9dwPvqSfpt2CJb7ij/FKb+th8h6LyLucrwzDbcCZVbPVcW kIk+QuqpYsofTJxMFWG+vNyer4n79WOMyB4pEnQ/kQQvkknrj4lzRZPjyIYxK7mFXDuY J41qIx0uoefe0B1RDI0d3/wk15OaaSVsLLnB+FhLB6xkxSOvu7kE5DPhmMQvBPmocLxq 9O7Q== X-Gm-Message-State: AOAM533wDH006pLO7kSZU6bZLro0YP6LDLndThSUXkmihvTaLqF4FEZv zBKNVWtrlHBL+POsCPSYZKMctjS/Da5+iw== X-Google-Smtp-Source: ABdhPJzYSsop0pNUIJfmmfQy+fj/UzN8gI90YmyOEw4Xt3myfYNegsNxc4067r271c7E/hG7T0l2zg== X-Received: by 2002:a5d:6288:: with SMTP id k8mr48823507wru.373.1594062192188; Mon, 06 Jul 2020 12:03:12 -0700 (PDT) Received: from localhost ([2001:8a0:f91a:c400:8728:8fef:5b85:5934]) by smtp.gmail.com with ESMTPSA id x124sm464972wmx.16.2020.07.06.12.03.10 for (version=TLS1_2 cipher=ECDHE-ECDSA-CHACHA20-POLY1305 bits=256/256); Mon, 06 Jul 2020 12:03:11 -0700 (PDT) From: Pedro Alves To: gdb-patches@sourceware.org Subject: [PATCH 5/7] Make handle_no_resumed transfer terminal Date: Mon, 6 Jul 2020 20:02:50 +0100 Message-Id: <20200706190252.22552-6-pedro@palves.net> X-Mailer: git-send-email 2.14.5 In-Reply-To: <20200706190252.22552-1-pedro@palves.net> References: <20200706190252.22552-1-pedro@palves.net> X-Spam-Status: No, score=-9.4 required=5.0 tests=BAYES_00, FREEMAIL_FORGED_FROMDOMAIN, FREEMAIL_FROM, GIT_PATCH_0, HEADER_FROM_DIFFERENT_DOMAINS, KAM_DMARC_STATUS, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham 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: Mon, 06 Jul 2020 19:03:14 -0000 Let's consider the same use case as in the previous commit: Say you have two inferiors 1 and 2, each connected to a different target, A and B. Now say you set inferior 2 running, with "continue &". Now you select a thread of inferior 1, say thread 1.2, and continue in the foreground. All other threads of inferior 1 are left stopped. Thread 1.2 exits, and thus target A has no other resumed thread, so it reports TARGET_WAITKIND_NO_RESUMED. At this point, because the threads of inferior 2 are still executing the TARGET_WAITKIND_NO_RESUMED event is ignored. Now, the user types Ctrl-C. Because GDB had previously put inferior 1 in the foreground, the kernel sends the SIGINT to that inferior. However, no thread in that inferior is executing right now, so ptrace never intercepts the SIGINT -- it is never dequeued by any thread. The result is that GDB's CLI is stuck. There's no way to get back the prompt (unless inferior 2 happens to report some event). The fix in this commit is to make handle_no_resumed give the terminal to some other inferior that still has threads executing so that a subsequent Ctrl-C reaches that target first (and then GDB intercepts the SIGINT). This is a bit hacky, but seems like the best we can do with the current design. I think that putting all native inferiors in their own session would help fixing this in a clean way, since with that a Ctrl-C on GDB's terminal will _always_ reach GDB first, and then GDB can decide how to pause the inferior. But that's a much larger change. The testcase added by the following patch needs this fix. gdb/ChangeLog: PR gdb/26199 * infrun.c (handle_no_resumed): Transfer terminal to inferior with executing threads. --- gdb/infrun.c | 66 +++++++++++++++++++++++++++++++++++++++++++++++++----------- 1 file changed, 54 insertions(+), 12 deletions(-) diff --git a/gdb/infrun.c b/gdb/infrun.c index 0f2f45a4d2..158b199069 100644 --- a/gdb/infrun.c +++ b/gdb/infrun.c @@ -5071,20 +5071,52 @@ handle_no_resumed (struct execution_control_state *ecs) the synchronous command and show "no unwaited-for " to the user. */ - { - scoped_restore_current_thread restore_thread; + inferior *curr_inf = current_inferior (); - for (auto *target : all_non_exited_process_targets ()) - { - switch_to_target_no_thread (target); - update_thread_list (); - } - } + scoped_restore_current_thread restore_thread; + + for (auto *target : all_non_exited_process_targets ()) + { + switch_to_target_no_thread (target); + update_thread_list (); + } + + /* If: + + - the current target has no thread executing, and + - the current inferior is native, and + - the current inferior is the one which has the terminal, and + - we did nothing, + + then a Ctrl-C from this point on would remain stuck in the + kernel, until a thread resumes and dequeues it. That would + result in the GDB CLI not reacting to Ctrl-C, not able to + interrupt the program. To address this, if the current inferior + no longer has any thread executing, we give the terminal to some + other inferior that has at least one thread executing. */ + bool swap_terminal = true; + + /* Whether to ignore this TARGET_WAITKIND_NO_RESUMED event, or + whether to report it to the user. */ + bool ignore_event = false; for (thread_info *thread : all_non_exited_threads ()) { - if (thread->executing - || thread->suspend.waitstatus_pending_p) + if (swap_terminal && thread->executing) + { + if (thread->inf != curr_inf) + { + target_terminal::ours (); + + switch_to_thread (thread); + target_terminal::inferior (); + } + swap_terminal = false; + } + + if (!ignore_event + && (thread->executing + || thread->suspend.waitstatus_pending_p)) { /* Either there were no unwaited-for children left in the target at some point, but there are now, or some target @@ -5094,9 +5126,19 @@ handle_no_resumed (struct execution_control_state *ecs) fprintf_unfiltered (gdb_stdlog, "infrun: TARGET_WAITKIND_NO_RESUMED " "(ignoring: found resumed)\n"); - prepare_to_wait (ecs); - return 1; + + ignore_event = true; } + + if (ignore_event && !swap_terminal) + break; + } + + if (ignore_event) + { + switch_to_inferior_no_thread (curr_inf); + prepare_to_wait (ecs); + return 1; } /* Go ahead and report the event. */ -- 2.14.5