From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 67277 invoked by alias); 21 Aug 2015 18:44:53 -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 67263 invoked by uid 89); 21 Aug 2015 18:44:51 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.1 required=5.0 tests=AWL,BAYES_05,KAM_LAZY_DOMAIN_SECURITY,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=no 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; Fri, 21 Aug 2015 18:44:51 +0000 Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) by mx1.redhat.com (Postfix) with ESMTPS id E2694550D4 for ; Fri, 21 Aug 2015 18:44:49 +0000 (UTC) Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.ams2.redhat.com [10.39.146.11]) by int-mx14.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id t7LIim4V001440 for ; Fri, 21 Aug 2015 14:44:49 -0400 Message-ID: <55D771A0.7040002@redhat.com> Date: Fri, 21 Aug 2015 18:44:00 -0000 From: Pedro Alves User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.5.0 MIME-Version: 1.0 To: gdb-patches@sourceware.org Subject: Re: [PATCH] gdbserver: don't pick a random thread if the current thread dies References: <1438872279-7416-1-git-send-email-palves@redhat.com> In-Reply-To: <1438872279-7416-1-git-send-email-palves@redhat.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-SW-Source: 2015-08/txt/msg00587.txt.bz2 On 08/06/2015 03:44 PM, Pedro Alves wrote: > In all-stop mode, if the current thread disappears while stopping all > threads, gdbserver calls set_desired_thread(0) ['0' means "I want the > continue thread"] which just picks the first thread in the list. > > This looks like a dangerous thing to do. GDBserver continues > processing whatever it was doing, but to the wrong thread. If > debugging more than one process, we may even pick the wrong process. > Instead, GDBserver should detect the situation and bail out of > whatever is was doing. > > The backends used to pay attention to the set 'cont_thread' (the Hc > thread, used in the old way to resume threads, before vCont), but all > such 'cont_thread' checks have been eliminated meanwhile. The > remaining implicit dependencies that I found on there being a selected > thread in the backends are in the Ctrl-C handling, which some backends > use as thread to send a signal to. Even that seems to me to be better > handled by always using the first thread in the list or by using the > signal_pid PID. > > In order to make this a systematic approach, I'm making > set_desired_thread never fallback to a random thread, and instead end > up with current_thread == NULL, like already done in non-stop mode. > Then I updated all callers to handle the situation. > > I stumbled on this while fixing other bugs exposed by > gdb.threads/fork-plus-threads.exp test. The problems I saw were fixed > in a different way, but in any case, I think the potential for > problems is more or less obvious, and the resulting code looks a bit > less magical to me. > > Tested on x86-64 Fedora 20, w/ native-extended-gdbserver board. > > gdb/gdbserver/ChangeLog: > 2015-08-06 Pedro Alves > > * linux-low.c (wait_for_sigstop): Always switch to no thread > selected if the previously current thread dies. > * lynx-low.c (lynx_request_interrupt): Use the first thread's > process instead of the current thread's. > * remote-utils.c (input_interrupt): Don't check if there's no > current thread. > * server.c (gdb_read_memory, gdb_write_memory): If setting the > current thread to the general thread fails, error out. > (handle_qxfer_auxv, handle_qxfer_libraries) > (handle_qxfer_libraries_svr4, handle_qxfer_siginfo) > (handle_qxfer_spu, handle_qxfer_statictrace, handle_qxfer_fdpic) > (handle_query): Check if there's a thread selected instead of > checking whether there's any thread in the thread list. > (handle_qxfer_threads, handle_qxfer_btrace) > (handle_qxfer_btrace_conf): Don't error out early if there's no > thread in the thread list. > (handle_v_cont, myresume): Don't set the current thread to the > continue thread. > (process_serial_event) : Also set thread_id if the > previous general thread is still alive. > (process_serial_event) : If setting the current > thread to the general thread fails, error out. > * spu-low.c (spu_resume, spu_request_interrupt): Use the first > thread's lwp instead of the current thread's. > * target.c (set_desired_thread): If the desired thread was not > found, leave the current thread pointing to NULL. Return an int > (boolean) indicating success. > * target.h (set_desired_thread): Change return type to int. I pushed this in. Thanks, Pedro Alves