public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
* [Bug libstdc++/113099] New: locale without RTTI uses dynamic_cast before gcc 13.2 or has ODR violation since gcc 13.2
@ 2023-12-20 23:41 andysem at mail dot ru
  2023-12-21  8:59 ` [Bug libstdc++/113099] " rguenth at gcc dot gnu.org
                   ` (15 more replies)
  0 siblings, 16 replies; 17+ messages in thread
From: andysem at mail dot ru @ 2023-12-20 23:41 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113099

            Bug ID: 113099
           Summary: locale without RTTI uses dynamic_cast before gcc 13.2
                    or has ODR violation since gcc 13.2
           Product: gcc
           Version: 11.4.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: libstdc++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: andysem at mail dot ru
  Target Milestone: ---

Consider this test case:

```
#include <locale>

class __attribute__((__visibility__("default"))) my_codecvt final :
    public std::codecvt< wchar_t, char, std::mbstate_t >
{
public:
    explicit my_codecvt(std::size_t refs = 0) :
        std::codecvt< wchar_t, char, std::mbstate_t >(refs)
    {
    }

protected:
    bool do_always_noconv() const noexcept override { return false; }

    int do_encoding() const noexcept override { return 0; }
    std::codecvt_base::result do_in(std::mbstate_t&, const char*,
        const char*, const char*&, wchar_t*, wchar_t*, wchar_t*&)
        const override
    { return ok; }
    std::codecvt_base::result do_out(std::mbstate_t&, const wchar_t*,
        const wchar_t*, const wchar_t*&, char*, char*, char*&)
        const override
    { return ok; }
    std::codecvt_base::result do_unshift(std::mbstate_t&, char*,
        char*, char*&) const override
    { return ok; }
    int do_length(std::mbstate_t&, const char*, const char*,
        std::size_t) const override
    { return 0; }
    int do_max_length() const noexcept override { return 0; }
};

int main()
{
    std::locale loc(std::locale(), new my_codecvt());
    auto const& fac = std::use_facet<
        std::codecvt< wchar_t, char, std::mbstate_t > >(loc);
    (void)fac;
}
```

```
g++ -std=c++17 -fno-rtti -o locale_no_rtti locale_no_rtti.cpp
```

When compiled with RTTI disabled, with the command line above, this code
crashes with the following backtrace:

```
#0  0x00007ffff7caccd1 in __dynamic_cast () from
/lib/x86_64-linux-gnu/libstdc++.so.6
#1  0x00007ffff7d5307f in std::codecvt<wchar_t, char, __mbstate_t> const&
std::use_facet<std::codecvt<wchar_t, char, __mbstate_t> >(std::locale const&)
() from /lib/x86_64-linux-gnu/libstdc++.so.6
#2  0x000055555555539b in main ()
```

This reproduces on gcc 11.4 and 12 at least.

```
$ g++ -v
Using built-in specs.
COLLECT_GCC=g++
COLLECT_LTO_WRAPPER=/usr/lib/gcc/x86_64-linux-gnu/11/lto-wrapper
OFFLOAD_TARGET_NAMES=nvptx-none:amdgcn-amdhsa
OFFLOAD_TARGET_DEFAULT=1
Target: x86_64-linux-gnu
Configured with: ../src/configure -v --with-pkgversion='Ubuntu
11.4.0-1ubuntu1~22.04' --with-bugurl=file:///usr/share/doc/gcc-11/README.Bugs
--enable-languages=c,ada,c++,go,brig,d,fortran,objc,obj-c++,m2 --prefix=/usr
--with-gcc-major-version-only --program-suffix=-11
--program-prefix=x86_64-linux-gnu- --enable-shared --enable-linker-build-id
--libexecdir=/usr/lib --without-included-gettext --enable-threads=posix
--libdir=/usr/lib --enable-nls --enable-bootstrap --enable-clocale=gnu
--enable-libstdcxx-debug --enable-libstdcxx-time=yes
--with-default-libstdcxx-abi=new --enable-gnu-unique-object
--disable-vtable-verify --enable-plugin --enable-default-pie --with-system-zlib
--enable-libphobos-checking=release --with-target-system-zlib=auto
--enable-objc-gc=auto --enable-multiarch --disable-werror --enable-cet
--with-arch-32=i686 --with-abi=m64 --with-multilib-list=m32,m64,mx32
--enable-multilib --with-tune=generic
--enable-offload-targets=nvptx-none=/build/gcc-11-XeT9lY/gcc-11-11.4.0/debian/tmp-nvptx/usr,amdgcn-amdhsa=/build/gcc-11-XeT9lY/gcc-11-11.4.0/debian/tmp-gcn/usr
--without-cuda-driver --enable-checking=release --build=x86_64-linux-gnu
--host=x86_64-linux-gnu --target=x86_64-linux-gnu
--with-build-config=bootstrap-lto-lean --enable-link-serialization=2
Thread model: posix
Supported LTO compression algorithms: zlib zstd
gcc version 11.4.0 (Ubuntu 11.4.0-1ubuntu1~22.04) 
```

