[Bug libstdc++/103240] std::type_info::before gives wrong answer for ARM EABI
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 > > (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.
[Bug libstdc++/103240] std::type_info::before gives wrong answer for ARM EABI
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103240 --- Comment #8 from Jonathan Wakely --- (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_PlaceholderILi1 > 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 > (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
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_PlaceholderILi1
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
#include
#include
#include
#include
#include
#include
std::vector v;
namespace ystdex
{
template
struct expanded_caller
{};
}
namespace NPL
{
enum class ReductionStatus
{
Clean
};
struct TermNode
{};
struct ContextNode
{};
struct Environment
{};
namespace A1
{
template
void
bar()
{
static struct Init
{
Init()
{
// std::cout << typeid(T).name() << std::endl;
v.push_back(typeid(T));
}
} init;
}
template
void
foo(C&&)
{
bar>();
}
template
void
foo2(C&&)
{
bar();
}
namespace
{
struct EvalSequence
{};
}
namespace
{
template
struct DoDefineOrSet
{
template
static ReductionStatus
Call(TermNode& term, ContextNode& ctx, std::shared_ptr p,
TermNode& t, A&&... args)
{
foo2(std::bind([&](const TermNode& saved,
const std::shared_ptr& p_e, const A&...){
return ReductionStatus::Clean;
}, std::move(t), std::move(p), std::move(args)...));
return ReductionStatus::Clean;
}
};
template
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::Call(t, c, {}, t);
LambdaVauWithEnvironment<1, 1>(t, c, true);
for(auto& id : v)
cout << "i = " << ( - [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_PlaceholderILi1
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) using
less 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
[Bug libstdc++/103240] std::type_info::before gives wrong answer for ARM EABI
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103240
--- Comment #6 from Jonathan Wakely ---
(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 ) 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
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
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
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103240 --- Comment #3 from CVS Commits --- The releases/gcc-11 branch has been updated by Jonathan Wakely : https://gcc.gnu.org/g:ec6ba81a038be488cec5e3bdc4f30b7876d9c5ea commit r11-9270-gec6ba81a038be488cec5e3bdc4f30b7876d9c5ea Author: Jonathan Wakely Date: Tue Nov 16 21:03:21 2021 + 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
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103240 --- Comment #2 from Jonathan Wakely --- Fixed on trunk, backports to follow.
[Bug libstdc++/103240] std::type_info::before gives wrong answer for ARM EABI
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103240 --- Comment #1 from CVS Commits --- The master branch has been updated by Jonathan Wakely : https://gcc.gnu.org/g:054bf99841aad3869c70643b2ba2d9f85770c980 commit r12-5342-g054bf99841aad3869c70643b2ba2d9f85770c980 Author: Jonathan Wakely Date: Tue Nov 16 21:03:21 2021 + 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
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103240 Jonathan Wakely 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
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103240 Jonathan Wakely changed: What|Removed |Added Status|UNCONFIRMED |NEW Last reconfirmed||2021-11-15 Ever confirmed|0 |1
