From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 48) id 9AE713858C1F; Tue, 2 May 2023 16:56:49 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 9AE713858C1F DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1683046609; bh=myAYXcaN2abQjfpsSqFtTEIAIhvs9uAWuM7u4QG0NfU=; h=From:To:Subject:Date:In-Reply-To:References:From; b=YL3j6ZK+B9hivAbNwxrpLXhyX66juT6rsp2UnK2Qw/k8Vqa6g8mrV4Tk9qafnmzJd piD0YOAeHG5WgjbyW6anJzb33WQ1lXqFlf2c/VZ1DK5UXeb7pHc+6g0riQouv2OPGQ utdMrIEz9LlnGjq+Bua24K2UiGHUTyyVyZlwlKTY= From: "redi at gcc dot gnu.org" To: gcc-bugs@gcc.gnu.org Subject: [Bug libstdc++/103240] std::type_info::before gives wrong answer for ARM EABI Date: Tue, 02 May 2023 16:56:49 +0000 X-Bugzilla-Reason: CC X-Bugzilla-Type: changed X-Bugzilla-Watch-Reason: None X-Bugzilla-Product: gcc X-Bugzilla-Component: libstdc++ X-Bugzilla-Version: 12.0 X-Bugzilla-Keywords: ABI, wrong-code X-Bugzilla-Severity: normal X-Bugzilla-Who: redi at gcc dot gnu.org X-Bugzilla-Status: ASSIGNED X-Bugzilla-Resolution: X-Bugzilla-Priority: P3 X-Bugzilla-Assigned-To: redi at gcc dot gnu.org X-Bugzilla-Target-Milestone: --- X-Bugzilla-Flags: X-Bugzilla-Changed-Fields: Message-ID: In-Reply-To: References: Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Bugzilla-URL: http://gcc.gnu.org/bugzilla/ Auto-Submitted: auto-generated MIME-Version: 1.0 List-Id: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=3D103240 --- Comment #6 from Jonathan Wakely --- (In reply to frankhb1989 from comment #5) > There are multiple issues. >=20 > 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, s= ee > 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 comparis= ons, 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=3D1' 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 fai= l, > 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::befo= re > 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 st= ring 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, beca= use 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] =3D=3D '*') ? name () < arg.name () - : __builtin_strcmp (name (), arg.name ()) < 0; +#if !__GXX_MERGED_TYPEINFO_NAMES + if (__name[0] =3D=3D '*' || arg.__name[0] =3D=3D '*') + return __builtin_strcmp (__name, arg.__name) < 0; #endif + return __name < arg.__name } #endif=