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

Pedro Alves
2014-09-26T12:05:35Z

On 09/22/2014 07:35 PM, Jan Kratochvil wrote:

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

Thanks for the testing.

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

OK. Best just use "struct mem_range" instead and avoid this
ambiguity whenever we need a range. That struct uses "int" for
length. Although that should be fixed, that can be done
independently. int is obviously sufficient for the vDSO.

>> +/* 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.)

I assume you means ULONGEST or some such. But, I just moved that
code (and renamed it). IIRC, it's size_t because that's what the
"bfd from remote memory" interface uses. This structure disappears in
the next version. We can work with struct mem_range here too.

>> +
>> +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?

No reason.

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

Yeah. The next version will make that clear in the documentation.

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

OK. The next version splits this out to a preparatory patch.

Thanks. I'll be posting the new series in a bit.

Pedro Alves