From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 1013 invoked by alias); 24 Mar 2004 06:45:14 -0000 Mailing-List: contact gdb-patches-help@sources.redhat.com; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sources.redhat.com Received: (qmail 966 invoked from network); 24 Mar 2004 06:45:13 -0000 Received: from unknown (HELO monty-python.gnu.org) (199.232.76.173) by sources.redhat.com with SMTP; 24 Mar 2004 06:45:13 -0000 Received: from [207.232.27.5] (helo=WST0054) by monty-python.gnu.org with asmtp (Exim 4.30) id 1B6287-00040p-K7; Wed, 24 Mar 2004 01:44:47 -0500 Date: Wed, 24 Mar 2004 06:45:00 -0000 Message-Id: From: Eli Zaretskii To: Elena Zannoni CC: gdb-patches@sources.redhat.com In-reply-to: <16481.2186.51092.295353@localhost.redhat.com> (message from Elena Zannoni on Tue, 23 Mar 2004 23:03:22 -0500) Subject: Re: [PATCH/ezannoni_pie-20040323-branch] New branch and pie Reply-to: Eli Zaretskii References: <16481.2186.51092.295353@localhost.redhat.com> X-SW-Source: 2004-03/txt/msg00554.txt.bz2 > From: Elena Zannoni > Date: Tue, 23 Mar 2004 23:03:22 -0500 > > I gave up updating the old branch that I had created for PIE work. > I just created a new one: ezannoni_pie-20040323-branch. Thanks. Any pointers to previous discussions of the reason(s) for this? I cannot say I understand what is this supposed to do. The following is based on proofreading only. > * auxv.h (target_auxv_parse, target_auxv_search): Update. This log entry doesn't say anything about the change you did. > + if (b->enable_state == bp_startup_disabled) > + { > + char buf[1]; > + > + /* Do not reenable the breakpoint if the shared library > + is still not mapped in. */ > + if (target_read_memory (b->loc->address, buf, 1) == 0) Why use an array for `buf' if all you read into it is a single byte? > --- breakpoint.h 2 Feb 2004 21:10:49 -0000 1.31 > +++ breakpoint.h 24 Mar 2004 04:02:16 -0000 > @@ -159,6 +159,7 @@ enum enable_state > automatically enabled and reset when the call > "lands" (either completes, or stops at another > eventpoint). */ > + bp_startup_disabled, > bp_permanent /* There is a breakpoint instruction hard-wired into > the target's code. Don't try to write another > breakpoint instruction on top of it, or restore The new enumerated value needs some comment to explain its usage, I think. > @@ -352,15 +355,40 @@ symbol_add_stub (void *arg) > /* Have we already loaded this shared object? */ > ALL_OBJFILES (so->objfile) > { > - if (strcmp (so->objfile->name, so->so_name) == 0) > + /* Found an already loaded shared library. */ > + if (strcmp (so->objfile->name, so->so_name) == 0 > + && !so->main) > return 1; > + /* Found an already loaded main executable. This could happen in > + two circumstances. > + First case: the main file has already been read in > + as the first thing that gdb does at startup, and the file > + hasn't been relocated properly yet. Therefor we need to read > + it in with the proper section info. > + Second case: it has been read in with the correct relocation, > + and therefore we need to skip it. */ > + if (strcmp (so->objfile->name, so->so_name) == 0 > + && so->main > + && so->main_relocated) > + return 1; Wouldn't it be better to have a nested if here? if (strcmp (so->objfile->name, so->so_name) == 0) { if (!so->main) return 1; else if (so->main_relocated) return 1; } or even better: if (strcmp (so->objfile->name, so->so_name) == 0) { if (!so->main || so->main_relocated) return 1; } Finally, I think this will eventually need some docs updates.