build-many-glibcs: Remove no_isolate from SH config
Commit Message
Now with d40dbe7 SH build does not require more the no_isolate gcc
options to correct build glibc (since SH build now does not generate
a trap anymore). This patch removes the unrequired options from
SH config.
Checked with a build for sh3-linux-gnu, sh3eb-linux-gnu, sh4-linux-gnu,
and sh4eb-linux-gnu.
* scripts/build-many-glibcs.py (Context.add_all_configs): Remove
no_isolate usage for SH.
---
ChangeLog | 5 +++++
scripts/build-many-glibcs.py | 23 ++++++++---------------
2 files changed, 13 insertions(+), 15 deletions(-)
Comments
On Mon, 13 Mar 2017, Adhemerval Zanella wrote:
> Now with d40dbe7 SH build does not require more the no_isolate gcc
> options to correct build glibc (since SH build now does not generate
> a trap anymore). This patch removes the unrequired options from
> SH config.
no_isolate is *only* used for SH, so should be removed completely if
removed from use for SH.
On 13/03/2017 13:40, Joseph Myers wrote:
> On Mon, 13 Mar 2017, Adhemerval Zanella wrote:
>
>> Now with d40dbe7 SH build does not require more the no_isolate gcc
>> options to correct build glibc (since SH build now does not generate
>> a trap anymore). This patch removes the unrequired options from
>> SH config.
>
> no_isolate is *only* used for SH, so should be removed completely if
> removed from use for SH.
>
Right, I thought about letting the options for possible other future
architectures that might face the same issue. I will remove it.
On Mon, 13 Mar 2017, Adhemerval Zanella wrote:
> On 13/03/2017 13:40, Joseph Myers wrote:
> > On Mon, 13 Mar 2017, Adhemerval Zanella wrote:
> >
> >> Now with d40dbe7 SH build does not require more the no_isolate gcc
> >> options to correct build glibc (since SH build now does not generate
> >> a trap anymore). This patch removes the unrequired options from
> >> SH config.
> >
> > no_isolate is *only* used for SH, so should be removed completely if
> > removed from use for SH.
> >
>
> Right, I thought about letting the options for possible other future
> architectures that might face the same issue. I will remove it.
It seems this broke the build for SH with GCC mainline (same
multiple-definitions errors that are symptoms of needing these options);
please revert.
https://sourceware.org/ml/libc-testresults/2017-q1/msg00248.html
On 13/03/2017 20:17, Joseph Myers wrote:
> On Mon, 13 Mar 2017, Adhemerval Zanella wrote:
>
>> On 13/03/2017 13:40, Joseph Myers wrote:
>>> On Mon, 13 Mar 2017, Adhemerval Zanella wrote:
>>>
>>>> Now with d40dbe7 SH build does not require more the no_isolate gcc
>>>> options to correct build glibc (since SH build now does not generate
>>>> a trap anymore). This patch removes the unrequired options from
>>>> SH config.
>>>
>>> no_isolate is *only* used for SH, so should be removed completely if
>>> removed from use for SH.
>>>
>>
>> Right, I thought about letting the options for possible other future
>> architectures that might face the same issue. I will remove it.
>
> It seems this broke the build for SH with GCC mainline (same
> multiple-definitions errors that are symptoms of needing these options);
> please revert.
>
> https://sourceware.org/ml/libc-testresults/2017-q1/msg00248.html
>
Right, I will revert and check what is not failing with default
version.
On 13/03/2017 20:44, Adhemerval Zanella wrote:
>
>
> On 13/03/2017 20:17, Joseph Myers wrote:
>> On Mon, 13 Mar 2017, Adhemerval Zanella wrote:
>>
>>> On 13/03/2017 13:40, Joseph Myers wrote:
>>>> On Mon, 13 Mar 2017, Adhemerval Zanella wrote:
>>>>
>>>>> Now with d40dbe7 SH build does not require more the no_isolate gcc
>>>>> options to correct build glibc (since SH build now does not generate
>>>>> a trap anymore). This patch removes the unrequired options from
>>>>> SH config.
>>>>
>>>> no_isolate is *only* used for SH, so should be removed completely if
>>>> removed from use for SH.
>>>>
>>>
>>> Right, I thought about letting the options for possible other future
>>> architectures that might face the same issue. I will remove it.
>>
>> It seems this broke the build for SH with GCC mainline (same
>> multiple-definitions errors that are symptoms of needing these options);
>> please revert.
>>
>> https://sourceware.org/ml/libc-testresults/2017-q1/msg00248.html
>>
>
> Right, I will revert and check what is not failing with default
> version.
>
Joseph, I tracked down the issue and it is due the snippet:
sysdeps/wordsize-32/divdi3.c:
133 else
134 {
135 /* qq = NN / 0d */
136
137 if (d0 == 0)
138 d0 = 1 / d0; /* Divide intentionally by zero. */
GCC 6.3 and older lowers it to a software division call (__sdivsi3_i4i)
while GCC 7.0 with -fisolate-erroneous-paths-dereference found the
undefined behaviour and transform to a trap and subsequent abort call.
So I think we have some options:
1. Revert the patch and make SH toolchain compile with
-fno-isolate-erroneous-paths-dereference (there is no need for
add -fno-isolate-erroneous-paths-attribute).
2. Build divdi3 for SH explicit with -fno-isolate-erroneous-paths-attribute.
3. Port libgcc r205444 with add an __udivmoddi4 implementation for
architectures that do not have division instruction (which does not
generate a trap for division by 0).
I would prefer either 2 or 3.
On Tue, 14 Mar 2017, Adhemerval Zanella wrote:
> So I think we have some options:
>
> 1. Revert the patch and make SH toolchain compile with
> -fno-isolate-erroneous-paths-dereference (there is no need for
> add -fno-isolate-erroneous-paths-attribute).
>
> 2. Build divdi3 for SH explicit with -fno-isolate-erroneous-paths-attribute.
>
> 3. Port libgcc r205444 with add an __udivmoddi4 implementation for
> architectures that do not have division instruction (which does not
> generate a trap for division by 0).
>
> I would prefer either 2 or 3.
This code is present in glibc purely for compat symbols, which are only
exported on a limited number of architectures, which do not include SH.
Thus, I'd favour arranging for the code only to be built at all for those
architectures (i386 m68k powerpc32 s390-32; ia64 also exports these
symbols, but from a separate implementation), and not for any other 32-bit
architecture (and if such architectures need these functions in glibc,
they will get them from libgcc.a). That would of course need
build-many-glibcs tests to make sure it doesn't break anything.
On 14/03/2017 15:09, Joseph Myers wrote:
> On Tue, 14 Mar 2017, Adhemerval Zanella wrote:
>
>> So I think we have some options:
>>
>> 1. Revert the patch and make SH toolchain compile with
>> -fno-isolate-erroneous-paths-dereference (there is no need for
>> add -fno-isolate-erroneous-paths-attribute).
>>
>> 2. Build divdi3 for SH explicit with -fno-isolate-erroneous-paths-attribute.
>>
>> 3. Port libgcc r205444 with add an __udivmoddi4 implementation for
>> architectures that do not have division instruction (which does not
>> generate a trap for division by 0).
>>
>> I would prefer either 2 or 3.
>
> This code is present in glibc purely for compat symbols, which are only
> exported on a limited number of architectures, which do not include SH.
> Thus, I'd favour arranging for the code only to be built at all for those
> architectures (i386 m68k powerpc32 s390-32; ia64 also exports these
> symbols, but from a separate implementation), and not for any other 32-bit
> architecture (and if such architectures need these functions in glibc,
> they will get them from libgcc.a). That would of course need
> build-many-glibcs tests to make sure it doesn't break anything.
>
Right, I will follow this idea then.
On Tue, 14 Mar 2017, Adhemerval Zanella wrote:
> > This code is present in glibc purely for compat symbols, which are only
> > exported on a limited number of architectures, which do not include SH.
> > Thus, I'd favour arranging for the code only to be built at all for those
> > architectures (i386 m68k powerpc32 s390-32; ia64 also exports these
> > symbols, but from a separate implementation), and not for any other 32-bit
> > architecture (and if such architectures need these functions in glibc,
> > they will get them from libgcc.a). That would of course need
> > build-many-glibcs tests to make sure it doesn't break anything.
> >
>
> Right, I will follow this idea then.
Note that this code goes along with the symbol-hacks.h code that redirects
__divdi3 to __divdi3_internal etc. - that code will also need to be
disabled except for the four architectures that actually export code from
divdi3.c.
On 14/03/2017 19:04, Joseph Myers wrote:
> On Tue, 14 Mar 2017, Adhemerval Zanella wrote:
>
>>> This code is present in glibc purely for compat symbols, which are only
>>> exported on a limited number of architectures, which do not include SH.
>>> Thus, I'd favour arranging for the code only to be built at all for those
>>> architectures (i386 m68k powerpc32 s390-32; ia64 also exports these
>>> symbols, but from a separate implementation), and not for any other 32-bit
>>> architecture (and if such architectures need these functions in glibc,
>>> they will get them from libgcc.a). That would of course need
>>> build-many-glibcs tests to make sure it doesn't break anything.
>>>
>>
>> Right, I will follow this idea then.
>
> Note that this code goes along with the symbol-hacks.h code that redirects
> __divdi3 to __divdi3_internal etc. - that code will also need to be
> disabled except for the four architectures that actually export code from
> divdi3.c.
>
Yes, this very issue showed itself on a architecture that is not suppose
to export these symbols.
@@ -161,7 +161,8 @@ class Context(object):
"""Add all known glibc build configurations."""
# On architectures missing __builtin_trap support, these
# options may be needed as a workaround; see
- # <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70216> for SH.
+ # <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70216> for SH
+ # (although since d40dbe7 SH does not generate trap instruction).
no_isolate = ('-fno-isolate-erroneous-paths-dereference'
' -fno-isolate-erroneous-paths-attribute')
self.add_config(arch='aarch64',
@@ -337,31 +338,23 @@ class Context(object):
glibcs=[{},
{'arch': 's390', 'ccopts': '-m31'}])
self.add_config(arch='sh3',
- os_name='linux-gnu',
- glibcs=[{'ccopts': no_isolate}])
+ os_name='linux-gnu')
self.add_config(arch='sh3eb',
- os_name='linux-gnu',
- glibcs=[{'ccopts': no_isolate}])
+ os_name='linux-gnu')
self.add_config(arch='sh4',
- os_name='linux-gnu',
- glibcs=[{'ccopts': no_isolate}])
+ os_name='linux-gnu')
self.add_config(arch='sh4eb',
- os_name='linux-gnu',
- glibcs=[{'ccopts': no_isolate}])
+ os_name='linux-gnu')
self.add_config(arch='sh4',
os_name='linux-gnu',
variant='soft',
gcc_cfg=['--without-fp'],
- glibcs=[{'variant': 'soft',
- 'cfg': ['--without-fp'],
- 'ccopts': no_isolate}])
+ glibcs=[{'variant': 'soft', 'cfg': ['--without-fp']}])
self.add_config(arch='sh4eb',
os_name='linux-gnu',
variant='soft',
gcc_cfg=['--without-fp'],
- glibcs=[{'variant': 'soft',
- 'cfg': ['--without-fp'],
- 'ccopts': no_isolate}])
+ glibcs=[{'variant': 'soft', 'cfg': ['--without-fp']}])
self.add_config(arch='sparc64',
os_name='linux-gnu',
glibcs=[{},