[Bug libstdc++/103240] New: std::type_info::before gives wrong answer for ARM EABI

[Bug libstdc++/103240] New: std::type_info::before gives wrong answer for ARM EABI

[redi at gcc dot gnu.org]

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

            Bug ID: 103240
           Summary: std::type_info::before gives wrong answer for ARM EABI
           Product: gcc
           Version: 12.0
            Status: UNCONFIRMED
          Keywords: ABI
          Severity: normal
          Priority: P3
         Component: libstdc++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: redi at gcc dot gnu.org
  Target Milestone: ---
            Target: arm*-*-*gnueabi

This passes on x86_64 but fails on armv7hl:

// f1.C
#include <typeinfo>

namespace
{
  struct S { };
}

std::type_info const& t1 = typeid(S);

// f2.C
#include <typeinfo>
#include <cassert>

extern std::type_info const& t1;

namespace
{
  struct S { };
}

std::type_info const& t2 = typeid(S);

int main()
{
  if (t1 == t2)
  {
    assert( !t1.before(t2) );
    assert( !t2.before(t1) );
  }
  else
  {
    assert( t1.before(t2) || t2.before(t1) );
  }
}

$ g++ f1.C f2.C
$ ./a.out
a.out: f2.C:22: int main(): Assertion `t1.before(t2) || t2.before(t1)' failed.
qemu: uncaught target signal 6 (Aborted) - core dumped
Aborted (core dumped)

The problem is in libsupc++/tinfo2.cc

bool
type_info::before (const type_info &arg) const _GLIBCXX_NOEXCEPT
{
#if __GXX_MERGED_TYPEINFO_NAMES
  return name () < arg.name ();
#else
  return (name ()[0] == '*') ? name () < arg.name ()
    :  __builtin_strcmp (name (), arg.name ()) < 0;
#endif
}

This should have been fixed like operator== was in r179236 so that it checks
__name[0] not name()[0], otherwise the '*' is skipped and so is never found
there. That means we do a strcmp comparison of the mangled names, even when the
'*' says the types must be compared by pointer address. In the testcase the two
structs have the same mangled name, even though they are distinct types (due to
the unnamed namespaces). The string comparison therefore returns 0, and so
neither type is "before" the other, despite being distinct types.

[Bug libstdc++/103240] std::type_info::before gives wrong answer for ARM EABI

[redi at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2021-11-15
     Ever confirmed|0                           |1

[Bug libstdc++/103240] std::type_info::before gives wrong answer for ARM EABI

[redi at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|unassigned at gcc dot gnu.org      |redi at gcc dot gnu.org
             Status|NEW                         |ASSIGNED

[Bug libstdc++/103240] std::type_info::before gives wrong answer for ARM EABI

[cvs-commit at gcc dot gnu.org]

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

--- Comment #1 from CVS 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:054bf99841aad3869c70643b2ba2d9f85770c980

commit r12-5342-g054bf99841aad3869c70643b2ba2d9f85770c980
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Tue Nov 16 21:03:21 2021 +0000

    libstdc++: Fix std::type_info::before for ARM [PR103240]

    The r179236 fix for std::type_info::operator== should also have been
    applied to std::type_info::before. Otherwise two distinct types can
    compare equivalent due to using a string comparison, when they should do
    a pointer comparison.

    libstdc++-v3/ChangeLog:

            PR libstdc++/103240
            * libsupc++/tinfo2.cc (type_info::before): Use unadjusted name
            to check for the '*' prefix.
            * testsuite/util/testsuite_shared.cc: Add type_info object for
            use in new test.
            * testsuite/18_support/type_info/103240.cc: New test.

[Bug libstdc++/103240] std::type_info::before gives wrong answer for ARM EABI

[redi at gcc dot gnu.org]

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

--- Comment #2 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Fixed on trunk, backports to follow.

[Bug libstdc++/103240] std::type_info::before gives wrong answer for ARM EABI

[cvs-commit at gcc dot gnu.org]

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

--- Comment #3 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-11 branch has been updated by Jonathan Wakely
<redi@gcc.gnu.org>:

https://gcc.gnu.org/g:ec6ba81a038be488cec5e3bdc4f30b7876d9c5ea

commit r11-9270-gec6ba81a038be488cec5e3bdc4f30b7876d9c5ea
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Tue Nov 16 21:03:21 2021 +0000

    libstdc++: Fix std::type_info::before for ARM [PR103240]

    The r179236 fix for std::type_info::operator== should also have been
    applied to std::type_info::before. Otherwise two distinct types can
    compare equivalent due to using a string comparison, when they should do
    a pointer comparison.

    libstdc++-v3/ChangeLog:

            PR libstdc++/103240
            * libsupc++/tinfo2.cc (type_info::before): Use unadjusted name
            to check for the '*' prefix.
            * testsuite/util/testsuite_shared.cc: Add type_info object for
            use in new test.
            * testsuite/18_support/type_info/103240.cc: New test.

    (cherry picked from commit 054bf99841aad3869c70643b2ba2d9f85770c980)

[Bug libstdc++/103240] std::type_info::before gives wrong answer for ARM EABI

[frankhb1989 at gmail dot com]

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

frankhb1989 at gmail dot com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |frankhb1989 at gmail dot com

--- Comment #4 from frankhb1989 at gmail dot com ---
This should affect all targets without __GXX_MERGED_TYPEINFO_NAMES enabled.
Actually the test case also fails on (some old versions of) MinGW GCC in MSYS2.

[Bug libstdc++/103240] std::type_info::before gives wrong answer for ARM EABI

[frankhb1989 at gmail dot com]

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

--- Comment #5 from frankhb1989 at gmail dot com ---
There are multiple issues.

1. The out-of-line definition and the inline definition of
std::type_info::before do different things. Specifically, only one name is
detected against to '*' in the former, but two in the latter. It seems the
latter is more correct. ("More", because it is still not quite correct, see
below.)

2. As stated in https://reviews.llvm.org/D100134, mixture of different methods
of comparisons breaks the ordering.

I've actually encountered the problem with std::type_index as the key type when
using flat_map::emplace (like
https://github.com/WG21-SG14/SG14/blob/master/SG14/flat_map.h, which uses
std::partition_point to find the position of insertion). The insertion suddenly
fails when std::partition_point meets two std::type_info objects x and y which
are unordered. It works with the workaround '-U__GXX_MERGED_TYPEINFO_NAMES
-D__GXX_MERGED_TYPEINFO_NAMES=1' in the compiler command line, but it seems not
enough in general without fixing the issue 2 here. (Although the same sequence
of key on std::map does not fail, occasionally, due to less comparisons are
made.)

3. Not sure the original change in r179236 is correct.

4. '||' in the condition of the inline definition of std::type_info::before
seems an overkill. '&&' may be enough. Assuming string comparison is more
expensive, this is a simple optimization. But once the issue 2 is fixed, the
change would look quite different.

[Bug libstdc++/103240] std::type_info::before gives wrong answer for ARM EABI

[redi at gcc dot gnu.org]

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

--- Comment #6 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to frankhb1989 from comment #5)
> There are multiple issues.
> 
> 1. The out-of-line definition and the inline definition of
> std::type_info::before do different things. Specifically, only one name is
> detected against to '*' in the former, but two in the latter. It seems the
> latter is more correct. ("More", because it is still not quite correct, see
> below.)

I think the latter is completely correct.

> 2. As stated in https://reviews.llvm.org/D100134, mixture of different
> methods of comparisons breaks the ordering.

I don't think that's a problem for our inline implementation.

For non-unique A, unique B, non-unique C we would always do string comparisons,
and we would consistently have "*B" < "A" < "C" in all TUs.

We would only do pointer comparisons for when comparing two unique types, e.g.
comparing "*B" and "*B" or comparing "*B" and "*D".

> I've actually encountered the problem with std::type_index as the key type
> when using flat_map::emplace (like
> https://github.com/WG21-SG14/SG14/blob/master/SG14/flat_map.h, which uses
> std::partition_point to find the position of insertion). The insertion
> suddenly fails when std::partition_point meets two std::type_info objects x
> and y which are unordered. It works with the workaround
> '-U__GXX_MERGED_TYPEINFO_NAMES -D__GXX_MERGED_TYPEINFO_NAMES=1' in the
> compiler command line, but it seems not enough in general without fixing the
> issue 2 here. (Although the same sequence of key on std::map does not fail,
> occasionally, due to less comparisons are made.)

What are the mangled type names that are unordered? (that's all you need to
describe, everything about flat_map and partition_point is irrelevant; just
tell us which names are unordered).

Is this using the inline implementation? (I assume so, otherwise redefining
__GXX_MERGED_TYPEINFO_NAMES won't do anything). How does forcing address
comparisons for all types solve anything? That isn't just "not enough in
general" it's **wrong** in general.

> 3. Not sure the original change in r179236 is correct.

It was an improvement, at least.

> 4. '||' in the condition of the inline definition of std::type_info::before
> seems an overkill. '&&' may be enough. Assuming string comparison is more
> expensive, this is a simple optimization. But once the issue 2 is fixed, the
> change would look quite different.

I don't think that's correct. We can only do a pointer comparison if both
strings begin with '*'. If we compared pointers for "*foo" and "bar" (where
"bar" is not unique), we might have "*foo" < "bar"  in one TU and "*foo" >
"bar" in another TU where "bar" has a different address. We need to do a string
comparison if either of them is not unique. Which is what the inline
implementation does.

It also correctly handles the problem case described by Richard Smith, because
all unique names start with '*' and all non-unique names start with one of
[A-Za-z_], and those are ordered after '*', at least for ASCII and UTF-8.

I agree there are issues with the non-inline implementation. We could just make
it match the inline one:

--- a/libstdc++-v3/libsupc++/tinfo2.cc
+++ b/libstdc++-v3/libsupc++/tinfo2.cc
@@ -33,15 +33,11 @@ using std::type_info;
 bool
 type_info::before (const type_info &arg) const _GLIBCXX_NOEXCEPT
 {
-#if __GXX_MERGED_TYPEINFO_NAMES
-  return name () < arg.name ();
-#else
-  /* The name() method will strip any leading '*' prefix. Therefore
-     take care to look at __name rather than name() when looking for
-     the "pointer" prefix.  */
-  return (__name[0] == '*') ? name () < arg.name ()
-    :  __builtin_strcmp (name (), arg.name ()) < 0;
+#if !__GXX_MERGED_TYPEINFO_NAMES
+  if (__name[0] == '*' || arg.__name[0] == '*')
+    return __builtin_strcmp (__name, arg.__name) < 0;
 #endif
+  return __name < arg.__name
 }

 #endif

[Bug libstdc++/103240] std::type_info::before gives wrong answer for ARM EABI

[frankhb1989 at gmail dot com]

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

--- Comment #7 from frankhb1989 at gmail dot com ---
(In reply to Jonathan Wakely from comment #6)

> What are the mangled type names that are unordered? (that's all you need to
> describe, everything about flat_map and partition_point is irrelevant; just
> tell us which names are unordered).

Here are the pair of the unordered names in the actual case:

1.
St5_BindIFPFN3NPL15ReductionStatusERNS0_8TermNodeERNS0_11ContextNodeEESt17reference_wrapperIS2_ESt12_PlaceholderILi1EEEE
2.
St5_BindIFZN3NPL2A112_GLOBAL__N_113DoDefineOrSetILb0EE4CallIJEEENS0_15ReductionStatusERNS0_8TermNodeERNS0_11ContextNodeESt10shared_ptrINS0_11EnvironmentEES8_DpOT_EUlRKS7_RKSD_E_S7_SD_EE

Here is the reduced program (exactly from the actual case) to show the sequence
and the ordering:

#include <iostream>
#include <typeinfo>
#include <typeindex>
#include <cassert>
#include <vector>
#include <functional>
#include <memory>

std::vector<std::type_index> v;

namespace ystdex
{

template<typename, typename>
struct expanded_caller
{};

}

namespace NPL
{

enum class ReductionStatus
{
        Clean
};

struct TermNode
{};

struct ContextNode
{};

struct Environment
{};

namespace A1
{

template<typename T>
void
bar()
{
        static struct Init
        {
                Init()
                {
                //      std::cout << typeid(T).name() << std::endl;
                        v.push_back(typeid(T));
                }
        } init;
}

template<typename C>
void
foo(C&&)
{
        bar<ystdex::expanded_caller<ReductionStatus(ContextNode&), C>>();
}

template<typename C>
void
foo2(C&&)
{
        bar<C>();
}

namespace
{

struct EvalSequence
{};

}

namespace
{

template<bool = false>
struct DoDefineOrSet
{
        template<typename... A>
        static ReductionStatus
        Call(TermNode& term, ContextNode& ctx, std::shared_ptr<Environment> p,
                TermNode& t, A&&... args)
        {
                foo2(std::bind([&](const TermNode& saved,
                        const std::shared_ptr<Environment>& p_e, const A&...){
                        return ReductionStatus::Clean;
                }, std::move(t), std::move(p), std::move(args)...));
                return ReductionStatus::Clean;
        }
};

template<unsigned long long, unsigned long long>
ReductionStatus
LambdaVauWithEnvironment(TermNode&, ContextNode&, bool)
{
        []{};
        foo([]{});
        return ReductionStatus::Clean;
}

}

}

ReductionStatus
f1(TermNode&, ContextNode&)
{
        return ReductionStatus::Clean;
}

}

int main()
{
        using namespace std;
        using namespace placeholders;
        using namespace NPL;
        using namespace A1;
        TermNode t;
        ContextNode c;

        cout << "__GXX_TYPEINFO_EQUALITY_INLINE = " <<
__GXX_TYPEINFO_EQUALITY_INLINE << endl;
        cout << "__GXX_MERGED_TYPEINFO_NAMES = " << __GXX_MERGED_TYPEINFO_NAMES
<< endl;

        foo2(bind(f1, ref(t), _1));
        foo2(EvalSequence{});
        DoDefineOrSet<false>::Call(t, c, {}, t);
        LambdaVauWithEnvironment<1, 1>(t, c, true);
        for(auto& id : v)
                cout << "i = " << (&id - &v[0]) << ", v[i] = " << id.name() <<
endl;
        for(std::size_t i = 0; i < v.size(); ++i)
                for(auto j = i; j < v.size(); ++j)
                        if(i != j)
                                cout << "i = " << i << ", " << "j = " << j <<
", v[i] < v[j] = " << (v[i] < v[j]) << ", v[j] < v[i] = " << (v[j] < v[i]) <<
endl;
}

//      g++ -std=c++11 -U__GXX_TYPEINFO_EQUALITY_INLINE
-D__GXX_TYPEINFO_EQUALITY_INLINE=0 -U__GXX_MERGED_TYPEINFO_NAMES
-D__GXX_MERGED_TYPEINFO_NAMES=0 a.cc

This is tested with x86_64-w64-mingw32-g++. Linking may fail on platforms where
libstdc++ is configured without the out-of-line definition of
std::type_info::before. Here is the output:

__GXX_TYPEINFO_EQUALITY_INLINE = 0
__GXX_MERGED_TYPEINFO_NAMES = 0
i = 0, v[i] =
St5_BindIFPFN3NPL15ReductionStatusERNS0_8TermNodeERNS0_11ContextNodeEESt17reference_wrapperIS2_ESt12_PlaceholderILi1EEEE
i = 1, v[i] = N3NPL2A112_GLOBAL__N_112EvalSequenceE
i = 2, v[i] =
St5_BindIFZN3NPL2A112_GLOBAL__N_113DoDefineOrSetILb0EE4CallIJEEENS0_15ReductionStatusERNS0_8TermNodeERNS0_11ContextNodeESt10shared_ptrINS0_11EnvironmentEES8_DpOT_EUlRKS7_RKSD_E_S7_SD_EE
i = 3, v[i] =
N6ystdex15expanded_callerIFN3NPL15ReductionStatusERNS1_11ContextNodeEEZNS1_2A112_GLOBAL__N_124LambdaVauWithEnvironmentILy1ELy1EEES2_RNS1_8TermNodeES4_bEUlvE0_EE
i = 0, j = 1, v[i] < v[j] = 0, v[j] < v[i] = 1
i = 0, j = 2, v[i] < v[j] = 1, v[j] < v[i] = 1
i = 0, j = 3, v[i] < v[j] = 0, v[j] < v[i] = 1
i = 1, j = 2, v[i] < v[j] = 0, v[j] < v[i] = 1
i = 1, j = 3, v[i] < v[j] = 0, v[j] < v[i] = 1
i = 2, j = 3, v[i] < v[j] = 0, v[j] < v[i] = 1

The unordered pair occurs iff. __GXX_TYPEINFO_EQUALITY_INLINE == 0, i.e. the
out-of-line definition is used, and regardless to the value of
__GXX_MERGED_TYPEINFO_NAMES (either 0 or 1).

The sequence (v[0], v[1], ...) here is exactly the inserted one in the actual
case, where the underlying sequence (of type vector<type_index>) using
less<type_index> as key_compare. Insertions of v[0] and v[1] are OK, and then
insertion of v[2] breaks the assumption of ordering in the vector. At this
point it is already corrupted. Assuming it is correct when
__GXX_TYPEINFO_EQUALITY_INLINE == 1, !(v[0] < v[2]) should hold, so the
resulted ordered vector is (v[2], v[1], v[0]) rather than the buggy (v[1],
v[2], v[0]). In this case, given that all elements are different, the further
insertions would not fail, as the container just contains all elements with an
arbitrary order. The bug would not show until some operations like find or
lower_bound. However, for some reasons (address space layout?), in the actual
case, the order is reversed. Then there occurs the pair where (!(v[0] < v[2])
&& !(v[2] < v[0])), which is treated as equivalent keys as per the assumption
of the ordered container. Thus, the insertion fail immediately.

> 
> Is this using the inline implementation? (I assume so, otherwise redefining
> __GXX_MERGED_TYPEINFO_NAMES won't do anything). How does forcing address
> comparisons for all types solve anything? That isn't just "not enough in
> general" it's **wrong** in general.

As above, I've later tested all 4 combinations for sure. Accidentally the
inline definition of std::type_info::before does the right thing (hopefully),
so __GXX_TYPEINFO_EQUALITY_INLINE == 1 just works. Otherwise there would be no
easy workaround without the modification on the standard headers.

Forcing address comparisons is wrong in general, but with some additional
assumptions to rule out all potential offending features, then all type_info
objects follow ODR in the strict sense, so this just works. When this is an
issue, __GXX_TYPEINFO_EQUALITY_INLINE == 1 && __GXX_MERGED_TYPEINFO_NAMES == 0
should be safe for all names not from unnamed namespaces. This is a real
problem for MinGW (at least with GNU ld which does not perform ICF on PE/COFF
AFAIK), where __GXX_TYPEINFO_EQUALITY_INLINE == 1 &&
__GXX_MERGED_TYPEINFO_NAMES == 1 causes something like &typeid(shared_ptr<...>)
not unique across module boundaries, and my code fails elsewhere due to this
reason.

> 
> I don't think that's correct. We can only do a pointer comparison if both
> strings begin with '*'. If we compared pointers for "*foo" and "bar" (where
> "bar" is not unique), we might have "*foo" < "bar"  in one TU and "*foo" >
> "bar" in another TU where "bar" has a different address. We need to do a
> string comparison if either of them is not unique. Which is what the inline
> implementation does.
> 
> It also correctly handles the problem case described by Richard Smith,
> because all unique names start with '*' and all non-unique names start with
> one of [A-Za-z_], and those are ordered after '*', at least for ASCII and
> UTF-8.
> 

OK, it is just a bit difficult to reason about. The rule of '*' seems not in
the Itanium ABI spec. Is there any normative documentation out of the GCC repo?
(BTW, IIRC Clang++ just does not yet implement things about '*'.)

> I agree there are issues with the non-inline implementation. We could just
> make it match the inline one:
> 
> --- a/libstdc++-v3/libsupc++/tinfo2.cc
> +++ b/libstdc++-v3/libsupc++/tinfo2.cc
> @@ -33,15 +33,11 @@ using std::type_info;
>  bool
>  type_info::before (const type_info &arg) const _GLIBCXX_NOEXCEPT
>  {
> -#if __GXX_MERGED_TYPEINFO_NAMES
> -  return name () < arg.name ();
> -#else
> -  /* The name() method will strip any leading '*' prefix. Therefore
> -     take care to look at __name rather than name() when looking for
> -     the "pointer" prefix.  */
> -  return (__name[0] == '*') ? name () < arg.name ()
> -    :  __builtin_strcmp (name (), arg.name ()) < 0;
> +#if !__GXX_MERGED_TYPEINFO_NAMES
> +  if (__name[0] == '*' || arg.__name[0] == '*')
> +    return __builtin_strcmp (__name, arg.__name) < 0;
>  #endif
> +  return __name < arg.__name
>  }
>  
>  #endif

LGTM.

[Bug libstdc++/103240] std::type_info::before gives wrong answer for ARM EABI

[redi at gcc dot gnu.org]

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

--- Comment #8 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to frankhb1989 from comment #7)
> (In reply to Jonathan Wakely from comment #6)
> 
> > What are the mangled type names that are unordered? (that's all you need to
> > describe, everything about flat_map and partition_point is irrelevant; just
> > tell us which names are unordered).
> 
> Here are the pair of the unordered names in the actual case:
> 
> 1.
> St5_BindIFPFN3NPL15ReductionStatusERNS0_8TermNodeERNS0_11ContextNodeEESt17ref
> erence_wrapperIS2_ESt12_PlaceholderILi1EEEE
> 2.
> St5_BindIFZN3NPL2A112_GLOBAL__N_113DoDefineOrSetILb0EE4CallIJEEENS0_15Reducti
> onStatusERNS0_8TermNodeERNS0_11ContextNodeESt10shared_ptrINS0_11EnvironmentEE
> S8_DpOT_EUlRKS7_RKSD_E_S7_SD_EE

Thanks. This second one has "*St5_BindIFZN3NPL..." in type_info::__name, so
yes, they'll be incorrectly ordered with the out-of-line definition.

t1.before(t2) will do a string comparison, but t2.before(t1) will do a pointer
comparison and depend on the string table layout.

> The unordered pair occurs iff. __GXX_TYPEINFO_EQUALITY_INLINE == 0, i.e. the
> out-of-line definition is used, and regardless to the value of
> __GXX_MERGED_TYPEINFO_NAMES (either 0 or 1).

I don't think that's true. If __GXX_MERGED_TYPEINFO_NAMES is true then the
out-of-line definition is correct. You can't just redefine that macro to 1 in
your code and expect it to affect the out-of-line definition in the library.
Even if you recompile the library with 
-D__GXX_MERGED_TYPEINFO_NAMES=1 that doesn't magically make it true. If the
platform ABI and compiler and linker really do guarantee that all typeinfo
names are unique, then address comparison works correctly.

The problem case is when both __GXX_MERGED_TYPEINFO_NAMES and
__GXX_TYPEINFO_EQUALITY_INLINE are zero.


> As above, I've later tested all 4 combinations for sure.

You can't expect that to give meaningful results though, you would need to
rebuild the entire compiler for meaningful results with redefined macros, and
even then it would fail when you use RTTI across dynamic library boundaries.

You can't just redefine those macros and expect the compiler and OS to change
how they work.

> Accidentally the
> inline definition of std::type_info::before does the right thing

I don't think it's an accident.

> (hopefully), so __GXX_TYPEINFO_EQUALITY_INLINE == 1 just works. Otherwise
> there would be no easy workaround without the modification on the standard
> headers.
> 
> Forcing address comparisons is wrong in general, but with some additional
> assumptions to rule out all potential offending features, then all type_info
> objects follow ODR in the strict sense, so this just works. When this is an
> issue, __GXX_TYPEINFO_EQUALITY_INLINE == 1 && __GXX_MERGED_TYPEINFO_NAMES ==
> 0 should be safe for all names not from unnamed namespaces. This is a real
> problem for MinGW (at least with GNU ld which does not perform ICF on
> PE/COFF AFAIK), where __GXX_TYPEINFO_EQUALITY_INLINE == 1 &&
> __GXX_MERGED_TYPEINFO_NAMES == 1 causes something like
> &typeid(shared_ptr<...>) not unique across module boundaries, and my code
> fails elsewhere due to this reason.

So stop redefining those macros then, you're lying to the compiler.

__GXX_MERGED_TYPEINFO_NAMES=1 is a lie on your target. Don't do that.

[Bug libstdc++/103240] std::type_info::before gives wrong answer for ARM EABI

[frankhb1989 at gmail dot com]

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

--- Comment #9 from frankhb1989 at gmail dot com ---
(In reply to Jonathan Wakely from comment #8)

> I don't think that's true. If __GXX_MERGED_TYPEINFO_NAMES is true then the
> out-of-line definition is correct. You can't just redefine that macro to 1
> in your code and expect it to affect the out-of-line definition in the
> library. Even if you recompile the library with 
> -D__GXX_MERGED_TYPEINFO_NAMES=1 that doesn't magically make it true. If the
> platform ABI and compiler and linker really do guarantee that all typeinfo
> names are unique, then address comparison works correctly.
> 

Yes, -D__GXX_MERGED_TYPEINFO_NAMES=1 is harmful and unnecessary to work around
the issue. As illustrated, it just works for some of my cases accidentally, and
fails for some other cases.

Further, I want to make sure no out-of-line definition of type_info::before is
called in the system libraries (which I can't rebuild). Does libstdc++ have
calls to type_info::before internally?

> You can't expect that to give meaningful results though, you would need to
> rebuild the entire compiler for meaningful results with redefined macros,
> and even then it would fail when you use RTTI across dynamic library
> boundaries.
> 
> You can't just redefine those macros and expect the compiler and OS to
> change how they work.
> 

I don't quite get this point about the compiler. Do you mean not only libstdc++
and the linker, but also the compiler's codegen of the mangled names works
differently depending on the macro definitions, besides the possible duplicate
(but having identical mangled names) definitions of the internal objects
providing the mangle names? Will it affect the section layout of the object
code? Are there existing optimizations relying on the assumptions?

I know it will not be formally supported to have consistent behavior just by
rebuilding the user programs, and it will be plain wrong without things like
ICF or in cases dynamic loading is relied on, but I'm curious: what are the
actual consequences from the compiler's view, when (1) all TUs of the program
code eventually use these macro definitions consistently, (2) definitions of
RTTI information for identical types are totally merged; and, (3) each instance
of the call to the out-of-line definition of type_info::before is avoided?

I am interesting in such questions because there are cases where rebuiding the
toolchain is impossible. It is not even possible to rebuilt libstdc++ for
production system unless more than one instance can be deployed side by side
(e.g. by static linking), because I cannot make sure no other part of the
environment have relied on the bug-to-bug compatibility to the existing system
libraries. In my case, I have to make sure which parts of the deployed code are
actually affected before the toolchain update (either system-wide or not) is
possible. And before that, the redefinition of macros seems the only viable
workaround (with certain limitations).

> > Accidentally the
> > inline definition of std::type_info::before does the right thing
> 
> I don't think it's an accident.
> 

Good news to me.

> > (hopefully), so __GXX_TYPEINFO_EQUALITY_INLINE == 1 just works. Otherwise
> > there would be no easy workaround without the modification on the standard
> > headers.
> > 
> > Forcing address comparisons is wrong in general, but with some additional
> > assumptions to rule out all potential offending features, then all type_info
> > objects follow ODR in the strict sense, so this just works. When this is an
> > issue, __GXX_TYPEINFO_EQUALITY_INLINE == 1 && __GXX_MERGED_TYPEINFO_NAMES ==
> > 0 should be safe for all names not from unnamed namespaces. This is a real
> > problem for MinGW (at least with GNU ld which does not perform ICF on
> > PE/COFF AFAIK), where __GXX_TYPEINFO_EQUALITY_INLINE == 1 &&
> > __GXX_MERGED_TYPEINFO_NAMES == 1 causes something like
> > &typeid(shared_ptr<...>) not unique across module boundaries, and my code
> > fails elsewhere due to this reason.
> 
> So stop redefining those macros then, you're lying to the compiler.
> 

Sadly, we *have to* lie for some degrees... ODR in ISO C++ is already
conceptually violated in cases when dynamic libraries are taken into account.
There is no such strong guarantee (like ICF) for all implementations, so
implementation-specific details are already kicked in.

> __GXX_MERGED_TYPEINFO_NAMES=1 is a lie on your target. Don't do that.

Agreed for the current case, though.