Re: [PATCH/ezannoni_pie-20040323-branch] New branch and pie

[Eli Zaretskii <eliz@elta.co.il>]

> 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.