Re: ping: [patchv2] Fix vDSO l_name for GDB's: Can't read pathname for load map: Input/output error.

["Carlos O'Donell" <carlos at redhat dot com>]

On 11/07/2013 11:09 AM, Jan Kratochvil wrote:
> Hi Carlos,
> 
> [patchv2] Fix vDSO l_name for GDB's: Can't read pathname for load map: Input/output error.
> https://sourceware.org/ml/libc-alpha/2013-08/msg00364.html
> Message-ID: <20130820133807.GA15877@host2.jankratochvil.net>
> 
> -  new->l_name = realname;
> +  /* Ensure empty strings from readonly memory are stored in a written page so
> +     the string gets dumped into the core file.  */
> +  new->l_name = *realname ? realname : newname->name + libname_len - 1;
> 
> That means that currently L_NAME points to a string (empty string "") which is
> not dumped into core file (because it is in a never-written read-only page).
> 
> This means when GDB (or other consumer) later reads the core file L_NAME
> points to a missing page.  GDB does not know there is just "" (empty string).
> 
> GDB therefore correctly prints an error it cannot read the string.
> In the end GDB behaves the same both on an unreadable and on an empty string.
> But the needless error message is confusing users.  GDB has to print the error
> message as the core file really is not consistent/correct.

Please try to understand that reviewing this code is difficult
(at least until we refactor it some more to make it less mystical).
To that end a reviewer really needs your help to understand the conditions
you considered while writing the patch.

Under what conditions do we have L_NAME pointing to an empty string?
* ld.so only?
* dlopen of ""?
* Is it possible with other libraries? 

Why does that imply the empty string is on a read-only page? 
* Is this something that always happens?
* Is this something that only happens with ld.so?

Does the entire glibc testsuite pass after this change?
* It might change the output of LD_DEBUG=all?
* It might change the output of sotruss-lib used for testing?

Do other tools run OK after this change?
* Valgrind has no problems?
* gdb itself has no problems? (I assume it doesn't)
* ldd shows now difference? (Related to LD_DEBUG=all above, but in trace mode)

> 2013-08-20  Jan Kratochvil  <jan.kratochvil@redhat.com>

Update date.
 
> 	Fix reading link map from a core file by debugger.

Move to commit message.

> 	* elf/dl-object.c (_dl_new_object): Initialize L_NAME from NEWNAME if
> 	it is empty.
> 
> diff --git a/elf/dl-object.c b/elf/dl-object.c
> index 0f594d2..e432c46 100644
> --- a/elf/dl-object.c
> +++ b/elf/dl-object.c
> @@ -88,7 +88,9 @@ _dl_new_object (char *realname, const char *libname, int type,
>    /* newname->next = NULL;	We use calloc therefore not necessary.  */
>    newname->dont_free = 1;
>  
> -  new->l_name = realname;
> +  /* Ensure empty strings from readonly memory are stored in a written page so
> +     the string gets dumped into the core file.  */

This comment will need some expanding based on answers to the above questions.

> +  new->l_name = *realname ? realname : newname->name + libname_len - 1;
>    new->l_type = type;
>    /* If we set the bit now since we know it is never used we avoid
>       dirtying the cache line later.  */
> 

Cheers,
Carlos.