This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH] Skip VDSO when reading SO list
- From: Andreas Arnez <arnez at linux dot vnet dot ibm dot com>
- To: Jan Kratochvil <jan dot kratochvil at redhat dot com>
- Cc: gdb-patches at sourceware dot org, Andreas Krebbel <krebbel at linux dot vnet dot ibm dot com>
- Date: Fri, 20 Sep 2013 19:00:07 +0200
- Subject: Re: [PATCH] Skip VDSO when reading SO list
- Authentication-results: sourceware.org; auth=none
- References: <87d2p9oi4i dot fsf at br87z6lw dot de dot ibm dot com> <20130920131549 dot GA18629 at host2 dot jankratochvil dot net>
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?