Re: [PING^2][PATCH] in_plt_section: support alternate stub section names

[Pedro Alves <palves at redhat dot com>]

On 06/21/2013 12:42 PM, Maciej W. Rozycki wrote:

>  Well, I have focused here, perhaps mistakenly, on the intended use of the 
> call -- to determine whether the PC is in a dynamic function call 
> trampoline.  Contrary to the description we currently have at 
> in_plt_section, .plt is not -- per SVR4 ABI -- a standard name of the 
> trampoline section.  The name (and the presence of any such section in the 
> first place) is actually left to processor-specific ABI supplements.
> 
>  For many processors .plt has indeed been the choice, but for MIPS .plt 
> has only recently been added as an ABI extension.  The original MIPS SVR4 
> processor-specific ABI supplement defined no specific section name to be 
> used for its .plt equivalent.  I can't easily check what IRIX tools chose 
> for this section's name (if anything; in a final executable you can have 
> ELF segments whose contents are not mapped to any section).  Binutils 
> chose .stubs long ago and more recently switched to .MIPS.stubs.  This may 
> well be the same names IRIX used in different versions (compare .reginfo 
> vs .MIPS.options standard MIPS SVR4 psABI sections).

Right, but extending in_plt_section torwards a general "in trampoline
section" would imply to me hiding platform details underneath, through
e.g., recording the section name in gdbarch, as calls in common
code wouldn't known about such target details.

> 
>  That written...

...

>  ... actually I like your suggestion, especially as it seems pc_in_section 
> will have more uses than just to check for .plt or .MIPS.stubs.

Exactly.  Excellent.

> What I don't like is the extra call nesting for something that is otherwise 
> rather a trivial piece.  I'm not that particularly fond of macros either.  
> How about this change then?

static inline is fine with me.  However, what I really dislike is the
inclusion of solib-svr4.h in parts of the debugger that have nothing
to do with SVR4, or even are implementing a different shared library
support backend, like solib-frv.c solib-dsbt.c, solib-target.c, etc.
That's really an abstraction violation, there bits have nothing to so
with SVR4.

Clearly PLTs exist in the .plt section on multiple solib implementations,
so I'd rather we keep in_plt_section somewhere central (leaving it in
objfiles.h like where it is today is super fine with me).

> +/* Return non-zero if PC is in the SVR4 procedure linkage table section.  */
> +static inline int
> +in_plt_section (CORE_ADDR pc)
> +{
> +  return pc_in_section (pc, ".plt");
> +}

... and we can then just drop "SVR4" from its describing comment.  Even
Symbian has these.

>  
> +/* Return non-zero if PC is in the MIPS SVR4 lazy-binding stub section.  */
> +static inline int
> +in_mips_stubs_section (CORE_ADDR pc)

GDB coding standards require an empty line between describing comment
and function definition.  (There's at least one more instance in
the patch.)

> +{
> +  return pc_in_section (pc, ".MIPS.stubs");
> +}
> +

 
> Index: gdb-fsf-trunk-quilt/gdb/objfiles.h
> ===================================================================
> --- gdb-fsf-trunk-quilt.orig/gdb/objfiles.h	2013-06-06 20:41:11.000000000 +0100
> +++ gdb-fsf-trunk-quilt/gdb/objfiles.h	2013-06-21 00:48:49.961188758 +0100
> @@ -495,7 +495,8 @@ extern int have_minimal_symbols (void);
>  
>  extern struct obj_section *find_pc_section (CORE_ADDR pc);
>  
> -extern int in_plt_section (CORE_ADDR, char *);
> +/* Return non-zero if PC is in a section called NAME.  */
> +extern int pc_in_section (CORE_ADDR, char *);

-- 
Pedro Alves