Re: time to workaround libc/13097 in fsf gdb?

[Jan Kratochvil <jan.kratochvil@redhat.com>]

On Sun, 21 Sep 2014 21:12:17 +0200, Pedro Alves wrote:
> Is it really a pain though?

95 lines of gdbarch.* patch + its ChangeLog is really a pain compared to
1 line of C++ virtual override.


> Sounds like a predicate like this would work then?
> 
> 	  if (vsyscall_start <= so->lm_info->l_ld && so->lm_info->l_ld < vsyscall_end)

Yes, I find this test correct.


On Sun, 21 Sep 2014 21:46:38 +0200, Pedro Alves wrote:
> Here's a quick update to the patch that does (just) that, for
> testing.  Doesn't update comments, etc.  It works on f20 here.
> 
> WDYT ?

Yes, it works for me on kernel-2.6.32-220.el6.x86_64 (also verified your
previous patch still displayed the warning).

Clarifying it does not fulfill this your suggestion:

On Fri, 12 Sep 2014 14:14:36 +0200, Pedro Alves wrote:
# I was more inclined to leave the vdso in the shared library list
# though, like ldd does, than filtering it out.

But I understand it was more a suggestion than requirement for patch acceptance.
IMO it was request for an unrelated feature.


> --- a/gdb/gdbarch.sh
> +++ b/gdb/gdbarch.sh
> @@ -1029,6 +1029,10 @@ m:int:insn_is_jump:CORE_ADDR addr:addr::default_insn_is_jump::0
>  # Return -1 if there is insufficient buffer for a whole entry.
>  # Return 1 if an entry was read into *TYPEP and *VALP.
>  M:int:auxv_parse:gdb_byte **readptr, gdb_byte *endptr, CORE_ADDR *typep, CORE_ADDR *valp:readptr, endptr, typep, valp
> +
> +# Find the address range of the current inferior's vsyscall/vDSO, and
> +# write it to *START, *END.  Returns true if found, false otherwise.

I find unclear the description whether *END is the last byte address or the
after-the-last byte address.


> +m:int:vsyscall_range:CORE_ADDR *start, CORE_ADDR *end:start, end::default_vsyscall_range::0
>  EOF
>  }
> 
> diff --git a/gdb/linux-tdep.c b/gdb/linux-tdep.c
> index dae59c5..3f28fc9 100644
> --- a/gdb/linux-tdep.c
> +++ b/gdb/linux-tdep.c
> @@ -1782,6 +1782,54 @@ linux_gdb_signal_to_target (struct gdbarch *gdbarch,
>    return -1;
>  }
> 
> +/* Arguments for symbol_file_add_from_memory_wrapper.  */
> +
> +struct find_mapping_size_args
> +{
> +  CORE_ADDR vaddr;
> +  size_t size;

size_t and not CORE_ADDR?  (This patch uses also unsigned long for this value.)


> +};
> +
> +/* Rummage through mappings to find a mapping size.  */
> +
> +static int
> +find_mapping_size (CORE_ADDR vaddr, unsigned long size,
> +		   int read, int write, int exec, int modified,
> +		   void *data)
> +{
> +  struct find_mapping_size_args *args = data;
> +
> +  if (vaddr == args->vaddr)
> +    {
> +      args->size = size;
> +      return 1;
> +    }
> +  return 0;
> +}
> +
> +/* Implementation of the "vsyscall_range" gdbarch hook.  */
> +
> +static int
> +linux_vsyscall_range (struct gdbarch *gdbarch, CORE_ADDR *start, CORE_ADDR *end)
> +{
> +  struct find_mapping_size_args args;
> +
> +  if (target_auxv_search (&current_target, AT_SYSINFO_EHDR, &args.vaddr) <= 0)
> +    return 0;
> +
> +  /* This is installed by linux_init_abi below, so should always be
> +     available.  */
> +  gdb_assert (gdbarch_find_memory_regions_p (target_gdbarch ()));

Is there a reason for target_gdbarch () and not gdbarch?


> +
> +  args.size = 0;
> +  gdbarch_find_memory_regions (target_gdbarch (),
> +			       find_mapping_size, &args);
> +
> +  *start = args.vaddr;
> +  *end = *start + args.size;
> +  return 1;

Here it returns 1 even if the vDSO was not found.
It will return *start == *end so the current only caller behaves correctly.
But I do not find it fully compliant to its gdbarch.sh documentation.


> +}
> +
>  /* To be called from the various GDB_OSABI_LINUX handlers for the
>     various GNU/Linux architectures and machine types.  */
> 
> @@ -1799,6 +1847,7 @@ linux_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
>  				      linux_gdb_signal_from_target);
>    set_gdbarch_gdb_signal_to_target (gdbarch,
>  				    linux_gdb_signal_to_target);
> +  set_gdbarch_vsyscall_range (gdbarch, linux_vsyscall_range);
>  }
> 
>  /* Provide a prototype to silence -Wmissing-prototypes.  */
[...]
> --- a/gdb/symfile-mem.c
> +++ b/gdb/symfile-mem.c
> @@ -188,33 +188,16 @@ symbol_file_add_from_memory_wrapper (struct ui_out *uiout, void *data)
>    return 0;
>  }
> 
> -/* Rummage through mappings to find the vsyscall page size.  */
> -
> -static int
> -find_vdso_size (CORE_ADDR vaddr, unsigned long size,
> -		int read, int write, int exec, int modified,
> -		void *data)
> -{
> -  struct symbol_file_add_from_memory_args *args = data;
> -
> -  if (vaddr == args->sysinfo_ehdr)
> -    {
> -      args->size = size;
> -      return 1;
> -    }
> -  return 0;
> -}
> -
>  /* Try to add the symbols for the vsyscall page, if there is one.
>     This function is called via the inferior_created observer.  */
> 
>  static void
>  add_vsyscall_page (struct target_ops *target, int from_tty)
>  {
> -  CORE_ADDR sysinfo_ehdr;
> +  CORE_ADDR vsyscall_start, vsyscall_end;
> 
> -  if (target_auxv_search (target, AT_SYSINFO_EHDR, &sysinfo_ehdr) > 0
> -      && sysinfo_ehdr != (CORE_ADDR) 0)
> +  if (gdbarch_vsyscall_range (target_gdbarch (),
> +			      &vsyscall_start, &vsyscall_end))

This is a code cleanup part of the patch which could be separate.


>      {
>        struct bfd *bfd;
>        struct symbol_file_add_from_memory_args args;


Thanks,
Jan