This was apparently fixed by accident in gcc 13.2 by this commit:

https://github.com/gcc-mirror/gcc/commit/b3ac43a3c05744d62a963d656bed782fc867ad79

The commit introduces shortcuts that use static_casts for the standard facets,
which allows to avoid the crash, but that still retains an ODR violation, where
the explicitly instantiated __try_use_facet templates in libstdc++ library use
dynamic_cast (even if unreachable) and the template definition that is visible
to user's code uses static_cast. The problematic code is here:

https://github.com/gcc-mirror/gcc/blob/d7e9ae4fa94afd5517536b4dfc7d6be0b3e8c2c3/libstdc%2B%2B-v3/include/bits/locale_classes.tcc#L142-L146

When libstdc++ is compiled, RTTI is enabled and __cpp_rtti is defined, but when
user's code is compiled with RTTI disabled, that macro is not defined, so the
__try_use_facet template definition is different.

It doesn't seem like the commit I mentioned above intended to fix the original
issue with dynamic_cast anyway, so I thought it was worth creating this bug
report, even though the original test case passes on the latest gcc.

I think, the code should be modified so that __cpp_rtti is only tested in the
code that is instantiated by the user but not libstdc++. libstdc++ should
export two different functions - one that could use dynamic_cast and another
one that doesn't - and the selection of which one to call should happen in the
user-visible code based on the __cpp_rtti macro.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [Bug libstdc++/113099] locale without RTTI uses dynamic_cast before gcc 13.2 or has ODR violation since gcc 13.2
  2023-12-20 23:41 [Bug libstdc++/113099] New: locale without RTTI uses dynamic_cast before gcc 13.2 or has ODR violation since gcc 13.2 andysem at mail dot ru
@ 2023-12-21  8:59 ` rguenth at gcc dot gnu.org
  2023-12-21 21:49 ` redi at gcc dot gnu.org
                   ` (14 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: rguenth at gcc dot gnu.org @ 2023-12-21  8:59 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113099

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |ABI, documentation

--- Comment #1 from Richard Biener <rguenth at gcc dot gnu.org> ---
It seems to me that -fno-rtti is an ABI changing option, at least to the
runtime.  Maybe we should clarify that in its documentation?  -fno-exceptions
could possibly have similar effects if not used consistently across all TUs in
a project.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [Bug libstdc++/113099] locale without RTTI uses dynamic_cast before gcc 13.2 or has ODR violation since gcc 13.2
  2023-12-20 23:41 [Bug libstdc++/113099] New: locale without RTTI uses dynamic_cast before gcc 13.2 or has ODR violation since gcc 13.2 andysem at mail dot ru
  2023-12-21  8:59 ` [Bug libstdc++/113099] " rguenth at gcc dot gnu.org
