From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 121957 invoked by alias); 23 Jul 2017 21:27:55 -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 121773 invoked by uid 89); 23 Jul 2017 21:27:50 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-24.8 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,RP_MATCHES_RCVD,SPF_HELO_PASS,SPF_PASS autolearn=ham version=3.3.2 spammy=occasionally, circumstance, reciept, Hx-languages-length:3648 X-HELO: smtp.polymtl.ca Received: from smtp.polymtl.ca (HELO smtp.polymtl.ca) (132.207.4.11) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Sun, 23 Jul 2017 21:27:48 +0000 Received: from simark.ca (simark.ca [158.69.221.121]) (authenticated bits=0) by smtp.polymtl.ca (8.14.7/8.14.7) with ESMTP id v6NLRfnn004511 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT) for ; Sun, 23 Jul 2017 17:27:46 -0400 Received: by simark.ca (Postfix, from userid 112) id A81D41E5E6; Sun, 23 Jul 2017 17:27:41 -0400 (EDT) Received: from simark.ca (localhost [127.0.0.1]) by simark.ca (Postfix) with ESMTP id 3195E1E009; Sun, 23 Jul 2017 17:27:40 -0400 (EDT) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII; format=flowed Content-Transfer-Encoding: 7bit Date: Sun, 23 Jul 2017 21:27:00 -0000 From: Simon Marchi To: Kevin Buettner Cc: gdb-patches@sourceware.org Subject: Re: [PATCH v3 5/7] Add thread_db_notice_clone to gdbserver In-Reply-To: <20170718175525.71d6a418@pinnacle.lan> References: <20170718174156.5da204b0@pinnacle.lan> <20170718175525.71d6a418@pinnacle.lan> Message-ID: <0143cff541ec1983cf3c30fab0998b60@polymtl.ca> X-Sender: simon.marchi@polymtl.ca User-Agent: Roundcube Webmail/1.3.0 X-Poly-FromMTA: (simark.ca [158.69.221.121]) at Sun, 23 Jul 2017 21:27:41 +0000 X-IsSubscribed: yes X-SW-Source: 2017-07/txt/msg00346.txt.bz2 Hi Kevin, On 2017-07-19 02:55, Kevin Buettner wrote: > While working on a patch for fetching a thread handle in gdbserver, I > ran into a circumstance in which tests in gdb.mi/mi-nsmoribund.exp > would occasionally fail. Over a large enough number of runs, it would > fail roughly 2% of the time. > > That thread handle patch caused find_one_thread() to be called on > every stop. find_one_thread() calls td_ta_map_lwp2thr() which, in > turn, can cause ps_get_thread_area() to be called. > ps_get_thread_area() makes a call to ptrace() for getting the thread > area address. If this should happen when the thread is not stopped, > the call to ptrace will return error which in turn propogates back to > find_one_thread(). find_one_thread() calls error() in this instance > which causes the program to die. > > This patch causes find_one_thread() to be called upon reciept of a > clone event. Since the clone is stopped, the circumstances described > above cannot occur. > > gdb/gdbserver/ChangeLog: > > * linux-low.c (handle_extended_wait): Call > thread_db_notice_clone(). > * linux-low.h (thread_db_notice_clone): Declare. > * thread-db.c (thread_db_notice_clone): New function. > --- > gdb/gdbserver/linux-low.c | 2 ++ > gdb/gdbserver/linux-low.h | 7 +++++++ > gdb/gdbserver/thread-db.c | 14 ++++++++++++++ > 3 files changed, 23 insertions(+) > > diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c > index 3d7cfe3..9d831e7 100644 > --- a/gdb/gdbserver/linux-low.c > +++ b/gdb/gdbserver/linux-low.c > @@ -656,6 +656,8 @@ handle_extended_wait (struct lwp_info > **orig_event_lwp, int wstat) > new_lwp->status_pending = status; > } > > + thread_db_notice_clone (get_thread_process (event_thr), ptid); > + > /* Don't report the event. */ > return 1; > } > diff --git a/gdb/gdbserver/linux-low.h b/gdb/gdbserver/linux-low.h > index 6328da0..86cfe51 100644 > --- a/gdb/gdbserver/linux-low.h > +++ b/gdb/gdbserver/linux-low.h > @@ -410,4 +410,11 @@ int thread_db_get_tls_address (struct thread_info > *thread, CORE_ADDR offset, > CORE_ADDR load_module, CORE_ADDR *address); > int thread_db_look_up_one_symbol (const char *name, CORE_ADDR *addrp); > > +/* Called from linux-low.c when a clone event is detected. Upon > entry, > + both the clone and the parent should be stopped. This function > does > + whatever is required have the clone under thread_db's control. */ New line here. > +void thread_db_notice_clone (struct process_info *proc, ptid_t lwp); > + > +int thread_db_thread_handle (ptid_t ptid, gdb_byte **handle, int > *handle_len); This last declaration doesn't belong in this patch I think. > extern int have_ptrace_getregset; > diff --git a/gdb/gdbserver/thread-db.c b/gdb/gdbserver/thread-db.c > index 1ffb79d..eff1914 100644 > --- a/gdb/gdbserver/thread-db.c > +++ b/gdb/gdbserver/thread-db.c > @@ -864,3 +864,17 @@ thread_db_handle_monitor_command (char *mon) > /* Tell server.c to perform default processing. */ > return 0; > } > + Just add a /* See linux-low.h. */ to be fully GDB-style-compliant :) > +void > +thread_db_notice_clone (struct process_info *proc, ptid_t ptid) > +{ > + struct thread_db *thread_db = proc->priv->thread_db; > + > + /* If the thread layer isn't initialized, return. It may just > + be that the program uses clone, but does not use libthread_db. > */ > + if (thread_db == NULL || !thread_db->all_symbols_looked_up) > + return; > + > + if (!find_one_thread (ptid)) > + warning ("Cannot find thread after clone.\n"); > +} Again, this patch LGTM, but if somebody else could look at it I'd appreciate it. Thanks! Simon