[committed] libstdc++: Fix recent changes to __glibcxx_assert [PR112882]

[Jonathan Wakely]

On Sun, 3 Aug 2025, 14:54 Jonathan Wakely, <jwakely.gcc@gmail.com> wrote:

>
>
> On Sun, 3 Aug 2025, 01:55 Andrew Pinski, <pinskia@gmail.com> wrote:
>
>> On Thu, Dec 7, 2023 at 2:07 PM Jonathan Wakely <jwakely@redhat.com>
>> wrote:
>> >
>> > Tested x86_64-linux. Pushed to trunk.
>> >
>> > -- >8 --
>> >
>> > The changes in r14-6198-g5e8a30d8b8f4d7 were broken, as I used
>> > _GLIBCXX17_CONSTEXPR for the 'if _GLIBCXX17_CONSTEXPR (true)' condition,
>> > forgetting that it would also be used for the is_constant_evaluated()
>> > check. Using 'if constexpr (std::is_constant_evaluated())' is a bug.
>> >
>> > Additionally, relying on __glibcxx_assert_fail to give a "not a constant
>> > expression" error is a problem because at -O0 an undefined reference to
>> > __glibcxx_assert_fail is present in the compiled code. This means you
>> > can't use libstdc++ headers without also linking to libstdc++ for the
>> > symbol definition.
>> >
>> > This fix rewrites the __glibcxx_assert macro again. This still avoids
>> > doing the duplicate checks, once for constexpr and once at runtime (if
>> > _GLIBCXX_ASSERTIONS is defined). When _GLIBCXX_ASSERTIONS is defined we
>> > still rely on __glibcxx_assert_fail to give a "not a constant
>> > expression" error during constant evaluation (because when assertions
>> > are defined it's not a problem to emit a reference to the symbol). But
>> > when that macro is not defined, we use a new inline (but not constexpr)
>> > overload of __glibcxx_assert_fail to cause compilation to fail. That
>> > inline function doesn't cause an undefined reference to a symbol in the
>> > library (and will be optimized away anyway).
>> >
>> > We can also add always_inline to the __is_constant_evaluated function,
>> > although this doesn't actually matter for -O0 and it's always inlined
>> > with any optimization enabled.
>>
>> I know this is an old patch but I am curious if we should include
>> __builtin_unreachable() in __glibcxx_assert_fail for the current case
>> where it is empty?
>>
>
> No. Assertions are not assumptions.
>


https://wg21.link/p2064



> Any check which is worth asserting means it's something that users might
> get wrong. If we mark it unreachable, we risk misoptimizing user code in
> ways that are hard to debug and hard to understand. It might also cause the
> compiler to remove other checks that users put into their own code, using
> defensive programming techniques.
>
>
> I was looking into PR 121280 where it uses front/back without checking
>> if the vector is empy and GCC is producing a warning which is not easy
>> to understand but seems correct due to this undefinedness (well a
>> missed jump threading). Do we think the warning should stay or adding
>> the unreachable in __glibcxx_assert_fail is a good idea?
>>
>> Note I suspect we don't want to add the unreachable. But maybe it is
>> better to aways enable the assert instead?
>>
>> Thanks,
>> Andrew
>>
>> >
>> > libstdc++-v3/ChangeLog:
>> >
>> >         PR libstdc++/112882
>> >         * include/bits/c++config (__is_constant_evaluated): Add
>> >         always_inline attribute.
>> >         (_GLIBCXX_DO_ASSERT): Remove macro.
>> >         (__glibcxx_assert): Define separately for assertions-enabled and
>> >         constexpr-only cases.
>> > ---
>> >  libstdc++-v3/include/bits/c++config | 33 ++++++++++++++++++++---------
>> >  1 file changed, 23 insertions(+), 10 deletions(-)
>> >
>> > diff --git a/libstdc++-v3/include/bits/c++config
>> b/libstdc++-v3/include/bits/c++config
>> > index 284d24d933f..25d37428fc1 100644
>> > --- a/libstdc++-v3/include/bits/c++config
>> > +++ b/libstdc++-v3/include/bits/c++config
>> > @@ -538,6 +538,7 @@ namespace std
>> >    // This can be used without checking if the compiler supports the
>> feature.
>> >    // The macro _GLIBCXX_HAVE_IS_CONSTANT_EVALUATED can be used to
>> check if
>> >    // the compiler support is present to make this function work as
>> expected.
>> > +  __attribute__((__always_inline__))
>> >    _GLIBCXX_CONSTEXPR inline bool
>> >    __is_constant_evaluated() _GLIBCXX_NOEXCEPT
>> >    {
>> > @@ -598,19 +599,31 @@ namespace std
>> >  #endif
>> >
>> >  #if defined(_GLIBCXX_ASSERTIONS)
>> > -# define _GLIBCXX_DO_ASSERT true
>> > -#elif _GLIBCXX_HAVE_IS_CONSTANT_EVALUATED
>> > -# define _GLIBCXX_DO_ASSERT std::__is_constant_evaluated()
>> > -#else
>> > -# define _GLIBCXX_DO_ASSERT false
>> > -#endif
>> > -
>> > +// Enable runtime assertion checks, and also check in constant
>> expressions.
>> >  # define __glibcxx_assert(cond)
>>         \
>> >    do {
>>  \
>> > -    if _GLIBCXX17_CONSTEXPR (_GLIBCXX_DO_ASSERT)
>>  \
>> > -      if (__builtin_expect(!bool(cond), false))
>>         \
>> > -       _GLIBCXX_ASSERT_FAIL(cond);
>>  \
>> > +    if (__builtin_expect(!bool(cond), false))
>> \
>> > +      _GLIBCXX_ASSERT_FAIL(cond);
>> \
>> >    } while (false)
>> > +#elif _GLIBCXX_HAVE_IS_CONSTANT_EVALUATED
>> > +// Only check assertions during constant evaluation.
>> > +namespace std
>> > +{
>> > +  __attribute__((__always_inline__,__visibility__("default")))
>> > +  inline void
>> > +  __glibcxx_assert_fail()
>> > +  { }
>> > +}
>> > +# define __glibcxx_assert(cond)
>>         \
>> > +  do {
>>  \
>> > +    if (std::__is_constant_evaluated())
>>         \
>> > +      if (__builtin_expect(!bool(cond), false))
>>         \
>> > +       std::__glibcxx_assert_fail();
>>  \
>> > +  } while (false)
>> > +#else
>> > +// Don't check any assertions.
>> > +# define __glibcxx_assert(cond)
>> > +#endif
>> >
>> >  // Macro indicating that TSAN is in use.
>> >  #if __SANITIZE_THREAD__
>> > --
>> > 2.43.0
>> >
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://gcc.gnu.org/pipermail/libstdc++/attachments/20250803/5ecf9f55/attachment.htm>