diff mbox

[COMMITTED] Fix missing __sqrtl_finite symbol in libm on sparc 32-bit.

Message ID 20160124.211545.53854406887189856.davem@davemloft.net
State New
Headers show

Commit Message

David Miller Jan. 25, 2016, 5:15 a.m. UTC 
* sysdeps/sparc/sparc32/fpu/e_sqrtl.c: New file.
	* sysdeps/sparc/sparc32/soft-fp/q_sqrt.c (__ieee754_sqrtl): Remove alias.
	* sysdeps/unix/sysv/linux/sparc/sparc32/libm.abilist: Add __sqrtl_finite.
---
 ChangeLog                                          |  4 +++
 sysdeps/sparc/sparc32/fpu/e_sqrtl.c                | 29 +++++++++++++++++++++-
 sysdeps/sparc/sparc32/soft-fp/q_sqrt.c             |  1 -
 sysdeps/unix/sysv/linux/sparc/sparc32/libm.abilist |  1 +
 4 files changed, 33 insertions(+), 2 deletions(-)

Comments

Andreas Schwab Jan. 25, 2016, 10:02 a.m. UTC | #1 
David Miller <davem@davemloft.net> writes:

> diff --git a/sysdeps/unix/sysv/linux/sparc/sparc32/libm.abilist b/sysdeps/unix/sysv/linux/sparc/sparc32/libm.abilist
> index d50ef4a..6b31a54 100644
> --- a/sysdeps/unix/sysv/linux/sparc/sparc32/libm.abilist
> +++ b/sysdeps/unix/sysv/linux/sparc/sparc32/libm.abilist
> @@ -376,6 +376,7 @@ GLIBC_2.15 __sinhf_finite F
>  GLIBC_2.15 __sinhl_finite F
>  GLIBC_2.15 __sqrt_finite F
>  GLIBC_2.15 __sqrtf_finite F
> +GLIBC_2.15 __sqrtl_finite F
>  GLIBC_2.15 __y0_finite F
>  GLIBC_2.15 __y0f_finite F
>  GLIBC_2.15 __y0l_finite F

You cannot change the ABI after the release.

Andreas.
David Miller Jan. 25, 2016, 5:53 p.m. UTC | #2 
From: Andreas Schwab <schwab@suse.de>
Date: Mon, 25 Jan 2016 11:02:27 +0100

> David Miller <davem@davemloft.net> writes:
> 
>> diff --git a/sysdeps/unix/sysv/linux/sparc/sparc32/libm.abilist b/sysdeps/unix/sysv/linux/sparc/sparc32/libm.abilist
>> index d50ef4a..6b31a54 100644
>> --- a/sysdeps/unix/sysv/linux/sparc/sparc32/libm.abilist
>> +++ b/sysdeps/unix/sysv/linux/sparc/sparc32/libm.abilist
>> @@ -376,6 +376,7 @@ GLIBC_2.15 __sinhf_finite F
>>  GLIBC_2.15 __sinhl_finite F
>>  GLIBC_2.15 __sqrt_finite F
>>  GLIBC_2.15 __sqrtf_finite F
>> +GLIBC_2.15 __sqrtl_finite F
>>  GLIBC_2.15 __y0_finite F
>>  GLIBC_2.15 __y0f_finite F
>>  GLIBC_2.15 __y0l_finite F
> 
> You cannot change the ABI after the release.

