[11/15] math: Fix acos template for arguments greater than 1
Checks
Context |
Check |
Description |
redhat-pt-bot/TryBot-apply_patch |
success
|
Patch applied to master at the time it was sent
|
linaro-tcwg-bot/tcwg_glibc_build--master-arm |
success
|
Testing passed
|
linaro-tcwg-bot/tcwg_glibc_build--master-aarch64 |
success
|
Testing passed
|
linaro-tcwg-bot/tcwg_glibc_check--master-arm |
success
|
Testing passed
|
linaro-tcwg-bot/tcwg_glibc_check--master-aarch64 |
success
|
Testing passed
|
Commit Message
The template is used by some ABsI for static build, and it fails set
the expected floating exceptions if the argument is outside of the
range (on x86_64 this triggers an overflow calculation in
__ieee754_acos).
Checked on x86_64-linux-gnu.
---
math/Makefile | 1 +
math/w_acos_template.c | 9 +++++++--
2 files changed, 8 insertions(+), 2 deletions(-)
Comments
On Wed, 27 Mar 2024, Adhemerval Zanella wrote:
> The template is used by some ABsI for static build, and it fails set
> the expected floating exceptions if the argument is outside of the
> range (on x86_64 this triggers an overflow calculation in
> __ieee754_acos).
Patches 11 through 15 all seem incorrect; it's the responsibility of the
__ieee754_* functions to raise the correct exceptions, not of the
wrappers. Please make sure you don't have a compiler with a bug like
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95115 miscompiling those
__ieee754_* functions.
On 27/03/24 14:14, Joseph Myers wrote:
> On Wed, 27 Mar 2024, Adhemerval Zanella wrote:
>
>> The template is used by some ABsI for static build, and it fails set
>> the expected floating exceptions if the argument is outside of the
>> range (on x86_64 this triggers an overflow calculation in
>> __ieee754_acos).
>
> Patches 11 through 15 all seem incorrect; it's the responsibility of the
> __ieee754_* functions to raise the correct exceptions, not of the
> wrappers. Please make sure you don't have a compiler with a bug like
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95115 miscompiling those
> __ieee754_* functions.
>
The failures are both:
math/test-float64x-acos-static
math/test-ldouble-acos-static
$ cat math/test-ldouble-acos-static.out
testing long double (without inline functions)
Failure: acos (max_value): Exception "Overflow" set
Failure: acos (-max_value): Exception "Overflow" set
Failure: acos_downward (max_value): Exception "Overflow" set
Failure: acos_downward (-max_value): Exception "Overflow" set
Failure: acos_towardzero (max_value): Exception "Overflow" set
Failure: acos_towardzero (-max_value): Exception "Overflow" set
Failure: acos_upward (max_value): Exception "Overflow" set
Failure: acos_upward (-max_value): Exception "Overflow" set
Test suite completed:
452 test cases plus 448 tests for exception flags and
448 tests for errno executed.
8 errors occurred.
And I think it is unrelated to gcc PR95115 because x86_64/i686 will use
and specific sysdeps/i386/fpu/e_acosl.c that explicit does not handle this
input case for overflow exceptions. For shared build this case is
handle by w_acosl_compat.c:
if (__builtin_expect (isgreater (fabsl (x), 1.0L), 0)
&& _LIB_VERSION != _IEEE_)
{
/* acos(|x|>1) */
feraiseexcept (FE_INVALID);
return __kernel_standard_l (x, x, 201);
}
And that's why I though following the same logic on template would be
better. But I think maybe we should fix on x86_64 implementation instead.
On Wed, 27 Mar 2024, Adhemerval Zanella Netto wrote:
> And I think it is unrelated to gcc PR95115 because x86_64/i686 will use
> and specific sysdeps/i386/fpu/e_acosl.c that explicit does not handle this
> input case for overflow exceptions. For shared build this case is
> handle by w_acosl_compat.c:
>
> if (__builtin_expect (isgreater (fabsl (x), 1.0L), 0)
> && _LIB_VERSION != _IEEE_)
> {
> /* acos(|x|>1) */
> feraiseexcept (FE_INVALID);
> return __kernel_standard_l (x, x, 201);
> }
The compat code is dealing with the possibility of SVID exceptions, which
isn't relevant here.
> And that's why I though following the same logic on template would be
> better. But I think maybe we should fix on x86_64 implementation instead.
Yes, we should fix the x86_64 implementation.
Such issues in dbl-64, flt-32 or ldbl-128 sources would largely have been
fixed (modulo compiler bugs) when we started adding new architectures
after new architectures stopped using the compat wrappers - but any issues
for ldbl-128ibm, ldbl-96 or architecture-specific sources wouldn't have
been detected then.
On 27/03/24 16:04, Joseph Myers wrote:
> On Wed, 27 Mar 2024, Adhemerval Zanella Netto wrote:
>
>> And I think it is unrelated to gcc PR95115 because x86_64/i686 will use
>> and specific sysdeps/i386/fpu/e_acosl.c that explicit does not handle this
>> input case for overflow exceptions. For shared build this case is
>> handle by w_acosl_compat.c:
>>
>> if (__builtin_expect (isgreater (fabsl (x), 1.0L), 0)
>> && _LIB_VERSION != _IEEE_)
>> {
>> /* acos(|x|>1) */
>> feraiseexcept (FE_INVALID);
>> return __kernel_standard_l (x, x, 201);
>> }
>
> The compat code is dealing with the possibility of SVID exceptions, which
> isn't relevant here.
I meant that __ieee754_acosl in being called in both code paths, but
the compat code returns early without calling __ieee754_acosl for the
possible problematic code path.
>
>> And that's why I though following the same logic on template would be
>> better. But I think maybe we should fix on x86_64 implementation instead.
>
> Yes, we should fix the x86_64 implementation.
>
> Such issues in dbl-64, flt-32 or ldbl-128 sources would largely have been
> fixed (modulo compiler bugs) when we started adding new architectures
> after new architectures stopped using the compat wrappers - but any issues
> for ldbl-128ibm, ldbl-96 or architecture-specific sources wouldn't have
> been detected then.
>
I will drop this change and rather focus on the original issues on missing
symbols. I will open some bug against the x86 implementation so we keep
track of these issues with static linkage.
@@ -368,6 +368,7 @@ $(libm-test-c-narrow-obj): $(objpfx)libm-test%.c: libm-test%.inc \
libm-test-funcs-auto-static = \
+ acos \
exp10 \
# libm-test-funcs-auto-static
libm-test-funcs-noauto-static = \
@@ -30,8 +30,13 @@ FLOAT
M_DECL_FUNC (__acos) (FLOAT x)
{
if (__glibc_unlikely (isgreater (M_FABS (x), M_LIT (1.0))))
- /* Domain error: acos(|x|>1). */
- __set_errno (EDOM);
+ {
+ /* Domain error: acos(|x|>1). */
+ __feraiseexcept (FE_INVALID);
+ __set_errno (EDOM);
+ return __builtin_nanl ("");
+ }
+
return M_SUF (__ieee754_acos) (x);
}
declare_mgen_alias (__acos, acos)