@ 2023-12-21 21:49 ` redi at gcc dot gnu.org
  2023-12-23  9:37 ` andysem at mail dot ru
                   ` (13 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: redi at gcc dot gnu.org @ 2023-12-21 21:49 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113099

--- Comment #2 from Jonathan Wakely <redi at gcc dot gnu.org> ---
It's mostly OK to mix code with -frtti and -fno-rtti, but sometimes it bites
you.

The crash with older releases seems like __dynamic_cast should gracefully
handle missing RTTI and just fail, not segfault.

The ODR violation in current releaes is a non-issue. The code has the same
semantics either way.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [Bug libstdc++/113099] locale without RTTI uses dynamic_cast before gcc 13.2 or has ODR violation since gcc 13.2
  2023-12-20 23:41 [Bug libstdc++/113099] New: locale without RTTI uses dynamic_cast before gcc 13.2 or has ODR violation since gcc 13.2 andysem at mail dot ru
  2023-12-21  8:59 ` [Bug libstdc++/113099] " rguenth at gcc dot gnu.org
  2023-12-21 21:49 ` redi at gcc dot gnu.org
@ 2023-12-23  9:37 ` andysem at mail dot ru
  2023-12-23  9:39 ` andysem at mail dot ru
                   ` (12 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: andysem at mail dot ru @ 2023-12-23  9:37 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113099

--- Comment #3 from andysem at mail dot ru ---
I think, a failing dynamic_cast would not be useful as this would make
std::use_facet unusable with -fno-rtti.

Re. ODR violation in the latest code, while it is true that the
dynamic/static_cast is not reachable for the standard facets, it is still part
of the function definition and the compiler is free to generate code that takes
it into account. Note that the shortcuts for the standard facets are
implemented with conditional if-constexpr, which will turn into regular ifs if
libstdc++ is compiled in pre-C++17 mode. Which, I think, is the default, isn't
it? I think, the ODR violation is still worth fixing.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [Bug libstdc++/113099] locale without RTTI uses dynamic_cast before gcc 13.2 or has ODR violation since gcc 13.2
  2023-12-20 23:41 [Bug libstdc++/113099] New: locale without RTTI uses dynamic_cast before gcc 13.2 or has ODR violation since gcc 13.2 andysem at mail dot ru
                   ` (2 preceding siblings ...)
  2023-12-23  9:37 ` andysem at mail dot ru
@ 2023-12-23  9:39 ` andysem at mail dot ru
  2023-12-24 14:44 ` redi at gcc dot gnu.org
                   ` (11 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: andysem at mail dot ru @ 2023-12-23  9:39 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113099

--- Comment #4 from andysem at mail dot ru ---
> It's mostly OK to mix code with -frtti and -fno-rtti, but sometimes it bites you.

Note that in this case, it is the standard library that is built with -frtti
and the rest of the code is built with -fno-rtti. I.e. you *always* get this
sort of mix when you specify -fno-rtti.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [Bug libstdc++/113099] locale without RTTI uses dynamic_cast before gcc 13.2 or has ODR violation since gcc 13.2
  2023-12-20 23:41 [Bug libstdc++/113099] New: locale without RTTI uses dynamic_cast before gcc 13.2 or has ODR violation since gcc 13.2 andysem at mail dot ru
                   ` (3 preceding siblings ...)
  2023-12-23  9:39 ` andysem at mail dot ru
@ 2023-12-24 14:44 ` redi at gcc dot gnu.org
  2023-12-24 14:46 ` redi at gcc dot gnu.org
                   ` (10 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: redi at gcc dot gnu.org @ 2023-12-24 14:44 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113099

--- Comment #5 from Jonathan Wakely <redi at gcc dot gnu.org> ---
I'm not sure what you mean by "the compiler is free to generate code that takes
it into account." Takes what into account? What problem does that freedom
cause?

The locale facet instantiations are compiled as C++11 (but other parts of the
library are compiled as C++17 or newer). We should do this so it uses
if-constexpr for everything except C++98:

--- a/libstdc++-v3/include/bits/locale_classes.tcc
+++ b/libstdc++-v3/include/bits/locale_classes.tcc
@@ -87,6 +87,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
                                __s2.data(), __s2.data() + __s2.length()) < 0);
     }

+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wc++17-extensions"
   template<typename _Facet>
     inline const _Facet*
     __try_use_facet(const locale& __loc) _GLIBCXX_NOTHROW
@@ -97,7 +99,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       // We know these standard facets are always installed in every locale
       // so dynamic_cast always succeeds, just use static_cast instead.
 #define _GLIBCXX_STD_FACET(...) \
-      if _GLIBCXX17_CONSTEXPR (__is_same(_Facet, __VA_ARGS__)) \
+      if _GLIBCXX_CONSTEXPR (__is_same(_Facet, __VA_ARGS__)) \
        return static_cast<const _Facet*>(__facets[__i])

       _GLIBCXX_STD_FACET(ctype<char>);
@@ -145,6 +147,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       return static_cast<const _Facet*>(__facets[__i]);
 #endif
     }
