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

[Pedro Alves <palves at redhat dot com>]

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