[PATCH 1/2] libstdc++: Fix invalid signed arguments to <bit> functions

[Jonathan Wakely]

On Thu, 20 Feb 2025 at 14:41, Patrick Palka <ppalka@redhat.com> wrote:
>
> On Sat, 15 Feb 2025, Jonathan Wakely wrote:
>
> > These should have been unsigned, but the static assertions are only in
> > the public std::bit_ceil and std::bit_width functions, not the internal
> > __bit_ceil and __bit_width ones.
> >
> > libstdc++-v3/ChangeLog:
> >
> >       * include/experimental/bits/simd.h (__find_next_valid_abi): Cast
> >       __bit_ceil argument to unsigned.
> >       * src/c++17/floating_from_chars.cc (__floating_from_chars_hex):
> >       Cast __bit_ceil argument to unsigned.
> >       * src/c++17/memory_resource.cc (big_block): Cast __bit_width
> >       argument to unsigned.
> > ---
> >
> > Tested x86_64-linux.
> >
> >  libstdc++-v3/include/experimental/bits/simd.h | 2 +-
> >  libstdc++-v3/src/c++17/floating_from_chars.cc | 5 +++--
> >  libstdc++-v3/src/c++17/memory_resource.cc     | 3 ++-
> >  3 files changed, 6 insertions(+), 4 deletions(-)
> >
> > diff --git a/libstdc++-v3/include/experimental/bits/simd.h b/libstdc++-v3/include/experimental/bits/simd.h
> > index 500c1781ae54..f0cb101aaa82 100644
> > --- a/libstdc++-v3/include/experimental/bits/simd.h
> > +++ b/libstdc++-v3/include/experimental/bits/simd.h
> > @@ -4634,7 +4634,7 @@ template <template <int> class _Abi, int _Bytes, typename _Tp>
> >      static constexpr auto
> >      _S_choose()
> >      {
> > -      constexpr int _NextBytes = std::__bit_ceil(_Bytes) / 2;
> > +      constexpr int _NextBytes = std::__bit_ceil((unsigned)_Bytes) / 2;
> >        using _NextAbi = _Abi<_NextBytes>;
> >        if constexpr (_NextBytes < sizeof(_Tp) * 2) // break recursion
> >       return _Abi<_Bytes>();
> > diff --git a/libstdc++-v3/src/c++17/floating_from_chars.cc b/libstdc++-v3/src/c++17/floating_from_chars.cc
> > index f8b1e23937d2..d48f1c0d4545 100644
> > --- a/libstdc++-v3/src/c++17/floating_from_chars.cc
> > +++ b/libstdc++-v3/src/c++17/floating_from_chars.cc
> > @@ -1102,8 +1102,9 @@ namespace
> >        {
> >       // If the leading hexit is not '1', shift MANTISSA to make it so.
> >       // This normalizes input like "4.08p0" into "1.02p2".
> > -     const int leading_hexit = mantissa >> mantissa_bits;
> > -     const int leading_hexit_width = __bit_width(leading_hexit); // FIXME: optimize?
> > +     const unsigned leading_hexit = mantissa >> mantissa_bits;
> > +     const int leading_hexit_width
> > +       = __bit_width((unsigned)leading_hexit); // FIXME: optimize?
>
> Not a big deal, but isn't this cast redundant if we've defined
> leading_hexit as unsigned?

You can never be too careful in C++! ;-)

Yes you're absolutely right. I think I added the cast at first, then
realised that I could just make the variable unsigned, but forgot to
undo the cast.

The attached patch reverts the cast part, just leaving the
s/int/unsigned/ part on the previous line.
-------------- next part --------------
commit ef1655d3c3caeac56cc9124a2479a5bfac90a239
Author:     Jonathan Wakely <jwakely@redhat.com>
AuthorDate: Thu Feb 20 15:16:11 2025
Commit:     Jonathan Wakely <redi@gcc.gnu.org>
CommitDate: Thu Feb 20 15:16:11 2025

    libstdc++: Remove redundant cast in floating_from_chars.cc
    
    In r15-7647-g32457bc25fea80 I added a cast and also changed the type of
    the variable, making the cast redundant. This removes the cast.
    
    libstdc++-v3/ChangeLog:
    
            * src/c++17/floating_from_chars.cc (__floating_from_chars_hex):
            Remove redundant cast.

diff --git a/libstdc++-v3/src/c++17/floating_from_chars.cc b/libstdc++-v3/src/c++17/floating_from_chars.cc
index d48f1c0d454..9bad12c7f69 100644
--- a/libstdc++-v3/src/c++17/floating_from_chars.cc
+++ b/libstdc++-v3/src/c++17/floating_from_chars.cc
@@ -1103,8 +1103,7 @@ namespace
 	// If the leading hexit is not '1', shift MANTISSA to make it so.
 	// This normalizes input like "4.08p0" into "1.02p2".
 	const unsigned leading_hexit = mantissa >> mantissa_bits;
-	const int leading_hexit_width
-	  = __bit_width((unsigned)leading_hexit); // FIXME: optimize?
+	const int leading_hexit_width = __bit_width(leading_hexit); // FIXME: optimize?
 	__glibcxx_assert(leading_hexit_width >= 1 && leading_hexit_width <= 4);
 	shift_mantissa(leading_hexit_width - 1);
 	// After this adjustment, we can assume the leading hexit is '1'.