+#pragma GCC diagnostic pop

   /**
    *  @brief  Test for the presence of a facet.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [Bug libstdc++/113099] locale without RTTI uses dynamic_cast before gcc 13.2 or has ODR violation since gcc 13.2
  2023-12-20 23:41 [Bug libstdc++/113099] New: locale without RTTI uses dynamic_cast before gcc 13.2 or has ODR violation since gcc 13.2 andysem at mail dot ru
                   ` (4 preceding siblings ...)
  2023-12-24 14:44 ` redi at gcc dot gnu.org
@ 2023-12-24 14:46 ` redi at gcc dot gnu.org
  2023-12-24 15:04 ` pdimov at gmail dot com
                   ` (9 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: redi at gcc dot gnu.org @ 2023-12-24 14:46 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113099

--- Comment #6 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to andysem from comment #3)
> I think, a failing dynamic_cast would not be useful as this would make
> std::use_facet unusable with -fno-rtti.

I don't see a problem with that. If you want to use a polymorphic container of
facets, you need RTTI. The standard facets will work, but it doesn't seem
reasonable to expect it to work for program-defined facets. And a
std::use_facet that fails seems better than one that segfaults, doesn't it?

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [Bug libstdc++/113099] locale without RTTI uses dynamic_cast before gcc 13.2 or has ODR violation since gcc 13.2
  2023-12-20 23:41 [Bug libstdc++/113099] New: locale without RTTI uses dynamic_cast before gcc 13.2 or has ODR violation since gcc 13.2 andysem at mail dot ru
                   ` (5 preceding siblings ...)
  2023-12-24 14:46 ` redi at gcc dot gnu.org
@ 2023-12-24 15:04 ` pdimov at gmail dot com
  2023-12-24 15:27 ` redi at gcc dot gnu.org
                   ` (8 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: pdimov at gmail dot com @ 2023-12-24 15:04 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113099

--- Comment #7 from Peter Dimov <pdimov at gmail dot com> ---
You don't necessarily need dynamic_cast because facets are always installed and
obtained by their exact type, not via a reference to base. You can store the
Facet* as given (like shared_ptr does), and return it.

The only reason dynamic_cast is needed here is because you can't static_cast
from facet* to Facet* when virtual inheritance. But you are not required to
store facet* in the actual container; you can store the original Facet* as
void*.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [Bug libstdc++/113099] locale without RTTI uses dynamic_cast before gcc 13.2 or has ODR violation since gcc 13.2
  2023-12-20 23:41 [Bug libstdc++/113099] New: locale without RTTI uses dynamic_cast before gcc 13.2 or has ODR violation since gcc 13.2 andysem at mail dot ru
                   ` (6 preceding siblings ...)
  2023-12-24 15:04 ` pdimov at gmail dot com
@ 2023-12-24 15:27 ` redi at gcc dot gnu.org
  2023-12-24 15:30 ` redi at gcc dot gnu.org
                   ` (7 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: redi at gcc dot gnu.org @ 2023-12-24 15:27 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113099

--- Comment #8 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to Peter Dimov from comment #7)
> You don't necessarily need dynamic_cast because facets are always installed
> and obtained by their exact type, not via a reference to base.

Is that true? std::use_facet<X> will return a reference to a facet with X::id
but it could actually be something derived from X. e.g. a user can install
their own facet derived from std::ctype<char>, which overrides some members.
Code that does std::use_facet<std::ctype<char>>(loc) will get the user's facet,
but via reference to base.

> You can store
> the Facet* as given (like shared_ptr does), and return it.
> 
> The only reason dynamic_cast is needed here is because you can't static_cast
> from facet* to Facet* when virtual inheritance. But you are not required to
> store facet* in the actual container; you can store the original Facet* as
> void*.

An implementation could do that, but I don't think libstdc++ can do it now
without an ABI change.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [Bug libstdc++/113099] locale without RTTI uses dynamic_cast before gcc 13.2 or has ODR violation since gcc 13.2
  2023-12-20 23:41 [Bug libstdc++/113099] New: locale without RTTI uses dynamic_cast before gcc 13.2 or has ODR violation since gcc 13.2 andysem at mail dot ru
                   ` (7 preceding siblings ...)
  2023-12-24 15:27 ` redi at gcc dot gnu.org
@ 2023-12-24 15:30 ` redi at gcc dot gnu.org
  2023-12-24 15:40 ` pdimov at gmail dot com
                   ` (6 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: redi at gcc dot gnu.org @ 2023-12-24 15:30 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113099

--- Comment #9 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to Jonathan Wakely from comment #8)
> (In reply to Peter Dimov from comment #7)
> > You don't necessarily need dynamic_cast because facets are always installed
> > and obtained by their exact type, not via a reference to base.
> 
> Is that true? std::use_facet<X> will return a reference to a facet with
> X::id but it could actually be something derived from X. e.g. a user can
> install their own facet derived from std::ctype<char>, which overrides some
> members. Code that does std::use_facet<std::ctype<char>>(loc) will get the
> user's facet, but via reference to base.

And I guess if the user's derived facet uses virtual inheritance from
std::locale::facet, then this could break with the
static_cast<std::ctype<char>*> in libstdc++ today. Hmm.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [Bug libstdc++/113099] locale without RTTI uses dynamic_cast before gcc 13.2 or has ODR violation since gcc 13.2
  2023-12-20 23:41 [Bug libstdc++/113099] New: locale without RTTI uses dynamic_cast before gcc 13.2 or has ODR violation since gcc 13.2 andysem at mail dot ru
                   ` (8 preceding siblings ...)
  2023-12-24 15:30 ` redi at gcc dot gnu.org
@ 2023-12-24 15:40 ` pdimov at gmail dot com
  2023-12-24 16:22 ` andysem at mail dot ru
                   ` (5 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: pdimov at gmail dot com @ 2023-12-24 15:40 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113099

--- Comment #10 from Peter Dimov <pdimov at gmail dot com> ---
Maybe the right thing to do is to use dynamic_cast only for virtual inheritance
(either have a trait or check whether static_cast isn't a valid expression),
otherwise static_cast, in both cases (standard and user-defined Facet.)

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [Bug libstdc++/113099] locale without RTTI uses dynamic_cast before gcc 13.2 or has ODR violation since gcc 13.2
  2023-12-20 23:41 [Bug libstdc++/113099] New: locale without RTTI uses dynamic_cast before gcc 13.2 or has ODR violation since gcc 13.2 andysem at mail dot ru
                   ` (9 preceding siblings ...)
  2023-12-24 15:40 ` pdimov at gmail dot com
@ 2023-12-24 16:22 ` andysem at mail dot ru
  2023-12-26 12:31 ` redi at gcc dot gnu.org
                   ` (4 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: andysem at mail dot ru @ 2023-12-24 16:22 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113099

--- Comment #11 from andysem at mail dot ru ---
> I'm not sure what you mean by "the compiler is free to generate code that takes it into account." Takes what into account? What problem does that freedom cause?

I mean the compiler could move (some part of) dynamic_cast to precede the check
for the standard facet. I.e. of something like this:

  template< typename _Facet >
  const _Facet* __try_use_facet(locale const& loc)
  {
    const size_t __i = _Facet::id._M_id();
    const locale::facet** __facets = __loc._M_impl->_M_facets;
    const _Facet* __dyn_facet = __dynamic_cast< const _Facet* >(__facets[__i]);

    // checks for every standard facet type
    if (__is_same(_Facet, ...))
      return static_cast<const _Facet*>(__facets[__i]);

    return __dyn_facet;
  }

>> I think, a failing dynamic_cast would not be useful as this would make
>> std::use_facet unusable with -fno-rtti.
>
> I don't see a problem with that. If you want to use a polymorphic container of facets, you need RTTI. The standard facets will work, but it doesn't seem reasonable to expect it to work for program-defined facets.

AFAIK, the standard or libstdc++ docs do not require RTTI for std::locale to
function. In fact, the facet::id stuff seems to exist precisely to make RTTI
optional in the implementations. Besides, the code, as it was written, seems to
intend to work without RTTI by switching to static_cast instead of
dynamic_cast. So making std::use_facet always fail would go against that
intention.

> And a std::use_facet that fails seems better than one that segfaults, doesn't it?

No, not really. It still means users cannot use locale without RTTI, just for a
different reason.

> And I guess if the user's derived facet uses virtual inheritance from std::locale::facet, then this could break with the static_cast<std::ctype<char>*> in libstdc++ today. Hmm.

std::ctype non-virtually derives from std::locale::facet, so no, that would not
break. What wouldn't work is this:

   class my_facet : virtual public std::ctype< char > {};

   my_facet const& fac = std::use_facet< my_facet >(loc);

But this would fail in compile time with no RTTI, which *is* better than a
segfault or an exception in runtime.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [Bug libstdc++/113099] locale without RTTI uses dynamic_cast before gcc 13.2 or has ODR violation since gcc 13.2
  2023-12-20 23:41 [Bug libstdc++/113099] New: locale without RTTI uses dynamic_cast before gcc 13.2 or has ODR violation since gcc 13.2 andysem at mail dot ru
                   ` (10 preceding siblings ...)
  2023-12-24 16:22 ` andysem at mail dot ru
@ 2023-12-26 12:31 ` redi at gcc dot gnu.org
  2023-12-26 15:20 ` andysem at mail dot ru
                   ` (3 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: redi at gcc dot gnu.org @ 2023-12-26 12:31 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113099

--- Comment #12 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to andysem from comment #11)
> > I'm not sure what you mean by "the compiler is free to generate code that takes it into account." Takes what into account? What problem does that freedom cause?
> 
> I mean the compiler could move (some part of) dynamic_cast to precede the
> check for the standard facet. I.e. of something like this:
> 
>   template< typename _Facet >
>   const _Facet* __try_use_facet(locale const& loc)
>   {
>     const size_t __i = _Facet::id._M_id();
>     const locale::facet** __facets = __loc._M_impl->_M_facets;
>     const _Facet* __dyn_facet = __dynamic_cast< const _Facet*
> >(__facets[__i]);
> 
>     // checks for every standard facet type
>     if (__is_same(_Facet, ...))
>       return static_cast<const _Facet*>(__facets[__i]);
> 
>     return __dyn_facet;
>   }

But why? Maybe I'm being slow but I still don't understand what problem is
being solved here.

Also this would defeat the optimization that avoids the unnecessary overhead of
dynamic_cast for standard facets.

> AFAIK, the standard or libstdc++ docs do not require RTTI for std::locale to
> function.

The standard requires RTTI, period. Using -fno-rtti is completely non-standard
and so the standard has nothing to say about it.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [Bug libstdc++/113099] locale without RTTI uses dynamic_cast before gcc 13.2 or has ODR violation since gcc 13.2
  2023-12-20 23:41 [Bug libstdc++/113099] New: locale without RTTI uses dynamic_cast before gcc 13.2 or has ODR violation since gcc 13.2 andysem at mail dot ru
                   ` (11 preceding siblings ...)
  2023-12-26 12:31 ` redi at gcc dot gnu.org
@ 2023-12-26 15:20 ` andysem at mail dot ru
  2024-01-03 12:38 ` redi at gcc dot gnu.org
                   ` (2 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: andysem at mail dot ru @ 2023-12-26 15:20 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113099

--- Comment #13 from andysem at mail dot ru ---
(In reply to Jonathan Wakely from comment #12)
> (In reply to andysem from comment #11)
> > > I'm not sure what you mean by "the compiler is free to generate code that takes it into account." Takes what into account? What problem does that freedom cause?
> > 
> > I mean the compiler could move (some part of) dynamic_cast to precede the
> > check for the standard facet. I.e. of something like this:
> > 
> >   template< typename _Facet >
> >   const _Facet* __try_use_facet(locale const& loc)
> >   {
> >     const size_t __i = _Facet::id._M_id();
> >     const locale::facet** __facets = __loc._M_impl->_M_facets;
> >     const _Facet* __dyn_facet = __dynamic_cast< const _Facet*
> > >(__facets[__i]);
> > 
> >     // checks for every standard facet type
> >     if (__is_same(_Facet, ...))
> >       return static_cast<const _Facet*>(__facets[__i]);
> > 
> >     return __dyn_facet;
> >   }
> 
> But why? Maybe I'm being slow but I still don't understand what problem is
> being solved here.
> 
> Also this would defeat the optimization that avoids the unnecessary overhead
> of dynamic_cast for standard facets.

I have seen gcc sometimes reorder code like this (i.e. move code from under a
branch before it), presumably to improve ILP or prefetch data, I'm not sure.
Yes, that defeats the branch, if it is used as an optimization, and I had to
prevent this by adding compiler fences in those cases. Granted, in my case it
happened with inlined code, but I imagine LTO makes it possible to perform
similar code transformations with out-of-line code as well.

I'm not saying this is what actually happens. I'm just pointing out that even
the code that is apparently unreachable may influence codegen and cause ODR
issues.

> > AFAIK, the standard or libstdc++ docs do not require RTTI for std::locale to
> > function.
> 
> The standard requires RTTI, period. Using -fno-rtti is completely
> non-standard and so the standard has nothing to say about it.

Yes, but the standard is written with implementations in mind.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [Bug libstdc++/113099] locale without RTTI uses dynamic_cast before gcc 13.2 or has ODR violation since gcc 13.2
  2023-12-20 23:41 [Bug libstdc++/113099] New: locale without RTTI uses dynamic_cast before gcc 13.2 or has ODR violation since gcc 13.2 andysem at mail dot ru
                   ` (12 preceding siblings ...)
  2023-12-26 15:20 ` andysem at mail dot ru
@ 2024-01-03 12:38 ` redi at gcc dot gnu.org
  2024-01-03 12:43 ` redi at gcc dot gnu.org
  2024-01-05 10:23 ` cvs-commit at gcc dot gnu.org
  15 siblings, 0 replies; 17+ messages in thread
From: redi at gcc dot gnu.org @ 2024-01-03 12:38 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113099

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
             Status|UNCONFIRMED                 |RESOLVED
   Target Milestone|---                         |13.2

--- Comment #14 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to andysem from comment #13)
> (In reply to Jonathan Wakely from comment #12)
> > (In reply to andysem from comment #11)
> > > > I'm not sure what you mean by "the compiler is free to generate code that takes it into account." Takes what into account? What problem does that freedom cause?
> > > 
> > > I mean the compiler could move (some part of) dynamic_cast to precede the
> > > check for the standard facet. I.e. of something like this:
> > > 
> > >   template< typename _Facet >
> > >   const _Facet* __try_use_facet(locale const& loc)
> > >   {
> > >     const size_t __i = _Facet::id._M_id();
> > >     const locale::facet** __facets = __loc._M_impl->_M_facets;
> > >     const _Facet* __dyn_facet = __dynamic_cast< const _Facet*
> > > >(__facets[__i]);
> > > 
> > >     // checks for every standard facet type
> > >     if (__is_same(_Facet, ...))
> > >       return static_cast<const _Facet*>(__facets[__i]);
> > > 
> > >     return __dyn_facet;
> > >   }
> > 
> > But why? Maybe I'm being slow but I still don't understand what problem is
> > being solved here.
> > 
> > Also this would defeat the optimization that avoids the unnecessary overhead
> > of dynamic_cast for standard facets.
> 
> I have seen gcc sometimes reorder code like this (i.e. move code from under
> a branch before it), presumably to improve ILP or prefetch data, I'm not
> sure. Yes, that defeats the branch, if it is used as an optimization, and I
> had to prevent this by adding compiler fences in those cases. Granted, in my
> case it happened with inlined code, but I imagine LTO makes it possible to
> perform similar code transformations with out-of-line code as well.


Building libstdc++ with LTO is unsupported (and doesn't happen when building
GCC, it requires making custom changes to the build process, which again is
unsupported). That's a non-issue.

> I'm not saying this is what actually happens. I'm just pointing out that
> even the code that is apparently unreachable may influence codegen and cause
> ODR issues.

I'm still not seeing an actual problem here, just speculation.

The compiler knows that `if (__is_same(T,T))` is true, and will not move
unreachable code before it unless that code has no side effects. And if it has
no side effects, there's no problem.

Since GCC 13.2 there is no change in semantics or observable side effects when
the explicit instantiations for std::use_facet (which are compiled with -frtti)
are used, or the function is inlined (which might be compiled with -fno-rtti).

For the gcc-11 and gcc-12 branches would could suppress the explicit
instantiation declarations for -fno-rtti, so that the extern template
definitions do not try to use dynamic_cast on a type that has no RTTI. But then
we'd need to backport (at least part of) the PR 103755 changes to use
static_cast for standard facets. Otherwise std::use_facet and std::has_facet
would always fail, even for the standard facets which are guaranteed to be
present. I don't think we want to do that on the stable branches, so I think
this is WONTFIX for GCC 13.1 and earlier, and FIXED for 13.2 and later.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [Bug libstdc++/113099] locale without RTTI uses dynamic_cast before gcc 13.2 or has ODR violation since gcc 13.2
  2023-12-20 23:41 [Bug libstdc++/113099] New: locale without RTTI uses dynamic_cast before gcc 13.2 or has ODR violation since gcc 13.2 andysem at mail dot ru
                   ` (13 preceding siblings ...)
  2024-01-03 12:38 ` redi at gcc dot gnu.org
@ 2024-01-03 12:43 ` redi at gcc dot gnu.org
  2024-01-05 10:23 ` cvs-commit at gcc dot gnu.org
  15 siblings, 0 replies; 17+ messages in thread
From: redi at gcc dot gnu.org @ 2024-01-03 12:43 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113099

--- Comment #15 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to Jonathan Wakely from comment #8)
> (In reply to Peter Dimov from comment #7)
> > You don't necessarily need dynamic_cast because facets are always installed
> > and obtained by their exact type, not via a reference to base.
> 
> Is that true? std::use_facet<X> will return a reference to a facet with
> X::id but it could actually be something derived from X. e.g. a user can
> install their own facet derived from std::ctype<char>, which overrides some
> members. Code that does std::use_facet<std::ctype<char>>(loc) will get the
> user's facet, but via reference to base.

And conversely, for std::use_facet<Y> the locale might contain a base class of
Y, with the same id as Y::id, but there is no Y present. For example, given the
OP's my_codecvt, the following code must throw std::bad_cast:

    std::locale loc;
    (void) std::use_facet<my_codecvt>(loc);

There is no my_codecvt present. There is a facet with the same id, but its
dynamic type is std::codecvt<wchar_t, char, mbstate_t> not my_codecvt.

I don't see a way to avoid using dynamic_cast without an ABI change to the
std::locale in libstdc++.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [Bug libstdc++/113099] locale without RTTI uses dynamic_cast before gcc 13.2 or has ODR violation since gcc 13.2
  2023-12-20 23:41 [Bug libstdc++/113099] New: locale without RTTI uses dynamic_cast before gcc 13.2 or has ODR violation since gcc 13.2 andysem at mail dot ru
                   ` (14 preceding siblings ...)
  2024-01-03 12:43 ` redi at gcc dot gnu.org
@ 2024-01-05 10:23 ` cvs-commit at gcc dot gnu.org
  15 siblings, 0 replies; 17+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2024-01-05 10:23 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113099

--- Comment #16 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jonathan Wakely <redi@gcc.gnu.org>:

https://gcc.gnu.org/g:9f3eb93e72703f6ea30aa27d0b6fc6db62cb4a04

commit r14-6942-g9f3eb93e72703f6ea30aa27d0b6fc6db62cb4a04
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Wed Jan 3 12:23:32 2024 +0000

    libstdc++: Use if-constexpr in std::__try_use_facet [PR113099]

    As noted in the PR, we can use if-constexpr for the explicit
    instantantiation definitions that are compiled with -std=gnu++11. We
    just need to disable the -Wc++17-extensions diagnostics.

    libstdc++-v3/ChangeLog:

            PR libstdc++/113099
            * include/bits/locale_classes.tcc (__try_use_facet): Use
            if-constexpr for C++11 and up.

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2024-01-05 10:23 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-12-20 23:41 [Bug libstdc++/113099] New: locale without RTTI uses dynamic_cast before gcc 13.2 or has ODR violation since gcc 13.2 andysem at mail dot ru
2023-12-21  8:59 ` [Bug libstdc++/113099] " rguenth at gcc dot gnu.org
2023-12-21 21:49 ` redi at gcc dot gnu.org
2023-12-23  9:37 ` andysem at mail dot ru
2023-12-23  9:39 ` andysem at mail dot ru
2023-12-24 14:44 ` redi at gcc dot gnu.org
2023-12-24 14:46 ` redi at gcc dot gnu.org
2023-12-24 15:04 ` pdimov at gmail dot com
2023-12-24 15:27 ` redi at gcc dot gnu.org
2023-12-24 15:30 ` redi at gcc dot gnu.org
2023-12-24 15:40 ` pdimov at gmail dot com
2023-12-24 16:22 ` andysem at mail dot ru
2023-12-26 12:31 ` redi at gcc dot gnu.org
2023-12-26 15:20 ` andysem at mail dot ru
2024-01-03 12:38 ` redi at gcc dot gnu.org
2024-01-03 12:43 ` redi at gcc dot gnu.org
2024-01-05 10:23 ` cvs-commit at gcc dot gnu.org

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).