Re: [PATCH] Skip VDSO when reading SO list

Andreas Arnez
2013-09-20T17:00:15Z

Thanks for the comments!

Jan Kratochvil <jan.kratochvil@redhat.com> writes:

> On Mon, 19 Aug 2013 16:44:13 +0200, Andreas Arnez wrote:
>> 2013-08-19 Andreas Arnez <arnez@linux.vnet.ibm.com>
>>
>> * solib-svr4.c (svr4_read_so_list): Skip the VDSO when reading
> vDSO

OK, will replace all occurrences of 'VDSO' by 'vDSO'.

>> link map entries.
>
> Could you include here the testcase from:
> http://pkgs.fedoraproject.org/cgit/gdb.git/tree/gdb-core-open-vdso-warning.patch

Sure.

>> Index: gdb/gdb/solib-svr4.c
>> ===================================================================
>> --- gdb.orig/gdb/solib-svr4.c
>> +++ gdb/gdb/solib-svr4.c
>> @@ -1310,6 +1310,7 @@ static int
>> svr4_read_so_list (CORE_ADDR lm, CORE_ADDR prev_lm,
>> struct so_list ***link_ptr_ptr, int ignore_first)
>> {
>> + struct so_list *first = NULL;
>> CORE_ADDR next_lm;
>>
>> for (; lm != 0; prev_lm = lm, lm = next_lm)
>> @@ -1349,10 +1350,22 @@ svr4_read_so_list (CORE_ADDR lm, CORE_AD
>> {
>> struct svr4_info *info = get_svr4_info ();
>>
>> + first = new;
>> info->main_lm_addr = new->lm_info->lm_addr;
>> do_cleanups (old_chain);
>> continue;
>> }
>> +
>> + /* The l_name of a VDSO sometimes lies in read-only memory that
> vDSO
>> + is excluded from a core dump. In order to avoid the "can't
>> + read pathname" warning, we try to identify the VDSO. One
> vDSO
>> + criteria is that the l_name address matches that of the main
>> + executable. */
>> + if (first && new->lm_info->l_name == first->lm_info->l_name)
>
> Here should be also '&& ignore_first'.

Hm, this shouldn't be necessary, because 'first' is only set when
'ignore_first' is set. Or did I miss something?

>> + {
>> + do_cleanups (old_chain);
>> + continue;
>> + }
>
> And move this block below so that the condition is evaluated only if
> target_read_string has really failed.
>
> The purpose is that no workarounds should complicate the code in the case the
> system components are already bug-free (after glibc gets fixed).

That's a good point. Still, after thinking about this some more, I
prefer the order in the original patch, because it prevents a bogus
l_name from being detected in a second scan when the core dump is
debugged on a system with a different glibc version. Users may also
experience this after a glibc update. Thoughts?