So do I have to add it to the current GLIBC version?
Adhemerval Zanella Jan. 25, 2016, 6 p.m. UTC | #3 
On 25-01-2016 15:53, David Miller wrote:
> From: Andreas Schwab <schwab@suse.de>
> Date: Mon, 25 Jan 2016 11:02:27 +0100
> 
>> David Miller <davem@davemloft.net> writes:
>>
>>> diff --git a/sysdeps/unix/sysv/linux/sparc/sparc32/libm.abilist b/sysdeps/unix/sysv/linux/sparc/sparc32/libm.abilist
>>> index d50ef4a..6b31a54 100644
>>> --- a/sysdeps/unix/sysv/linux/sparc/sparc32/libm.abilist
>>> +++ b/sysdeps/unix/sysv/linux/sparc/sparc32/libm.abilist
>>> @@ -376,6 +376,7 @@ GLIBC_2.15 __sinhf_finite F
>>>  GLIBC_2.15 __sinhl_finite F
>>>  GLIBC_2.15 __sqrt_finite F
>>>  GLIBC_2.15 __sqrtf_finite F
>>> +GLIBC_2.15 __sqrtl_finite F
>>>  GLIBC_2.15 __y0_finite F
>>>  GLIBC_2.15 __y0f_finite F
>>>  GLIBC_2.15 __y0l_finite F
>>
>> You cannot change the ABI after the release.
> 
> So do I have to add it to the current GLIBC version?
> 

Yes.
David Miller Jan. 25, 2016, 6:02 p.m. UTC | #4 
From: David Miller <davem@davemloft.net>
Date: Mon, 25 Jan 2016 09:53:59 -0800 (PST)

> From: Andreas Schwab <schwab@suse.de>
> Date: Mon, 25 Jan 2016 11:02:27 +0100
> 
>> David Miller <davem@davemloft.net> writes:
>> 
>>> diff --git a/sysdeps/unix/sysv/linux/sparc/sparc32/libm.abilist b/sysdeps/unix/sysv/linux/sparc/sparc32/libm.abilist
>>> index d50ef4a..6b31a54 100644
>>> --- a/sysdeps/unix/sysv/linux/sparc/sparc32/libm.abilist
>>> +++ b/sysdeps/unix/sysv/linux/sparc/sparc32/libm.abilist
>>> @@ -376,6 +376,7 @@ GLIBC_2.15 __sinhf_finite F
>>>  GLIBC_2.15 __sinhl_finite F
>>>  GLIBC_2.15 __sqrt_finite F
>>>  GLIBC_2.15 __sqrtf_finite F
>>> +GLIBC_2.15 __sqrtl_finite F
>>>  GLIBC_2.15 __y0_finite F
>>>  GLIBC_2.15 __y0f_finite F
>>>  GLIBC_2.15 __y0l_finite F
>> 
>> You cannot change the ABI after the release.
> 
> So do I have to add it to the current GLIBC version?

And the reason I am asking this is because this symbol is missing from previous
GLIBC versions and just as I noticed when running the testsuite, probably
prevents successful linking of some applications.

What if I want to add this symbol in a backport?
David Miller Jan. 25, 2016, 6:02 p.m. UTC | #5 
From: Adhemerval Zanella <adhemerval.zanella@linaro.org>
Date: Mon, 25 Jan 2016 16:00:29 -0200

> 
> 
> On 25-01-2016 15:53, David Miller wrote:
>> From: Andreas Schwab <schwab@suse.de>
>> Date: Mon, 25 Jan 2016 11:02:27 +0100
>> 
>>> David Miller <davem@davemloft.net> writes:
>>>
>>>> diff --git a/sysdeps/unix/sysv/linux/sparc/sparc32/libm.abilist b/sysdeps/unix/sysv/linux/sparc/sparc32/libm.abilist
>>>> index d50ef4a..6b31a54 100644
>>>> --- a/sysdeps/unix/sysv/linux/sparc/sparc32/libm.abilist
>>>> +++ b/sysdeps/unix/sysv/linux/sparc/sparc32/libm.abilist
>>>> @@ -376,6 +376,7 @@ GLIBC_2.15 __sinhf_finite F
>>>>  GLIBC_2.15 __sinhl_finite F
>>>>  GLIBC_2.15 __sqrt_finite F
>>>>  GLIBC_2.15 __sqrtf_finite F
>>>> +GLIBC_2.15 __sqrtl_finite F
>>>>  GLIBC_2.15 __y0_finite F
>>>>  GLIBC_2.15 __y0f_finite F
>>>>  GLIBC_2.15 __y0l_finite F
>>>
>>> You cannot change the ABI after the release.
>> 
>> So do I have to add it to the current GLIBC version?
>> 
> 
> Yes.

