diff mbox series

newlib/libc/stdbit: Fix static assertions for 16-bit int

Message ID 20260420152504.827904-1-jwakely@redhat.com
State New
Headers
Series newlib/libc/stdbit: Fix static assertions for 16-bit int |

Commit Message

Jonathan Wakely April 20, 2026, 3:24 p.m. UTC 
  All the USHRT_WIDTH < UINT_WIDTH assertions fail for 16-bit int targets,
such as msp430-elf. Remove those static assertions and instead fix the
stdc_xxx_us functions to work correctly when USHRT_WIDTH == UINT_WIDTH.

Signed-off-by: Jonathan Wakely <jwakely@redhat.com>
---

This is untested, except to check that it now compiles on msp430-elf and
still compiles on arm-eabi.

 newlib/libc/stdbit/stdc_bit_ceil.c       |  9 +++++----
 newlib/libc/stdbit/stdc_leading_ones.c   | 10 ++++++----
 newlib/libc/stdbit/stdc_leading_zeros.c  | 10 ++++++----
 newlib/libc/stdbit/stdc_trailing_ones.c  | 10 ++++++----
 newlib/libc/stdbit/stdc_trailing_zeros.c | 11 +++++++----
 5 files changed, 30 insertions(+), 20 deletions(-)

Comments

Corinna Vinschen April 20, 2026, 7:32 p.m. UTC | #1 
[CC Joel Sherrill, please review]

