From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 29987 invoked by alias); 17 Dec 2014 13:35:48 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 29926 invoked by uid 89); 17 Dec 2014 13:35:45 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.0 required=5.0 tests=AWL,BAYES_00,SPF_HELO_PASS,SPF_PASS,T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Wed, 17 Dec 2014 13:35:44 +0000 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id sBHDZc6u020525 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL); Wed, 17 Dec 2014 08:35:39 -0500 Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.ams2.redhat.com [10.39.146.11]) by int-mx13.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id sBHDZbEB000456; Wed, 17 Dec 2014 08:35:38 -0500 Message-ID: <549186A9.9080800@redhat.com> Date: Wed, 17 Dec 2014 13:35:00 -0000 From: Pedro Alves User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.3.0 MIME-Version: 1.0 To: Simon Marchi , gdb-patches@sourceware.org Subject: Re: [PATCH 2/5] Linux: on attach, attach to lwps listed under /proc/$pid/task/ References: <1418748834-27545-1-git-send-email-palves@redhat.com> <1418748834-27545-3-git-send-email-palves@redhat.com> <54909B99.5070806@ericsson.com> In-Reply-To: <54909B99.5070806@ericsson.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-SW-Source: 2014-12/txt/msg00482.txt.bz2 On 12/16/2014 08:52 PM, Simon Marchi wrote: > On 2014-12-16 11:53 AM, Pedro Alves wrote: >> ... instead of relying on libthread_db. >> >> I wrote a test that attaches to a program that constantly spawns >> short-lived threads, which exposed several issues. This is one of >> them. >> >> On Linux, we need to attach to all threads of a process (thread group) >> individually. We currently rely on libthread_db to list the threads, >> but that is problematic, because libthread_db relies on reading data >> structures out of the inferior (which may well be corrupted). If >> threads are being created or exiting just while we try to attach, we >> may trip on inconsistencies in the inferior's thread list. To work >> around that, when we see a seemingly corrupt list, we currently retry >> a few times: >> >> static void >> thread_db_find_new_threads_2 (ptid_t ptid, int until_no_new) >> { >> ... >> if (until_no_new) >> { >> /* Require 4 successive iterations which do not find any new threads. >> The 4 is a heuristic: there is an inherent race here, and I have >> seen that 2 iterations in a row are not always sufficient to >> "capture" all threads. */ >> ... >> >> That heuristic may well fail, and when it does, we end up with threads >> in the program that aren't under GDB's control. That's obviously bad >> and results in quite mistifying failures, like e.g., the process dying >> for seeminly no reason when a thread that wasn't attached trips on a >> breakpoint. >> >> There's really no reason to rely on libthread_db for this nowadays >> when we have /proc mounted. In that case, which is the usual case, we >> can list the LWPs from /proc/PID/task/. In fact, GDBserver is already >> doing this. The patch factors out that code that knows to walk the >> task/ directory out of GDBserver, and makes GDB use it too. >> >> Like GDBserver, the patch makes GDB attach to LWPs and _not_ wait for >> them to stop immediately. Instead, we just tag the LWP as having an >> expected stop. Because we can only set the ptrace options when the >> thread stops, we need a new flag in the lwp structure to keep track of >> whether we've already set the ptrace options, just like in GDBserver. >> Note that nothing issues any ptrace command to the threads between the >> PTRACE_ATTACH and the stop, so this is safe (unlike one scenario >> described in gdbserver's linux-low.c). >> >> When we attach to a program that has threads exiting while we attach, >> it's easy to race with a thread just exiting as we try to attach to >> it, like: >> >> #1 - get current list of threads >> #2 - attach to each listed thread >> #3 - ooops, attach failed, thread is already gone >> >> As this is pretty normal, we shouldn't be issuing a scary warning in >> step #3. >> >> When #3 happens, PTRACE_ATTACH usually fails with ESRCH, but sometimes >> we'll see EPERM as well. That happens when the kernel still has the >> kernel in its task list, but the thread is marked as dead. > > "still has the kernel" -> "still has the thread" Indeed. Fixed locally. >> (linux_attach): Adjus to rename and use > > Adjus -> Adjust > Fixed. > > I think it makes sense, not that I know anything about it. Thanks for the review. Thanks, Pedro Alves