What about backports?  This symbol is missing in releases back to 2.15 and I'd
like to add it there too.  It prevents applications from linking.
Joseph Myers Jan. 25, 2016, 6:04 p.m. UTC | #6 
On Mon, 25 Jan 2016, David Miller wrote:

> >> You cannot change the ABI after the release.
> > 
> > So do I have to add it to the current GLIBC version?
> 
> And the reason I am asking this is because this symbol is missing from previous
> GLIBC versions and just as I noticed when running the testsuite, probably
> prevents successful linking of some applications.
> 
> What if I want to add this symbol in a backport?

Then your backport needs to add the GLIBC_2.23 symbol version (for older 
versions, that includes updating Versions.def, as new versions were less 
automatic than they are now).  That works fine, from experience.
David Miller Jan. 25, 2016, 6:10 p.m. UTC | #7 
From: Joseph Myers <joseph@codesourcery.com>
Date: Mon, 25 Jan 2016 18:04:34 +0000

> On Mon, 25 Jan 2016, David Miller wrote:
> 
>> >> You cannot change the ABI after the release.
>> > 
>> > So do I have to add it to the current GLIBC version?
>> 
>> And the reason I am asking this is because this symbol is missing from previous
>> GLIBC versions and just as I noticed when running the testsuite, probably
>> prevents successful linking of some applications.
>> 
>> What if I want to add this symbol in a backport?
> 
> Then your backport needs to add the GLIBC_2.23 symbol version (for older 
> versions, that includes updating Versions.def, as new versions were less 
> automatic than they are now).  That works fine, from experience.

Aha, great, that's exactly what I wanted to know.

Thanks!
Florian Weimer Jan. 25, 2016, 8:01 p.m. UTC | #8 
* Joseph Myers:

> On Mon, 25 Jan 2016, David Miller wrote:
>
>> >> You cannot change the ABI after the release.
>> > 
>> > So do I have to add it to the current GLIBC version?
>> 
>> And the reason I am asking this is because this symbol is missing
>> from previous
>> GLIBC versions and just as I noticed when running the testsuite, probably
>> prevents successful linking of some applications.
>> 
>> What if I want to add this symbol in a backport?
>
> Then your backport needs to add the GLIBC_2.23 symbol version (for older 
> versions, that includes updating Versions.def, as new versions were less 
> automatic than they are now).  That works fine, from experience.

It breaks the RPM dependency generator, unless you add *all*
GLIBC_2.23 symbols.  Probably not relevant to sparc, but for other
architectures, it's something to keep in mind.

I'm trying to convince Carlos that we need to fix the dependency
generator, but I'm not quite there yet. :)
Joseph Myers Jan. 25, 2016, 10:44 p.m. UTC | #9 
On Mon, 25 Jan 2016, Florian Weimer wrote:

> * Joseph Myers:
> 
> > On Mon, 25 Jan 2016, David Miller wrote:
> >
> >> >> You cannot change the ABI after the release.
> >> > 
> >> > So do I have to add it to the current GLIBC version?
> >> 
> >> And the reason I am asking this is because this symbol is missing
> >> from previous
> >> GLIBC versions and just as I noticed when running the testsuite, probably
> >> prevents successful linking of some applications.
> >> 
> >> What if I want to add this symbol in a backport?
> >
> > Then your backport needs to add the GLIBC_2.23 symbol version (for older 
> > versions, that includes updating Versions.def, as new versions were less 
> > automatic than they are now).  That works fine, from experience.
> 
> It breaks the RPM dependency generator, unless you add *all*
> GLIBC_2.23 symbols.  Probably not relevant to sparc, but for other
> architectures, it's something to keep in mind.

