From patchwork Fri Apr 19 15:13:37 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Pedro Alves X-Patchwork-Id: 88749 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 8890D3846422 for ; Fri, 19 Apr 2024 15:16:05 +0000 (GMT) X-Original-To: gdb-patches@sourceware.org Delivered-To: gdb-patches@sourceware.org Received: from mail-wm1-f43.google.com (mail-wm1-f43.google.com [209.85.128.43]) by sourceware.org (Postfix) with ESMTPS id 544B3384641E for ; Fri, 19 Apr 2024 15:14:09 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 544B3384641E Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=palves.net Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 544B3384641E Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=209.85.128.43 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1713539651; cv=none; b=wIkKwQHOHp7B5NxeGN5T8Z3SAh73VOXzlaBi6jynnJz4j+ruTIk3XEifomS77FTI7AozdCkJieS/LtDgfSTgvRil3o8WRjTik2969MTq1oPxcugTJD8p9YNhhq7v2rZZ5mFGkm0R4vh2XGCqsyyq8WOS0hh+iVvdXKCFYuUNI8s= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1713539651; c=relaxed/simple; bh=f5eEgqJ0JawEHmmgRdxWbXfbhQFzpF+hssXQ1H3nfQ4=; h=From:To:Subject:Date:Message-ID:MIME-Version; b=RUT0uwCW6hWT+CrE9zN2lmImBj91RpVibtw8KoDcE+I6MhxpMyeKA8JEXMYIChrWmHjuZMmip4Pc5Mtj9PUQvGH1LSXZqGWaeDZtHtvYnA+e8FOowYy27tidJ3pjQe2IBDTTuPgFZgWE5V0d3/WVHXfirfFUF36h3EvwbgR9MKw= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-wm1-f43.google.com with SMTP id 5b1f17b1804b1-416a8ec0239so12611615e9.0 for ; Fri, 19 Apr 2024 08:14:08 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1713539647; x=1714144447; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=GghbwyqU55ZVvD65hzISzUbz/amvNOJsjRf8qyg9lPc=; b=DNOqadw9Yh2oVPE3j3UN369fixI2gWVn6c+Hi5osyNyMV55780a9FiKF0qDGwEiQzq NYAfpJ1JKGNHV4ycHZTTqAGYUDczwDWJGDGQmBcyev2tpFfvLf7r4td6tVL07xbyqEvv EFSRbHDfzmw/CYKv5JOiivzuv2oA4N+k4iOvEBfTbefnrWI2XqqLD/kN7of73MkPiYt/ 1aVfTV1JyMbzpRG/+q8nut+RP121b55c9oe/ndaNNvGEZ+E4xIqGWkQ5yazyGAALluEJ G8PZ3l7xn1ry32EX01zkZTvudI3bzVGwt3LsufzphEJtYgWxDaQex88D3cOAyDUhZ2d8 MKnA== X-Gm-Message-State: AOJu0Yzd9m7MJMJBn1xoX7OXgbFBdthhvZ0NJ/y9FvTuNKtHBTKcke8N 3M6FweEb2/4PABkXcB1yLTIwNWRRE2HhsW68GCuVx9hPS/Nd0gB7sZE/I6gv X-Google-Smtp-Source: AGHT+IGk7rrxDwEBnHisAxzXEYhzFZtrRCvXU+DadrPfc07Je4Sv+m7neL+/J5UwaDSUNm4PF2RwJw== X-Received: by 2002:a05:600c:4708:b0:416:6a57:c52 with SMTP id v8-20020a05600c470800b004166a570c52mr2055407wmo.8.1713539647307; Fri, 19 Apr 2024 08:14:07 -0700 (PDT) Received: from localhost ([2001:8a0:f93d:b900:2438:d637:5572:c30a]) by smtp.gmail.com with UTF8SMTPSA id z9-20020a05600c0a0900b00418916f5848sm6621216wmp.43.2024.04.19.08.14.06 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 19 Apr 2024 08:14:06 -0700 (PDT) From: Pedro Alves To: gdb-patches@sourceware.org Subject: [PATCH 07/12] Fix "attach" failure handling with GDBserver Date: Fri, 19 Apr 2024 16:13:37 +0100 Message-ID: <20240419151342.1592474-8-pedro@palves.net> X-Mailer: git-send-email 2.43.2 In-Reply-To: <20240419151342.1592474-1-pedro@palves.net> References: <20240419151342.1592474-1-pedro@palves.net> MIME-Version: 1.0 X-Spam-Status: No, score=-10.6 required=5.0 tests=BAYES_00, FREEMAIL_FORGED_FROMDOMAIN, FREEMAIL_FROM, GIT_PATCH_0, HEADER_FROM_DIFFERENT_DOMAINS, KAM_DMARC_STATUS, KAM_SHORT, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gdb-patches-bounces+patchwork=sourceware.org@sourceware.org This fixes the same issue as the previous patch, but for "attach" instead of "run". If attaching to a process with "attach" (vAttach packet) fails, GDBserver throws an error that escapes all the way to the top level. When an error escapes all the way like that, GDBserver interprets it as a disconnection, and either goes back to waiting for a new GDB connection, or exits, if --once was specified. Here's an example: On the GDB side: ... (gdb) tar extended-remote :9999 ... Remote debugging using :9999 (gdb) attach 1 Attaching to process 1 Attaching to process 1 failed (gdb) On the GDBserver side: $ gdbserver --once --multi :9999 Listening on port 9999 Remote debugging from host 127.0.0.1, port 37464 gdbserver: Cannot attach to process 1: Operation not permitted (1) $ # gdbserver exited This is wrong, as we've connected with extended-remote/--multi. GDBserver should just report an error to vAttach, and continue connected to GDB, waiting for other commands. This commit fixes GDBserver by catching the error locally in handle_v_attach. Note we now let pid == 0 pass down to attach_inferior. That is so we get a useful textual error message to report to GDB. This fixes a couple KFAILs in gdb.base/attach.exp. Still, I thought it would be useful to add a new testcase specifically for this scenario, in case gdb.base/attach.exp is ever split and stops trying to attach again after a failed attach, with the same GDB session. Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=19558 Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31554 Change-Id: I25314c7e5f1435eff69cb84d57ecac13d8de3393 --- gdb/testsuite/gdb.base/attach-fail-twice.c | 27 ++++++ gdb/testsuite/gdb.base/attach-fail-twice.exp | 94 ++++++++++++++++++++ gdb/testsuite/gdb.base/attach.exp | 6 -- gdbserver/server.cc | 41 +++++---- 4 files changed, 145 insertions(+), 23 deletions(-) create mode 100644 gdb/testsuite/gdb.base/attach-fail-twice.c create mode 100644 gdb/testsuite/gdb.base/attach-fail-twice.exp diff --git a/gdb/testsuite/gdb.base/attach-fail-twice.c b/gdb/testsuite/gdb.base/attach-fail-twice.c new file mode 100644 index 00000000000..d045e35e4b4 --- /dev/null +++ b/gdb/testsuite/gdb.base/attach-fail-twice.c @@ -0,0 +1,27 @@ +/* This testcase is part of GDB, the GNU debugger. + + Copyright 2024 Free Software Foundation, Inc. + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see . */ + +#include + +int +main () +{ + for (int i = 0; i < 60; i++) + sleep (1); + + return 0; +} diff --git a/gdb/testsuite/gdb.base/attach-fail-twice.exp b/gdb/testsuite/gdb.base/attach-fail-twice.exp new file mode 100644 index 00000000000..59020463c67 --- /dev/null +++ b/gdb/testsuite/gdb.base/attach-fail-twice.exp @@ -0,0 +1,94 @@ +# Copyright 2024 Free Software Foundation, Inc. + +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + +# Test doing a "attach" that fails, and then another "attach". + +require can_spawn_for_attach + +standard_testfile + +if {[build_executable "failed to build" $testfile $srcfile {debug}]} { + return -1 +} + +set test_spawn_id [spawn_wait_for_attach $binfile] +set testpid [spawn_id_get_pid $test_spawn_id] + +# Test an attach that fails. + +proc test_bad_attach {test} { + global testpid gdb_prompt + + set boguspid 0 + if { [istarget "*-*-*bsd*"] } { + # In FreeBSD 5.0, PID 0 is used for "swapper". Use -1 instead + # (which should have the desired effect on any version of + # FreeBSD, and probably other *BSD's too). + set boguspid -1 + } + gdb_test_multiple "attach $boguspid" $test { + -re "Attaching to.*, process $boguspid.*No such process.*$gdb_prompt $" { + # Response expected on ptrace-based systems (i.e. GNU/Linux). + pass "$test" + } + -re "Attaching to.*, process $boguspid.*denied.*$gdb_prompt $" { + pass "$gdb_test_name" + } + -re "Attaching to.*, process $boguspid.*not permitted.*$gdb_prompt $" { + pass "$gdb_test_name" + } + -re "Attaching to.*, process .*couldn't open /proc file.*$gdb_prompt $" { + # Response expected from /proc-based systems. + pass "$gdb_test_name" + } + -re "Can't attach to process..*$gdb_prompt $" { + # Response expected on Windows. + pass "$gdb_test_name" + } + -re "Attaching to.*, process $boguspid.*failed.*$gdb_prompt $" { + # Response expected on the extended-remote target. + pass "$gdb_test_name" + } + } +} + +# Test an attach that succeeds. + +proc test_good_attach {test} { + gdb_test "attach $::testpid" \ + "Attaching to program.*, process $::testpid.*" \ + "$test" + + set thread_count [get_valueof "" "\$_inferior_thread_count" -1] + gdb_assert {$thread_count > 0} \ + "attached" +} + +proc_with_prefix test {} { + clean_restart $::binfile + + # GDB used to have a bug on Windows where failing to attach once + # made a subsequent "attach" or "run" hang. So it's important for + # this regression test that we try to attach more than once. + + test_bad_attach "bad attach 1" + test_bad_attach "bad attach 2" + + # For good measure, test that we can attach to something after + # failing to attach previously. + test_good_attach "good attach" +} + +test diff --git a/gdb/testsuite/gdb.base/attach.exp b/gdb/testsuite/gdb.base/attach.exp index 7907441054b..84b2d27f3eb 100644 --- a/gdb/testsuite/gdb.base/attach.exp +++ b/gdb/testsuite/gdb.base/attach.exp @@ -173,10 +173,6 @@ proc_with_prefix do_attach_failure_tests {} { gdb_test "add-inferior" "Added inferior 2.*" "add empty inferior 2" gdb_test "inferior 2" "Switching to inferior 2.*" "switch to inferior 2" - # Probe this before the failing attach: the failed attach against GDBserver - # currently leaves the extended-remote target in a bad state. - set do_kfail [target_is_gdbserver] - set test "fail to attach again" gdb_test_multiple "attach $testpid" "$test" { -re "Attaching to process $testpid.*warning: process .* is already traced by process .*$gdb_prompt $" { @@ -193,14 +189,12 @@ proc_with_prefix do_attach_failure_tests {} { gdb_test_no_output "set confirm off" gdb_test "inferior 1" "Switching to inferior 1.*" "switch to inferior 1" - if { $do_kfail } { setup_kfail "gdb/19558" "*-*-*" } gdb_test "kill" "killed.*" "exit after attach failures" # This can probably be replaced with a call to runto or runto_main once # the kfail is removed. gdb_breakpoint "main" gdb_run_cmd - if { $do_kfail } { setup_kfail "gdb/19558" "*-*-*" } gdb_test_multiple "" "stop at main" { -wrap -re "Breakpoint $::decimal, main .*" { pass $gdb_test_name diff --git a/gdbserver/server.cc b/gdbserver/server.cc index b170da44f6a..2633df08ddb 100644 --- a/gdbserver/server.cc +++ b/gdbserver/server.cc @@ -3294,29 +3294,36 @@ static void handle_v_attach (char *own_buf) { client_state &cs = get_client_state (); - int pid; - pid = strtol (own_buf + 8, NULL, 16); - if (pid != 0 && attach_inferior (pid) == 0) - { - /* Don't report shared library events after attaching, even if - some libraries are preloaded. GDB will always poll the - library list. Avoids the "stopped by shared library event" - notice on the GDB side. */ - current_process ()->dlls_changed = false; + int pid = strtol (own_buf + 8, NULL, 16); - if (non_stop) + try + { + if (attach_inferior (pid) == 0) { - /* In non-stop, we don't send a resume reply. Stop events - will follow up using the normal notification - mechanism. */ - write_ok (own_buf); + /* Don't report shared library events after attaching, even if + some libraries are preloaded. GDB will always poll the + library list. Avoids the "stopped by shared library event" + notice on the GDB side. */ + current_process ()->dlls_changed = false; + + if (non_stop) + { + /* In non-stop, we don't send a resume reply. Stop events + will follow up using the normal notification + mechanism. */ + write_ok (own_buf); + } + else + prepare_resume_reply (own_buf, cs.last_ptid, cs.last_status); } else - prepare_resume_reply (own_buf, cs.last_ptid, cs.last_status); + write_enn (own_buf); + } + catch (const gdb_exception_error &exception) + { + sprintf (own_buf, "E.%s", exception.what ()); } - else - write_enn (own_buf); } /* Decode an argument from the vRun packet buffer. PTR points to the