From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 111095 invoked by alias); 13 Aug 2015 15:38:29 -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 111080 invoked by uid 89); 13 Aug 2015 15:38:29 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.4 required=5.0 tests=AWL,BAYES_00,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; Thu, 13 Aug 2015 15:38:28 +0000 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (Postfix) with ESMTPS id E0E208E241; Thu, 13 Aug 2015 15:38:26 +0000 (UTC) Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.ams2.redhat.com [10.39.146.11]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id t7DFcOwU015761; Thu, 13 Aug 2015 11:38:25 -0400 Message-ID: <55CCB9F0.60205@redhat.com> Date: Thu, 13 Aug 2015 15:38: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: Don Breazeal , gdb-patches@sourceware.org Subject: Re: [PATCH v2 4/5] Eliminate spurious warnings from remote exec References: <1436996979-32350-1-git-send-email-donb@codesourcery.com> <1438298360-29594-1-git-send-email-donb@codesourcery.com> <1438298360-29594-5-git-send-email-donb@codesourcery.com> In-Reply-To: <1438298360-29594-5-git-send-email-donb@codesourcery.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-SW-Source: 2015-08/txt/msg00357.txt.bz2 On 07/31/2015 12:19 AM, Don Breazeal wrote: > This patch is unchanged from the previous version. > > ----- > > This patch eliminates some spurious gdbserver warnings that occur when > following an exec event on extended-remote Linux targets. > > When gdbserver on Linux sets up the hook for shared library load > detection, an initial step is to read the version number field of the > r_debug structure from memory. In the current implementation, if the > version number is not equal to one, a warning is printed by gdbserver. > However, the number can be zero if the structure has not been > initialized yet. This seems to happen most of the time after an exec. I wonder how come this doesn't trigger right after connection with "target remote"? > > To suppress the warnings the error check was changed so that if > the version number is not equal to one the function silently returns > -1. Subsequent calls to the routine find an initialized r_debug > structure. > > Tested on x86_64 GNU/Linux, both GDB tests and manual testing which > followed an exec, then debugged a shared library loaded by the exec'd > program to ensure that there were no warnings and that debugging shared > libs was not adversely affected. > > Thanks > --Don > > gdb/gdbserver/ > 2015-07-30 Don Breazeal > > * linux-low.c (linux_qxfer_libraries_svr4): > Return silently on r_debug version error instead of > printing a warning. > > --- > gdb/gdbserver/linux-low.c | 20 ++++++++++++++++---- > 1 file changed, 16 insertions(+), 4 deletions(-) > > diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c > index af4619f..c0770b8 100644 > --- a/gdb/gdbserver/linux-low.c > +++ b/gdb/gdbserver/linux-low.c > @@ -580,6 +580,7 @@ handle_extended_wait (struct lwp_info *event_lwp, int wstat) > else if (event == PTRACE_EVENT_EXEC && report_exec_events) > { > struct regcache *regcache; > + struct process_info *proc; > > if (debug_threads) > { > @@ -598,10 +599,15 @@ handle_extended_wait (struct lwp_info *event_lwp, int wstat) > > /* The new executable may be for a different architecture than > that of the execing process, so re-initialize the architecture. > - The call to get_pc will refill the register cache. */ > + The call to get_pc will refill the register cache. Force re- > + initialization of r_debug from the (possibly) different dynamic > + loader. */ > linux_arch_setup_thread (event_thr); > event_lwp->stop_pc = get_pc (event_lwp); > + proc = get_thread_process (event_thr); > + proc->priv->r_debug = 0; (you only mentioned doing this in an earlier patch, I believe. The changelog and commit logs of this patch don't mention this, only the warning.) Seems to be we should reset everything, not just r_debug. E.g., priv->thread_db. After the exec, the new program might not even be threaded. Thus we should probably call thread_db_mourn. Also, the priv->arch_private bits -- those will hold debug registers things, for watchpoints/hw-breakpoints. So what happens if a process that has watchpoints set, execs? It would seem to me that we should completely forget about the previous debug registers mirrors, etc.? Also, what about breakpoints managed by gdbserver? If they were inserted at the time of the exec, mem-break.c will continue believing they are still inserted. That means that if GDB tries to insert another breakpoint at the same address, gdbserver won't actually insert it. And also, if gdb reads code where an old breakpoint is still marked inserted, gdb reads back the old breakpoint's shadow, which doesn't make sense any longer after the exec. This comment in follow_exec in gdb puts it best: /* We've followed the inferior through an exec. Therefore, the inferior has essentially been killed & reborn. */ > > + /* Save the event for reporting. */ (this hunk also seems to belong in some other earlier patch.) > event_lwp->waitstatus.kind = TARGET_WAITKIND_EXECD; > event_lwp->waitstatus.value.execd_pathname > = xstrdup (linux_proc_pid_to_exec_file (lwpid_of (event_thr))); > @@ -6462,10 +6468,16 @@ linux_qxfer_libraries_svr4 (const char *annex, unsigned char *readbuf, > { > if (linux_read_memory (priv->r_debug + lmo->r_version_offset, > (unsigned char *) &r_version, > - sizeof (r_version)) != 0 > - || r_version != 1) > + sizeof (r_version)) != 0) > + warning ("error reading r_debug version from memory"); > + else if (r_version != 1) > { > - warning ("unexpected r_debug version %d", r_version); > + /* We expect version 1 for glibc. If the version is incorrect, > + it probably means that r_debug hasn't been initialized yet. > + Just silently return an error. We will try again in a > + subsequent pass through here, e.g. at the next library load > + event. */ > + return -1; > } > else if (read_one_ptr (priv->r_debug + lmo->r_map_offset, > &lm_addr, ptr_size) != 0) > Thanks, Pedro Alves