When I say it works, I mean (a) the glibc with the backport builds OK and 
passes its tests, and (b) binaries linked with that glibc that don't use 
the new symbols work fine with glibc of the same version without the 
backport, while binaries linked with it that do use the new symbols work 
fine with the glibc version (2.23 in this case) that introduced them 
officially.

I don't think such a backport introducing new symbols should go on any 
official glibc release branch, however.
Andreas Schwab Jan. 26, 2016, 9:11 a.m. UTC | #10 
Joseph Myers <joseph@codesourcery.com> writes:

> When I say it works, I mean (a) the glibc with the backport builds OK and 
> passes its tests, and (b) binaries linked with that glibc that don't use 
> the new symbols work fine with glibc of the same version without the 
> backport, while binaries linked with it that do use the new symbols work 
> fine with the glibc version (2.23 in this case) that introduced them 
> officially.

But binaries linked against the real 2.23 version will eventually fail
*at runtime* (not at load time) if they use any other new 2.23 symbol.

Andreas.
diff mbox

Patch

diff --git a/ChangeLog b/ChangeLog
index 50f2880..6c21184 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,9 @@ 
 2016-01-24  David S. Miller  <davem@davemloft.net>
 
+	* sysdeps/sparc/sparc32/fpu/e_sqrtl.c: New file.
+	* sysdeps/sparc/sparc32/soft-fp/q_sqrt.c (__ieee754_sqrtl): Remove alias.
+	* sysdeps/unix/sysv/linux/sparc/sparc32/libm.abilist: Add __sqrtl_finite.
+
 	* sysdeps/sparc/fpu/libm-test-ulps: Update.
 
 2016-01-25  Maciej W. Rozycki  <macro@imgtec.com>
diff --git a/sysdeps/sparc/sparc32/fpu/e_sqrtl.c b/sysdeps/sparc/sparc32/fpu/e_sqrtl.c
index 3c3acfe..46ad861 100644
--- a/sysdeps/sparc/sparc32/fpu/e_sqrtl.c
+++ b/sysdeps/sparc/sparc32/fpu/e_sqrtl.c
@@ -1 +1,28 @@ 
-/* __ieee754_sqrtl is defined in q_sqrt.c.  */
+/* Long double square root, sparc32 version.
+   Copyright (C) 2016 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+#include <math.h>
+
+extern long double _Q_sqrt(const long double a);
+
+long double
+__ieee754_sqrtl (long double x)
+{
+  return _Q_sqrt (x);
+}
+strong_alias (__ieee754_sqrtl, __sqrtl_finite)
diff --git a/sysdeps/sparc/sparc32/soft-fp/q_sqrt.c b/sysdeps/sparc/sparc32/soft-fp/q_sqrt.c
index 0eb43a2..999398e 100644
--- a/sysdeps/sparc/sparc32/soft-fp/q_sqrt.c
+++ b/sysdeps/sparc/sparc32/soft-fp/q_sqrt.c
@@ -35,4 +35,3 @@  long double _Q_sqrt(const long double a)
   FP_HANDLE_EXCEPTIONS;
   return c;
 }
-strong_alias (_Q_sqrt, __ieee754_sqrtl);
diff --git a/sysdeps/unix/sysv/linux/sparc/sparc32/libm.abilist b/sysdeps/unix/sysv/linux/sparc/sparc32/libm.abilist
index d50ef4a..6b31a54 100644
--- a/sysdeps/unix/sysv/linux/sparc/sparc32/libm.abilist
+++ b/sysdeps/unix/sysv/linux/sparc/sparc32/libm.abilist
@@ -376,6 +376,7 @@  GLIBC_2.15 __sinhf_finite F
 GLIBC_2.15 __sinhl_finite F
 GLIBC_2.15 __sqrt_finite F
 GLIBC_2.15 __sqrtf_finite F
+GLIBC_2.15 __sqrtl_finite F
 GLIBC_2.15 __y0_finite F
 GLIBC_2.15 __y0f_finite F
 GLIBC_2.15 __y0l_finite F