This is the mail archive of the
gdb-patches@sources.redhat.com
mailing list for the GDB project.
Re: [PATCH/ezannoni_pie-20040323-branch] New branch and pie
- From: Eli Zaretskii <eliz at elta dot co dot il>
- To: Elena Zannoni <ezannoni at redhat dot com>
- Cc: gdb-patches at sources dot redhat dot com
- Date: 24 Mar 2004 08:46:57 +0200
- Subject: Re: [PATCH/ezannoni_pie-20040323-branch] New branch and pie
- References: <16481.2186.51092.295353@localhost.redhat.com>
- Reply-to: Eli Zaretskii <eliz at gnu dot org>
> From: Elena Zannoni <ezannoni@redhat.com>
> 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.