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

Jan Kratochvil
2014-09-22T18:35:16Z

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