Re: [PATCH] Ensure DSO_FILENAME does not dereference NULL.

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

On 11/12/2013 10:24 AM, Marcus Shawcroft wrote:
> The AArch64 and ARM glibc build is currently broken for trunk gcc.
> Both builds fail with link errors for the multiple definition of
> _dl_addr_inside_object.
> 
> This is caused by the "Isolate erroneous paths optimization" patch
> recently added to gcc here:
> 
> http://gcc.gnu.org/ml/gcc-patches/2013-10/msg02648.html
> 
> One effect of this patch is to replace unreachable paths with a call to abort().
> 
> There is code in _dl_lookup_symbol_x() which may pass NULL to
> DSO_FILENAME().  This macro always dereferences the filename.  The
> isolate erroneous paths cod in gcc inserts an abort() in place of the
> ill defined code. The linker subsequently pulls in a bunch of code
> from libpic.a, including the alternative definition of
> _dl_addr_inside_object.
> 
> The convention elsewhere appears to be to pass "" rather than NULL.
> This patch seems obvious to me, I'll commit it tomorrow unless anyone
> objects.

I've reviewed the relevant macro and code and I agree. Your analysis
looks correct.

> * elf/dl-lookup.c (_dl_lookup_symbol_x): Pass "" instead of NULL to
> DSO_FILENAME().

Please use correct GNU ChangeLog format in your patch postings.

Suggest:

2013-11-12  Marcus Shawcroft  <marcus.shawcroft@linaro.org>

	* elf/dl-lookup.c (_dl_lookup_symbol_x): If UNDEF_MAP is false
	set reference_name to "" to avoid passing NULL to DSO_FILENAME.

> 0002-DSO_FILENAME-expects-rather-than-NULL.txt
> 
> 
> diff --git a/elf/dl-lookup.c b/elf/dl-lookup.c
> index 39f463e..f869dcf 100644
> --- a/elf/dl-lookup.c
> +++ b/elf/dl-lookup.c
> @@ -746,7 +746,7 @@ _dl_lookup_symbol_x (const char *undef_name, struct link_map *undef_map,
>  	     contain the needed symbol.  This code is never reached
>  	     for unversioned lookups.  */
>  	  assert (version != NULL);
> -	  const char *reference_name = undef_map ? undef_map->l_name : NULL;
> +	  const char *reference_name = undef_map ? undef_map->l_name : "";
>  
>  	  /* XXX We cannot translate the message.  */
>  	  _dl_signal_cerror (0, DSO_FILENAME (reference_name),
> -- 1.7.9.5

OK.

Cheers,
Carlos.