This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] [RFC] Building for SuperH fails with gcc5/6


I did check this out in real hardware and I was about to check out the
make check results for any regression, but I got caught in some other
stuff.  I will sort this out this week.

On 01/03/2017 08:02, Alexey Neyman wrote:
> Ping?
> 
> 
> On 02/08/2017 09:56 AM, Adhemerval Zanella wrote:
>>
>> On 07/02/2017 17:59, Alexey Neyman wrote:
>>> Ping. With 2.25 out, can this patch be picked up?
>>>
>>> Regards,
>>> Alexey.
>>>
>>> On 02/04/2017 12:28 PM, Alexey Neyman wrote:
>>>> On 01/26/2017 03:52 AM, Adhemerval Zanella wrote:
>>>>> On 26/01/2017 05:03, Alexey Neyman wrote:
>>>>>> Hi,
>>>>>>
>>>>>> Build glibc for sh4-unknown-linux-gnu currently fails if one's using GCC5/6: in dl-conflict.c, the elf_machine_rela() function is called with NULL as its 3rd argument, sym. The implementation of that function in sysdeps/sh/dl-machine.h dereferences that pointer:
>>>>>>
>>>>>> const Elf32_Sym *const refsym = sym;
>>>>>> ...
>>>>>> if (map == &GL(dl_rtld_map))
>>>>>>     value -= map->l_addr + refsym->st_value + reloc->r_addend;
>>>>>>
>>>>>> GCC discovers a null pointer dereference, and in accordance with -fdelete-null-pointer-checks (which is enabled in -O2) replaces this code with a trap - which, as SH does not implement a trap pattern in GCC, evaluates to an abort() call. This abort() call pulls many more objects from libc_nonshared.a, eventually resulting in link failure due to multiple definitions for a number of symbols.
>>>>>>
>>>>> This issue is being tracked by PR#70216 "[SH] Implement __builtin_trap" [1].
>>>>>
>>>>> [1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70216
>>>> Thanks. Sorry, I wanted but forgot to reference this GCC issue.
>>>>>> As far as I see, the conditional before this code is always false in rtld: _dl_resolve_conflicts() is called with main_map as the first argument, not GL(_dl_rtld_map), but since that call is in yet another compilation unit, GCC does not know about it. Patch that wraps this conditional into !defined RESOLVE_CONFLICT_FIND_MAP attached.
>>>>>>
>>>>>> However, I thought the abort() call may be also inserted by GCC on other architectures that do not implement the trap pattern, so it may make sense to provide a dummy abort() in rtld that does not pull any other dependencies. An alternative patch for this approach also attached.
>>>>>>
>>>>>> And, of course, it is also possible to just add -fno-delete-null-pointer-checks to the compiler flags for dl-conflict.c on SH, even though it would unnecessarily pessimize the code.
>>>>>>
>>>>>> Comments?
>>>>> Current build-many-glibcs scripts uses both '-fno-isolate-erroneous-paths-dereference' and
>>>>> '-fno-isolate-erroneous-paths-attribute' on all SH targets and I am not sure which option
>>>>> is better in this specific situation.
>>>> This condition is covered by the former; the latter is not currently enabled by -O2. My bad, I referred to a wrong GCC option.
>>>>
>>>> However, enabling it in the whole build (as that script does) will pessimize all the code, even in locations where abort() can be called. If anything, it should be constrained to rtld or even to specific files in rtld.
>>>>>> 0001-Provide-a-minimal-abort-stub-for-rtld.patch
>>>>>>   From 279acf7a059f2d2296f690d7f2d886bd0e404f30 Mon Sep 17 00:00:00 2001
>>>>>> From: Alexey Neyman <stilor@att.net>
>>>>>> Date: Wed, 25 Jan 2017 21:46:53 -0800
>>>>>> Subject: [PATCH] sh: conditional is false in dl-conflict.c
>>>>>>
>>>>>> ... ifdef it out, so that it doesn't create a call to abort().
>>>>>>
>>>>>> Signed-off-by: Alexey Neyman <stilor@att.net>
>>>>>> ---
>>>>>>    sysdeps/sh/dl-machine.h | 2 +-
>>>>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/sysdeps/sh/dl-machine.h b/sysdeps/sh/dl-machine.h
>>>>>> index 449deea..2b468af 100644
>>>>>> --- a/sysdeps/sh/dl-machine.h
>>>>>> +++ b/sysdeps/sh/dl-machine.h
>>>>>> @@ -389,7 +389,7 @@ elf_machine_rela (struct link_map *map, const Elf32_Rela *reloc,
>>>>>>          break;
>>>>>>        case R_SH_DIR32:
>>>>>>          {
>>>>>> -#ifndef RTLD_BOOTSTRAP
>>>>>> +#if !defined RTLD_BOOTSTRAP && !defined RESOLVE_CONFLICT_FIND_MAP
>>>>>>           /* This is defined in rtld.c, but nowhere in the static
>>>>>>              libc.a; make the reference weak so static programs can
>>>>>>              still link.  This declaration cannot be done when
>>>>>> -- 2.9.3
>>>>>>
>>>>> This change seems a better alternative. Cross-compiling sh it shows no
>>>>> more build issues and I am assuming here this change would not cause
>>>>> any side effects in runtime.
>>>> I have no SH hardware either. If anyone could verify the resulting glibc, that would be great
>>>>
>>>> It shouldn't be much worse that GCC4 behavior, or the behavior with -fno-isolate-errnoneous-paths-dereference (which, if I am wrong and that conditional can execute from dl-conflict.c, would've dereferenced a NULL pointer).
>>>>> In any way, I would prefer to add a SH specific abort implementation
>>>>> to be used by the loader.  However it seems in PR#70216 comments that
>>>>> gcc developers are still defining which would be best instruction
>>>>> to use.
>>>> GCC5/6 do not have such a trap, and these versions are going to be in use for a while. GCC7 is around the corner and as it looks, isn't going to implement SH-specific trap. I think glibc should adapt to its shortcomings.
>>>>
>>>> Can the patch be picked up?
>>>>
>>>> Regards,
>>>> Alexey.
>> I will check it on a SH hardware I have access to see if this generates any runtime
>> side-effects.
>>
> 


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]