* [PATCH] s390: Fix undefined behaviour in feenableexcept, fedisableexcept [BZ #30960]
@ 2023-10-18 13:08 Stefan Liebler
2023-10-19 8:48 ` Andreas Schwab
0 siblings, 1 reply; 3+ messages in thread
From: Stefan Liebler @ 2023-10-18 13:08 UTC (permalink / raw)
To: libc-alpha; +Cc: Stefan Liebler
If feenableexcept or fedisableexcept gets excepts=FE_INVALID=0x80
as input, we have a signed left shift: 0x80 << 24 which is not
representable as int and thus is undefined behaviour according to
C standard.
This patch casts excepts as unsigned int before shifting, which is
defined.
For me, the observed undefined behaviour is that the shift is done
with "unsigned"-instructions, which is exactly what we want.
Furthermore, I don't get any exception-flags.
After the fix, the code is using the same instruction sequence as
before.
---
sysdeps/s390/fpu/fedisblxcpt.c | 3 ++-
sysdeps/s390/fpu/feenablxcpt.c | 3 ++-
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/sysdeps/s390/fpu/fedisblxcpt.c b/sysdeps/s390/fpu/fedisblxcpt.c
index 728f103f43..84b2c5e64a 100644
--- a/sysdeps/s390/fpu/fedisblxcpt.c
+++ b/sysdeps/s390/fpu/fedisblxcpt.c
@@ -26,7 +26,8 @@ fedisableexcept (int excepts)
_FPU_GETCW (temp);
old_exc = (temp & FPC_EXCEPTION_MASK) >> FPC_EXCEPTION_MASK_SHIFT;
- new_flags = (temp & (~((excepts & FE_ALL_EXCEPT) << FPC_EXCEPTION_MASK_SHIFT)));
+ new_flags = (temp & (~(((unsigned int) excepts & FE_ALL_EXCEPT)
+ << FPC_EXCEPTION_MASK_SHIFT)));
_FPU_SETCW (new_flags);
return old_exc;
diff --git a/sysdeps/s390/fpu/feenablxcpt.c b/sysdeps/s390/fpu/feenablxcpt.c
index 0807e610a2..76d25316f4 100644
--- a/sysdeps/s390/fpu/feenablxcpt.c
+++ b/sysdeps/s390/fpu/feenablxcpt.c
@@ -26,7 +26,8 @@ feenableexcept (int excepts)
_FPU_GETCW (temp);
old_exc = (temp & FPC_EXCEPTION_MASK) >> FPC_EXCEPTION_MASK_SHIFT;
- new_flags = (temp | ((excepts & FE_ALL_EXCEPT) << FPC_EXCEPTION_MASK_SHIFT));
+ new_flags = (temp | ((unsigned int) (excepts & FE_ALL_EXCEPT)
+ << FPC_EXCEPTION_MASK_SHIFT));
_FPU_SETCW (new_flags);
return old_exc;
--
2.41.0
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] s390: Fix undefined behaviour in feenableexcept, fedisableexcept [BZ #30960]
2023-10-18 13:08 [PATCH] s390: Fix undefined behaviour in feenableexcept, fedisableexcept [BZ #30960] Stefan Liebler
@ 2023-10-19 8:48 ` Andreas Schwab
2023-10-19 12:31 ` Stefan Liebler
0 siblings, 1 reply; 3+ messages in thread
From: Andreas Schwab @ 2023-10-19 8:48 UTC (permalink / raw)
To: Stefan Liebler; +Cc: libc-alpha
On Okt 18 2023, Stefan Liebler wrote:
> diff --git a/sysdeps/s390/fpu/fedisblxcpt.c b/sysdeps/s390/fpu/fedisblxcpt.c
> index 728f103f43..84b2c5e64a 100644
> --- a/sysdeps/s390/fpu/fedisblxcpt.c
> +++ b/sysdeps/s390/fpu/fedisblxcpt.c
> @@ -26,7 +26,8 @@ fedisableexcept (int excepts)
>
> _FPU_GETCW (temp);
> old_exc = (temp & FPC_EXCEPTION_MASK) >> FPC_EXCEPTION_MASK_SHIFT;
> - new_flags = (temp & (~((excepts & FE_ALL_EXCEPT) << FPC_EXCEPTION_MASK_SHIFT)));
> + new_flags = (temp & (~(((unsigned int) excepts & FE_ALL_EXCEPT)
> + << FPC_EXCEPTION_MASK_SHIFT)));
There is a redundant pair of parens around the ~ operator that makes the
whole expression more difficult to read. Please consider removing it.
> _FPU_SETCW (new_flags);
>
> return old_exc;
> diff --git a/sysdeps/s390/fpu/feenablxcpt.c b/sysdeps/s390/fpu/feenablxcpt.c
> index 0807e610a2..76d25316f4 100644
> --- a/sysdeps/s390/fpu/feenablxcpt.c
> +++ b/sysdeps/s390/fpu/feenablxcpt.c
> @@ -26,7 +26,8 @@ feenableexcept (int excepts)
>
> _FPU_GETCW (temp);
> old_exc = (temp & FPC_EXCEPTION_MASK) >> FPC_EXCEPTION_MASK_SHIFT;
> - new_flags = (temp | ((excepts & FE_ALL_EXCEPT) << FPC_EXCEPTION_MASK_SHIFT));
> + new_flags = (temp | ((unsigned int) (excepts & FE_ALL_EXCEPT)
> + << FPC_EXCEPTION_MASK_SHIFT));
> _FPU_SETCW (new_flags);
>
> return old_exc;
Please place the cast consistently (either inside or outside the
(excepts & FE_ALL_EXCEPT) expression). Ok with that change.
--
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] s390: Fix undefined behaviour in feenableexcept, fedisableexcept [BZ #30960]
2023-10-19 8:48 ` Andreas Schwab
@ 2023-10-19 12:31 ` Stefan Liebler
0 siblings, 0 replies; 3+ messages in thread
From: Stefan Liebler @ 2023-10-19 12:31 UTC (permalink / raw)
To: Andreas Schwab; +Cc: libc-alpha
On 19.10.23 10:48, Andreas Schwab wrote:
> On Okt 18 2023, Stefan Liebler wrote:
>
>> diff --git a/sysdeps/s390/fpu/fedisblxcpt.c b/sysdeps/s390/fpu/fedisblxcpt.c
>> index 728f103f43..84b2c5e64a 100644
>> --- a/sysdeps/s390/fpu/fedisblxcpt.c
>> +++ b/sysdeps/s390/fpu/fedisblxcpt.c
>> @@ -26,7 +26,8 @@ fedisableexcept (int excepts)
>>
>> _FPU_GETCW (temp);
>> old_exc = (temp & FPC_EXCEPTION_MASK) >> FPC_EXCEPTION_MASK_SHIFT;
>> - new_flags = (temp & (~((excepts & FE_ALL_EXCEPT) << FPC_EXCEPTION_MASK_SHIFT)));
>> + new_flags = (temp & (~(((unsigned int) excepts & FE_ALL_EXCEPT)
>> + << FPC_EXCEPTION_MASK_SHIFT)));
>
> There is a redundant pair of parens around the ~ operator that makes the
> whole expression more difficult to read. Please consider removing it.
>
>> _FPU_SETCW (new_flags);
>>
>> return old_exc;
>> diff --git a/sysdeps/s390/fpu/feenablxcpt.c b/sysdeps/s390/fpu/feenablxcpt.c
>> index 0807e610a2..76d25316f4 100644
>> --- a/sysdeps/s390/fpu/feenablxcpt.c
>> +++ b/sysdeps/s390/fpu/feenablxcpt.c
>> @@ -26,7 +26,8 @@ feenableexcept (int excepts)
>>
>> _FPU_GETCW (temp);
>> old_exc = (temp & FPC_EXCEPTION_MASK) >> FPC_EXCEPTION_MASK_SHIFT;
>> - new_flags = (temp | ((excepts & FE_ALL_EXCEPT) << FPC_EXCEPTION_MASK_SHIFT));
>> + new_flags = (temp | ((unsigned int) (excepts & FE_ALL_EXCEPT)
>> + << FPC_EXCEPTION_MASK_SHIFT));
>> _FPU_SETCW (new_flags);
>>
>> return old_exc;
>
> Please place the cast consistently (either inside or outside the
> (excepts & FE_ALL_EXCEPT) expression). Ok with that change.
>
Thanks for the review. I've just committed it with your two mentioned
changes.
Thanks,
Stefan
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2023-10-19 12:31 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-10-18 13:08 [PATCH] s390: Fix undefined behaviour in feenableexcept, fedisableexcept [BZ #30960] Stefan Liebler
2023-10-19 8:48 ` Andreas Schwab
2023-10-19 12:31 ` Stefan Liebler
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).