Re: PATCH: Update DF_1_XXX from Solaris

["H.J. Lu" <hjl dot tools at gmail dot com>]

On Wed, Nov 21, 2012 at 11:50 AM, Carlos O'Donell
<carlos@systemhalted.org> wrote:
> On Wed, Nov 21, 2012 at 1:38 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>> On Wed, Nov 21, 2012 at 10:23 AM, Markus Trippelsdorf
>> <markus@trippelsdorf.de> wrote:
>>> On 2012.11.16 at 08:49 -0800, H.J. Lu wrote:
>>>>
>>>> Here is the new patch.  However, I'd like to fix
>>>>
>>>> http://www.sourceware.org/bugzilla/show_bug.cgi?id=14853
>>>>
>>>> so that we can add DF_1_GLOBAL to supported bits.  To test
>>>> it, I need to add a "-z global" linker option to set DF_1_GLOBAL.
>>>
>>> This breaks valgrind on my system (Linux amd64):
>>>
>>> Inconsistency detected by ld.so: get-dynamic-info.h: 155: elf_get_dynamic_info: Assertion `(l->l_flags_1 & ~(0x00000001 | 0x00000008 | 0x00000020 | 0x00000040 | 0x00000080 | 0x00000800)) == 0' failed!
>>> ==18162== Jump to the invalid address stated on the next line
>>> ==18162==    at 0x546: ???
>>> ==18162==    by 0x40155C8: __assert_fail (dl-minimal.c:206)
>>> ==18162==    by 0x4006E29: _dl_map_object_from_fd (get-dynamic-info.h:155)
>>> ==18162==    by 0x4007EDC: _dl_map_object (dl-load.c:2365)
>>> ==18162==    by 0x40013FD: map_doit (rtld.c:626)
>>> ==18162==    by 0x400E445: _dl_catch_error (dl-error.c:177)
>>> ==18162==    by 0x4000F4D: do_preload (rtld.c:815)
>>> ==18162==    by 0x4003294: dl_main (rtld.c:1629)
>>> ==18162==    by 0x4014D3D: _dl_sysdep_start (dl-sysdep.c:241)
>>> ==18162==    by 0x4004A35: _dl_start (rtld.c:331)
>>> ==18162==    by 0x4001217: ??? (in /lib64/ld-2.16.90.so)
>>> ==18162==    by 0xF: ???
>>> ==18162==  Address 0x546 is not stack'd, malloc'd or (recently) free'd
>>> ==18162==
>>> ==18162==
>>> ==18162== Process terminating with default action of signal 11 (SIGSEGV)
>>> ==18162==  Bad permissions for mapped region at address 0x546
>>> ==18162==    at 0x546: ???
>>> ==18162==    by 0x40155C8: __assert_fail (dl-minimal.c:206)
>>> ==18162==    by 0x4006E29: _dl_map_object_from_fd (get-dynamic-info.h:155)
>>> ==18162==    by 0x4007EDC: _dl_map_object (dl-load.c:2365)
>>> ==18162==    by 0x40013FD: map_doit (rtld.c:626)
>>> ==18162==    by 0x400E445: _dl_catch_error (dl-error.c:177)
>>> ==18162==    by 0x4000F4D: do_preload (rtld.c:815)
>>> ==18162==    by 0x4003294: dl_main (rtld.c:1629)
>>> ==18162==    by 0x4014D3D: _dl_sysdep_start (dl-sysdep.c:241)
>>> ==18162==    by 0x4004A35: _dl_start (rtld.c:331)
>>> ==18162==    by 0x4001217: ??? (in /lib64/ld-2.16.90.so)
>>> ==18162==    by 0xF: ???
>>>
>>
>> That is exactly the problem my patch tries to catch:
>>
>> [hjl@gnu-6 valgrind]$ readelf -d
>> /usr/lib64/valgrind/vgpreload_memcheck-x86-linux.so | grep FLAGS_1
>>  0x6ffffffb (FLAGS_1)                    Flags: INITFIRST INTERPOSE
>> [hjl@gnu-6 valgrind]$
>>
>> We don't support INTERPOSE.  We used to ignore it.  What should we
>> do when seeing an unsupported DF_1_XXX bit?
>
> Let's try to understand what's going on here...
>
> In Valgrind trunk Makefile.all.am has:
> ...
> # Baseline link flags for making vgpreload shared objects.
> #
> PRELOAD_LDFLAGS_COMMON_LINUX  = -nodefaultlibs -shared
> -Wl,-z,interpose,-z,initfirst
> ...
>
> Thus INTERPOSE is set *only* for Linux hosts.
>
> We've never supported INTERPOSE for Linux hosts and glibc has always
> ignored this DF_1_* flag.
>
> Therefore this is a valgrind bug, whose fix is the one-liner:
>
> Index: Makefile.all.am
> ===================================================================
> --- Makefile.all.am     (revision 13132)
> +++ Makefile.all.am     (working copy)
> @@ -195,7 +195,7 @@
>
>  # Baseline link flags for making vgpreload shared objects.
>  #
> -PRELOAD_LDFLAGS_COMMON_LINUX  = -nodefaultlibs -shared
> -Wl,-z,interpose,-z,initfirst
> +PRELOAD_LDFLAGS_COMMON_LINUX  = -nodefaultlibs -shared -Wl,-z,initfirst
>  PRELOAD_LDFLAGS_COMMON_DARWIN = -dynamic -dynamiclib -all_load
>
>  if VGCONF_PLATVARIANT_IS_ANDROID
> ---
>
> However, given that binutils accepted the flag in the first place
> without consideration for support from glibc it looks like asserting
> risks breaking programs at runtime (as opposed to build time if ld had
> rejected the unsupported flag).
>
> H.J.,
>
> What do you think about softening the assert to a _dl_debug_printf?
>
> That way we can diagnose the ignored flag by using LD_DEBUG=all?
>

Sounds good to me.

Thanks.


-- 
H.J.