On Apr 20 16:24, Jonathan Wakely wrote:
> All the USHRT_WIDTH < UINT_WIDTH assertions fail for 16-bit int targets,
> such as msp430-elf. Remove those static assertions and instead fix the
> stdc_xxx_us functions to work correctly when USHRT_WIDTH == UINT_WIDTH.
> 
> Signed-off-by: Jonathan Wakely <jwakely@redhat.com>
> ---
> 
> This is untested, except to check that it now compiles on msp430-elf and
> still compiles on arm-eabi.
> 
>  newlib/libc/stdbit/stdc_bit_ceil.c       |  9 +++++----
>  newlib/libc/stdbit/stdc_leading_ones.c   | 10 ++++++----
>  newlib/libc/stdbit/stdc_leading_zeros.c  | 10 ++++++----
>  newlib/libc/stdbit/stdc_trailing_ones.c  | 10 ++++++----
>  newlib/libc/stdbit/stdc_trailing_zeros.c | 11 +++++++----
>  5 files changed, 30 insertions(+), 20 deletions(-)
> [...]
>  unsigned int
>  stdc_leading_ones_us(unsigned short x)
>  {
> +#if USHRT_WIDTH == UINT_WIDTH
> +	/* Avoid triggering undefined behavior if x == 0. */
> +	if (x == ~0U)

~0 is not 0...


Thanks,
Corinna
Joel Sherrill April 20, 2026, 7:47 p.m. UTC | #2 
Is there a conditional defined when building newlib to know you are inside
newlib? I'd like to guard changes from the original FreeBSD code if that's
possible. We do that inside RTEMS and it makes updates easier.

--joel

On Mon, Apr 20, 2026, 2:32 PM Corinna Vinschen <corinna@vinschen.de> wrote:

>
> [CC Joel Sherrill, please review]
>
> On Apr 20 16:24, Jonathan Wakely wrote:
> > All the USHRT_WIDTH < UINT_WIDTH assertions fail for 16-bit int targets,
> > such as msp430-elf. Remove those static assertions and instead fix the
> > stdc_xxx_us functions to work correctly when USHRT_WIDTH == UINT_WIDTH.
> >
> > Signed-off-by: Jonathan Wakely <jwakely@redhat.com>
> > ---
> >
> > This is untested, except to check that it now compiles on msp430-elf and
> > still compiles on arm-eabi.
> >
> >  newlib/libc/stdbit/stdc_bit_ceil.c       |  9 +++++----
> >  newlib/libc/stdbit/stdc_leading_ones.c   | 10 ++++++----
> >  newlib/libc/stdbit/stdc_leading_zeros.c  | 10 ++++++----
> >  newlib/libc/stdbit/stdc_trailing_ones.c  | 10 ++++++----
> >  newlib/libc/stdbit/stdc_trailing_zeros.c | 11 +++++++----
> >  5 files changed, 30 insertions(+), 20 deletions(-)
> > [...]
> >  unsigned int
> >  stdc_leading_ones_us(unsigned short x)
> >  {
> > +#if USHRT_WIDTH == UINT_WIDTH
> > +     /* Avoid triggering undefined behavior if x == 0. */
> > +     if (x == ~0U)
>
> ~0 is not 0...
>
>
> Thanks,
> Corinna
>
Jonathan Wakely April 20, 2026, 8:29 p.m. UTC | #3 
On Mon, 20 Apr 2026 at 21:32 +0200, Corinna Vinschen wrote:
>
>[CC Joel Sherrill, please review]
>
>On Apr 20 16:24, Jonathan Wakely wrote:
>> All the USHRT_WIDTH < UINT_WIDTH assertions fail for 16-bit int targets,
>> such as msp430-elf. Remove those static assertions and instead fix the
>> stdc_xxx_us functions to work correctly when USHRT_WIDTH == UINT_WIDTH.
>>
>> Signed-off-by: Jonathan Wakely <jwakely@redhat.com>
>> ---
>>
>> This is untested, except to check that it now compiles on msp430-elf and
>> still compiles on arm-eabi.
>>
>>  newlib/libc/stdbit/stdc_bit_ceil.c       |  9 +++++----
>>  newlib/libc/stdbit/stdc_leading_ones.c   | 10 ++++++----
>>  newlib/libc/stdbit/stdc_leading_zeros.c  | 10 ++++++----
>>  newlib/libc/stdbit/stdc_trailing_ones.c  | 10 ++++++----
>>  newlib/libc/stdbit/stdc_trailing_zeros.c | 11 +++++++----
>>  5 files changed, 30 insertions(+), 20 deletions(-)
>> [...]
>>  unsigned int
>>  stdc_leading_ones_us(unsigned short x)
>>  {
>> +#if USHRT_WIDTH == UINT_WIDTH
>> +	/* Avoid triggering undefined behavior if x == 0. */
>> +	if (x == ~0U)
>
>~0 is not 0...

I know. I just moved the original comment, but as I said at
https://inbox.sourceware.org/newlib/CAH6eHdRheB4uCJQCvQLQ3rC5xGgLgA+9H_V60meVR3xrMikzxQ@mail.gmail.com/T/#t
it looks like the comments in that file (the one I moved, and the
earlier one on line 13) are both wrong.
Jonathan Wakely April 20, 2026, 8:32 p.m. UTC | #4 
On Mon, 20 Apr 2026 at 14:47 -0500, Joel Sherrill wrote:
>Is there a conditional defined when building newlib to know you are inside
>newlib? I'd like to guard changes from the original FreeBSD code if that's
>possible. We do that inside RTEMS and it makes updates easier.

You could add:

--- a/newlib/libc/stdbit/stdbit_internal.h
+++ b/newlib/libc/stdbit/stdbit_internal.h
@@ -44,4 +44,6 @@
  #define ULLONG_WIDTH __LONG_LONG_WIDTH__
  #endif
  
+#define newlib_stdbit_h 1
+
  #endif

And then test for it in stdc_bit_ceil.c etc. because that macro would
not be defined by the FreeBSD version of stdbit_internal.h

Or propose the fixes upstream to FreeBSD, but they might not be
interested in supporting 16-bit targets.


>--joel
>
>On Mon, Apr 20, 2026, 2:32 PM Corinna Vinschen <corinna@vinschen.de> wrote:
>
>>
>> [CC Joel Sherrill, please review]
>>
>> On Apr 20 16:24, Jonathan Wakely wrote:
>> > All the USHRT_WIDTH < UINT_WIDTH assertions fail for 16-bit int targets,
>> > such as msp430-elf. Remove those static assertions and instead fix the
>> > stdc_xxx_us functions to work correctly when USHRT_WIDTH == UINT_WIDTH.
>> >
>> > Signed-off-by: Jonathan Wakely <jwakely@redhat.com>
>> > ---
>> >
>> > This is untested, except to check that it now compiles on msp430-elf and
>> > still compiles on arm-eabi.
>> >
>> >  newlib/libc/stdbit/stdc_bit_ceil.c       |  9 +++++----
>> >  newlib/libc/stdbit/stdc_leading_ones.c   | 10 ++++++----
>> >  newlib/libc/stdbit/stdc_leading_zeros.c  | 10 ++++++----
>> >  newlib/libc/stdbit/stdc_trailing_ones.c  | 10 ++++++----
>> >  newlib/libc/stdbit/stdc_trailing_zeros.c | 11 +++++++----
>> >  5 files changed, 30 insertions(+), 20 deletions(-)
>> > [...]
>> >  unsigned int
>> >  stdc_leading_ones_us(unsigned short x)
>> >  {
>> > +#if USHRT_WIDTH == UINT_WIDTH
>> > +     /* Avoid triggering undefined behavior if x == 0. */
>> > +     if (x == ~0U)
>>
>> ~0 is not 0...
>>
>>
>> Thanks,
>> Corinna
>>
diff mbox series

Patch

diff --git a/newlib/libc/stdbit/stdc_bit_ceil.c b/newlib/libc/stdbit/stdc_bit_ceil.c
index f25d184af..7ffed05f4 100644
--- a/newlib/libc/stdbit/stdc_bit_ceil.c
+++ b/newlib/libc/stdbit/stdc_bit_ceil.c
@@ -23,16 +23,17 @@  stdc_bit_ceil_uc(unsigned char x)
 	return (1U << (UINT_WIDTH - __builtin_clz(x - 1)));
 }
 
-/* Ensure we don't shift 1U out of range. */
-_Static_assert(USHRT_WIDTH < UINT_WIDTH,
-    "stdc_bit_ceil_us needs USHRT_WIDTH < UINT_WIDTH");
-
 unsigned short
 stdc_bit_ceil_us(unsigned short x)
 {
 	if (x <= 1)
 		return (1);
 
+#if USHRT_WIDTH == UINT_WIDTH
+	if (x > USHRT_MAX/2 + 1)
+		return (0);
+#endif
+
 	return (1U << (UINT_WIDTH - __builtin_clz(x - 1)));
 }
 
diff --git a/newlib/libc/stdbit/stdc_leading_ones.c b/newlib/libc/stdbit/stdc_leading_ones.c
index c7ab75b98..48f61d71e 100644
--- a/newlib/libc/stdbit/stdc_leading_ones.c
+++ b/newlib/libc/stdbit/stdc_leading_ones.c
@@ -22,13 +22,15 @@  stdc_leading_ones_uc(unsigned char x)
 	return (__builtin_clz(~(x << offset)));
 }
 
-/* Avoid triggering undefined behavior if x == 0. */
-_Static_assert(USHRT_WIDTH < UINT_WIDTH,
-    "stdc_leading_ones_us needs USHRT_WIDTH < UINT_WIDTH");
-
 unsigned int
 stdc_leading_ones_us(unsigned short x)
 {
+#if USHRT_WIDTH == UINT_WIDTH
+	/* Avoid triggering undefined behavior if x == 0. */
+	if (x == ~0U)
+		return (USHRT_WIDTH);
+#endif
+
 	const int offset = UINT_WIDTH - USHRT_WIDTH;
 
 	return (__builtin_clz(~(x << offset)));
diff --git a/newlib/libc/stdbit/stdc_leading_zeros.c b/newlib/libc/stdbit/stdc_leading_zeros.c
index dba6a14e2..204f71e21 100644
--- a/newlib/libc/stdbit/stdc_leading_zeros.c
+++ b/newlib/libc/stdbit/stdc_leading_zeros.c
@@ -22,13 +22,15 @@  stdc_leading_zeros_uc(unsigned char x)
 	return (__builtin_clz((x << offset) + (1U << (offset - 1))));
 }
 
-/* Offset must be greater than zero. */
-_Static_assert(USHRT_WIDTH < UINT_WIDTH,
-    "stdc_leading_zeros_us needs USHRT_WIDTH < UINT_WIDTH");
-
 unsigned int
 stdc_leading_zeros_us(unsigned short x)
 {
+#if USHRT_WIDTH == UINT_WIDTH
+	/* Offset must be greater than zero. */
+	if (x == 0)
+		return (USHRT_WIDTH);
+#endif
+
 	const int offset = UINT_WIDTH - USHRT_WIDTH;
 
 	return (__builtin_clz((x << offset) + (1U << (offset - 1))));
diff --git a/newlib/libc/stdbit/stdc_trailing_ones.c b/newlib/libc/stdbit/stdc_trailing_ones.c
index 0f7ccb553..22f002878 100644
--- a/newlib/libc/stdbit/stdc_trailing_ones.c
+++ b/newlib/libc/stdbit/stdc_trailing_ones.c
@@ -20,13 +20,15 @@  stdc_trailing_ones_uc(unsigned char x)
 	return (__builtin_ctz(~x));
 }
 
-/* Avoid triggering undefined behavior if x == ~0. */
-_Static_assert(USHRT_WIDTH < UINT_WIDTH,
-    "stdc_trailing_ones_uc needs USHRT_WIDTH < UINT_WIDTH");
-
 unsigned int
 stdc_trailing_ones_us(unsigned short x)
 {
+#if USHRT_WIDTH == UINT_WIDTH
+	/* Avoid triggering undefined behavior if x == ~0. */
+	if (x == ~0U)
+		return (USHRT_WIDTH);
+#endif
+
 	return (__builtin_ctz(~x));
 }
 
diff --git a/newlib/libc/stdbit/stdc_trailing_zeros.c b/newlib/libc/stdbit/stdc_trailing_zeros.c
index 396333e17..8526461bc 100644
--- a/newlib/libc/stdbit/stdc_trailing_zeros.c
+++ b/newlib/libc/stdbit/stdc_trailing_zeros.c
@@ -20,14 +20,17 @@  stdc_trailing_zeros_uc(unsigned char x)
 	return (__builtin_ctz(x | 1U << UCHAR_WIDTH));
 }
 
-/* Ensure we do not shift 1U out of range. */
-_Static_assert(USHRT_WIDTH < UINT_WIDTH,
-    "stdc_trailing_zeros_uc needs USHRT_WIDTH < UINT_WIDTH");
-
 unsigned int
 stdc_trailing_zeros_us(unsigned short x)
 {
+#if USHRT_WIDTH == UINT_WIDTH
+	/* Ensure we do not shift 1U out of range. */
+	if (x == 0U)
+		return (USHRT_WIDTH);
+	return (__builtin_ctz(x));
+#else
 	return (__builtin_ctz(x | 1U << USHRT_WIDTH));
+#endif
 }
 
 unsigned int