From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-io1-xd33.google.com (mail-io1-xd33.google.com [IPv6:2607:f8b0:4864:20::d33]) by sourceware.org (Postfix) with ESMTPS id AE3523858C00 for ; Tue, 3 Oct 2023 17:56:28 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org AE3523858C00 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=adacore.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=adacore.com Received: by mail-io1-xd33.google.com with SMTP id ca18e2360f4ac-79faba5fe12so45674039f.3 for ; Tue, 03 Oct 2023 10:56:28 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=adacore.com; s=google; t=1696355788; x=1696960588; darn=sourceware.org; h=to:in-reply-to:references:message-id:content-transfer-encoding :mime-version:subject:date:from:from:to:cc:subject:date:message-id :reply-to; bh=UKgfk2KljkrUcihAUERt7f3/tQEHZwSMOxbkWGruPuo=; b=Zo4mjDkj82x5oWfMRxr7k2QheNaVyI09iZMSe7D8Eh4+VsFtqPvZuZecMfBPfcAisa mSaFi6m47Wb6jwrbW9QWe3foTQff94TbyoyWzo9VXD11nh35zKWkda0vpexSYTGhqO+x 6ejyqgcsvnR3lehREryMRE0I4cn/jMwOatbDjWV2i5M3MmabwopTEpejlokt/ztZGRsq sKw92nD9o44C/KwnjeL+NsxIErcVu/2X9BA/R772vRh60ZEi5oJjXYqMsIc8Z+TbnlVr cr5akoUNdAimYAe+KXMAwiza/c7DrwIakSNk0LTYbToY+qHJZDopmMRDI15FblbrQonm c9xw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1696355788; x=1696960588; h=to:in-reply-to:references:message-id:content-transfer-encoding :mime-version:subject:date:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=UKgfk2KljkrUcihAUERt7f3/tQEHZwSMOxbkWGruPuo=; b=kJm2MwM8Y+yDuN5MSHKTpnL8ZqMyjT92cXiZDr7Ln+2mJR3VOxuUPSgLpcGRwm/arC KV1Pxb3TrvVz/GwvK1pKtt31FiX/ADE4tv/gbvC3Gtl3Sk752SKZyPFgS81kFSegA/YP /AALUPkRAc8baIGIEzCDki8pKQlzNsehSTIew4PGvfovsV4Ju+lVh7PYSWD01NA8XiPb yMkkccyvArH4mmmJDDVBbU5eIn4kfW8JDU+WKxgspAmAI84fBFvRh1z62FDaOeB0oN5x tDgRlBYu1iZftrVKhNRgSt/wTC+EfBe8trklFkv4gJzYH/8SEkqLs0HOX40EjK6Trsuy /k+A== X-Gm-Message-State: AOJu0YzfUBbltXu3PoC1Y+uyu7GtCGRyEHoy+vfJ1ZvG1xIpZPLYaY0x JyexJ8OAUUsl/HI6yIaYHjSS23kSyCczoXcyXQ6hYA== X-Google-Smtp-Source: AGHT+IGccC8SkGW9Kckp64TeOzMH5JLw/ZDYRNhPo1OV40aSBidKJvU9M8qscxJ8fTPcmFGtjRKubw== X-Received: by 2002:a6b:5b15:0:b0:786:cc36:360c with SMTP id v21-20020a6b5b15000000b00786cc36360cmr209355ioh.8.1696355787796; Tue, 03 Oct 2023 10:56:27 -0700 (PDT) Received: from localhost.localdomain (71-211-130-31.hlrn.qwest.net. [71.211.130.31]) by smtp.gmail.com with ESMTPSA id t9-20020a028789000000b0042b326ed1ebsm486965jai.48.2023.10.03.10.56.27 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 03 Oct 2023 10:56:27 -0700 (PDT) From: Tom Tromey Date: Tue, 03 Oct 2023 11:56:24 -0600 Subject: [PATCH 3/3] Bail out of "attach" if a thread cannot be traced MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit Message-Id: <20231003-attach-bug-v1-3-f5de2e583c5d@adacore.com> References: <20231003-attach-bug-v1-0-f5de2e583c5d@adacore.com> In-Reply-To: <20231003-attach-bug-v1-0-f5de2e583c5d@adacore.com> To: gdb-patches@sourceware.org X-Mailer: b4 0.12.3 X-Spam-Status: No, score=-11.5 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,KAM_SHORT,RCVD_IN_DNSWL_NONE,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 List-Id: On Linux, threads are treated much like separate processes by the kernel. In particular, it's possible to ptrace just a single thread. If gdb tries to attach to a multi-threaded inferior, where a non-main thread is already being traced (e.g., by strace), then gdb will get into an infinite loop attempting to attach. This patch fixes this problem by having the attach fail if ptrace fails to attach to any thread of the inferior. --- gdb/linux-nat.c | 49 +++++++++++---- gdb/testsuite/gdb.base/traced-thread.c | 105 +++++++++++++++++++++++++++++++ gdb/testsuite/gdb.base/traced-thread.exp | 54 ++++++++++++++++ gdbserver/linux-low.cc | 15 ++++- 4 files changed, 210 insertions(+), 13 deletions(-) diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c index 7e36ced6292..5974ab95390 100644 --- a/gdb/linux-nat.c +++ b/gdb/linux-nat.c @@ -1024,8 +1024,8 @@ attach_proc_task_lwp_callback (ptid_t ptid) std::string reason = linux_ptrace_attach_fail_reason_string (ptid, err); - warning (_("Cannot attach to lwp %d: %s"), - lwpid, reason.c_str ()); + error (_("Cannot attach to lwp %d: %s"), + lwpid, reason.c_str ()); } } else @@ -1045,13 +1045,6 @@ attach_proc_task_lwp_callback (ptid_t ptid) /* So that wait collects the SIGSTOP. */ lp->resumed = 1; - - /* Also add the LWP to gdb's thread list, in case a - matching libthread_db is not found (or the process uses - raw clone). */ - add_thread (linux_target, lp->ptid); - set_running (linux_target, lp->ptid, true); - set_executing (linux_target, lp->ptid, true); } return 1; @@ -1146,8 +1139,42 @@ linux_nat_target::attach (const char *args, int from_tty) of threads/LWPs, and those structures may well be corrupted. Note that once thread_db is loaded, we'll still use it to list threads and associate pthread info with each LWP. */ - linux_proc_attach_tgid_threads (lp->ptid.pid (), - attach_proc_task_lwp_callback); + try + { + linux_proc_attach_tgid_threads (lp->ptid.pid (), + attach_proc_task_lwp_callback); + } + catch (const gdb_exception_error &) + { + /* Failed to attach to some LWP. Detach any we've already + attached to. */ + iterate_over_lwps (ptid_t (ptid.pid ()), + [] (struct lwp_info *lwp) -> int + { + /* Ignore errors when detaching. */ + ptrace (PTRACE_DETACH, lwp->ptid.lwp (), 0, 0); + delete_lwp (lwp->ptid); + return 0; + }); + + target_terminal::ours (); + target_mourn_inferior (inferior_ptid); + + throw; + } + + /* Add all the LWPs to gdb's thread list. */ + iterate_over_lwps (ptid_t (ptid.pid ()), + [] (struct lwp_info *lwp) -> int + { + if (lwp->ptid.pid () != lwp->ptid.lwp ()) + { + add_thread (linux_target, lwp->ptid); + set_running (linux_target, lwp->ptid, true); + set_executing (linux_target, lwp->ptid, true); + } + return 0; + }); } /* Ptrace-detach the thread with pid PID. */ diff --git a/gdb/testsuite/gdb.base/traced-thread.c b/gdb/testsuite/gdb.base/traced-thread.c new file mode 100644 index 00000000000..f30e09b1fc5 --- /dev/null +++ b/gdb/testsuite/gdb.base/traced-thread.c @@ -0,0 +1,105 @@ +/* This testcase is part of GDB, the GNU debugger. + + Copyright 2023 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 . */ + +#define _GNU_SOURCE + +#include +#include +#include +#include +#include +#include +#include + +int fds[2]; + +_Atomic pid_t bg_tid = 0; + +pthread_barrier_t barrier; + +#define FIVE_MINUTES (5 * 60) + +/* One thread of the child process. This is traced by the parent + process. */ +void * +block (void *ignore) +{ + bg_tid = gettid (); + pthread_barrier_wait (&barrier); + sleep (FIVE_MINUTES); + return 0; +} + +/* The parent process blocks in this function. */ +void +parent_stop (pid_t child_thread_pid) +{ + sleep (FIVE_MINUTES); +} + +int +main () +{ + int result; + + pthread_barrier_init (&barrier, NULL, 2); + + result = pipe (fds); + assert (result != -1); + + pid_t child = fork (); + if (child != 0) + { + /* Parent. */ + close (fds[1]); + + pid_t the_tid; + result = read (fds[0], &the_tid, sizeof (the_tid)); + assert (result == sizeof (the_tid)); + + /* Trace a single, non-main thread of the child. This should + prevent gdb from attaching to the child at all. The bug here + was that gdb would get into an infinite loop repeatedly + trying to attach to this thread. */ + result = ptrace (PTRACE_SEIZE, the_tid, (void *) 0, (void *) 0); + if (result == -1) + perror ("ptrace"); + + parent_stop (child); + } + else + { + /* Child. */ + + close (fds[0]); + + pthread_t thr; + result = pthread_create (&thr, 0, block, 0); + assert (result == 0); + + /* Wait until the TID has been assigned. */ + pthread_barrier_wait (&barrier); + assert (bg_tid != 0); + + result = write (fds[1], &bg_tid, sizeof (bg_tid)); + assert (result == sizeof (bg_tid)); + + sleep (FIVE_MINUTES); + } + + exit (0); +} diff --git a/gdb/testsuite/gdb.base/traced-thread.exp b/gdb/testsuite/gdb.base/traced-thread.exp new file mode 100644 index 00000000000..d590ab1bc5e --- /dev/null +++ b/gdb/testsuite/gdb.base/traced-thread.exp @@ -0,0 +1,54 @@ +# Copyright 2023 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 . */ + +# This test only works on GNU/Linux. +require !use_gdb_stub isnative +require {!is_remote host} +require {istarget *-linux*} + +standard_testfile + +if {[prepare_for_testing "failed to prepare" $testfile $srcfile \ + {debug pthreads}]} { + return -1 +} + +if {![runto "parent_stop"]} { + return -1 +} + +# We don't want to be bothered. +gdb_test_no_output "set confirm off" + +set child_pid [get_integer_valueof child_thread_pid -1 "get child pid"] + +# We should not be able to attach to the child at all, because one +# thread is being traced. The bug here was that gdb would get into an +# infinite loop trying to attach to this thread. +gdb_test "add-inferior" "Added inferior 2.*" "add empty inferior 2" +gdb_test "inferior 2" "Switching to inferior 2.*" "switch to inferior 2" +# Recognize failures from either gdb or gdbserver. +gdb_test "attach $child_pid" \ + "(Cannot attach to|Attaching to process $decimal failed).*" \ + "should not attach to child process" + + +# Now kill the parent process, ending the trace. +gdb_test "inferior 1" "Switching to inferior 1.*" "switch to inferior 1" +gdb_test "kill" ".*" "kill the parent process" + +# Kill the child process as well. Use the shell to avoid funny +# business with gdbserver testing. +remote_exec target "kill -9 $child_pid" diff --git a/gdbserver/linux-low.cc b/gdbserver/linux-low.cc index 40b6a907ad9..cdaf730af47 100644 --- a/gdbserver/linux-low.cc +++ b/gdbserver/linux-low.cc @@ -1128,7 +1128,7 @@ attach_proc_task_lwp_callback (ptid_t ptid) std::string reason = linux_ptrace_attach_fail_reason_string (ptid, err); - warning (_("Cannot attach to lwp %d: %s"), lwpid, reason.c_str ()); + error (_("Cannot attach to lwp %d: %s"), lwpid, reason.c_str ()); } return 1; @@ -1181,7 +1181,18 @@ linux_process_target::attach (unsigned long pid) threads/LWPs, and those structures may well be corrupted. Note that once thread_db is loaded, we'll still use it to list threads and associate pthread info with each LWP. */ - linux_proc_attach_tgid_threads (pid, attach_proc_task_lwp_callback); + try + { + linux_proc_attach_tgid_threads (pid, attach_proc_task_lwp_callback); + } + catch (const gdb_exception_error &) + { + /* Make sure we do not deliver the SIGSTOP to the process. */ + initial_thread->last_resume_kind = resume_continue; + + this->detach (proc); + throw; + } /* GDB will shortly read the xml target description for this process, to figure out the process' architecture. But the target -- 2.40.1