From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id 454463857B98 for ; Thu, 11 Aug 2022 02:22:50 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 454463857B98 Received: from mail-qt1-f200.google.com (mail-qt1-f200.google.com [209.85.160.200]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-501-GzxjeGppNa661drHA9VZtA-1; Wed, 10 Aug 2022 22:22:48 -0400 X-MC-Unique: GzxjeGppNa661drHA9VZtA-1 Received: by mail-qt1-f200.google.com with SMTP id v5-20020ac873c5000000b003434ef0a8c7so3631250qtp.21 for ; Wed, 10 Aug 2022 19:22:48 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=to:references:message-id:content-transfer-encoding:cc:date :in-reply-to:from:subject:mime-version:x-gm-message-state:from:to:cc; bh=0kYJ2nNwuVeFrr5KVkzsT3exf9l5t/787jsa0iDTaOk=; b=Q5oCGTIrc6gHpA7XOu57k3vFs/lSgIy7BBdEsq7HIwekUhtdO2NT4+sVHLSM8qVxTs aAvjvPCG7OyvjPNz2fxUBqkdXP0nbhHCTKz8/YG1zWhFzbBJjscCPsiA6FDGz3WxgJdp zT+SFXodXz/4pu1WbcUxbIwN0VaYO6+MrQVx2KYop4rpKZHvQDwSjCWZ1EUFKcifFaj3 gWD+6Ev9z3koXNuE5y2ksdx9zYtxwBwx94QrsRdWtXTsoOS0sKvvwKV/iSJGanRHqNUZ JV9tZaNhEujwVh0ZPp9eF3QPUj/s80AdxQHq/LcZBVGKKpC3tHiOBC/etDn5b6YuYtXt Lnng== X-Gm-Message-State: ACgBeo0WWBKwz6qNZW/PXmggqTRxK+FqquX+Bhqzqdbl7akWYex5qZSb oGmfSKqaPsfY8r4+OijvFtQvsxUA7fMfNkDsS2BCw7pfQH30eXK5c5Jy/KbzxdK1NrZL6MsaDSg +11bDvSOPGOGX+PYOE1nm X-Received: by 2002:a05:6214:2aae:b0:476:b97e:1c1e with SMTP id js14-20020a0562142aae00b00476b97e1c1emr26248805qvb.126.1660184568214; Wed, 10 Aug 2022 19:22:48 -0700 (PDT) X-Google-Smtp-Source: AA6agR7YMRJJbwaPZOeePW/34ViMsMffW6LF9yVhPfCdwH4cgRV8tU0GQLJeiRaS0JxG1eaCyAB/tw== X-Received: by 2002:a05:6214:2aae:b0:476:b97e:1c1e with SMTP id js14-20020a0562142aae00b00476b97e1c1emr26248788qvb.126.1660184567795; Wed, 10 Aug 2022 19:22:47 -0700 (PDT) Received: from smtpclient.apple ([47.208.199.57]) by smtp.gmail.com with ESMTPSA id q13-20020a05620a0d8d00b006b9b319adacsm1066690qkl.126.2022.08.10.19.22.46 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Wed, 10 Aug 2022 19:22:47 -0700 (PDT) Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3696.120.41.1.1\)) Subject: Re: [PATCH 3/3] ir: Consider integral types of same kind and size as equivalent From: Ben Woodard In-Reply-To: Date: Wed, 10 Aug 2022 19:22:44 -0700 Cc: Dodji Seketeli , Dodji Seketeli , Dodji Seketeli via Libabigail , =?utf-8?Q?Matthias_M=C3=A4nnich?= Message-Id: References: <87sfms91n0.fsf@redhat.com> <87czdw910h.fsf@seketeli.org> To: Giuliano Procida X-Mailer: Apple Mail (2.3696.120.41.1.1) X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-5.9 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, RCVD_IN_DNSWL_LOW, SPF_HELO_NONE, SPF_NONE, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: libabigail@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Mailing list of the Libabigail project List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 11 Aug 2022 02:22:52 -0000 Dodji is on vacation. Thank you for double checking this. > On Aug 10, 2022, at 8:23 AM, Giuliano Procida via Libabigail wrote: >=20 > Hi Dodji. >=20 > On Sat, 23 Jul 2022 at 00:33, Dodji Seketeli wrote: >>=20 >> Hello, >>=20 >> On some platforms, "long int" and "long long int" can have the same >> size. In that case, we want those two types to be equivalent from ABI >> standpoint. Otherwise, through the use of typedefs and pointers, two >> structs "C" defined in different translation units where one uses >> "long int" in a translation unit and "long long int" in another should >> be considered ABI compatible if long int and long long int have the >> same size on that platform. >=20 > While such types may be ABI compatible, they are not API compatible as th= ey > impact (at least) C++ overload resolution. hmm maybe this kind of resolution should only apply to C linkage symbols an= d not C++ where they are in fact different.=20 You of course correct about the difference between ABI and API in this case= . It does bring up the interesting question is libabigail just an ABI change = detection tool or is it also a API change detection tool. With name manglin= g, I think that the dynamic linker will continue to wire all up the correct= function call. I can=E2=80=99t think of a case where this may not be true = but if you can, please speak up. >=20 > All of char, unsigned char, signed char, int, unsigned, long, etc. are > distinct types. > Conflating some subsets of these will result in confusing ABI > difference reports. Interestingly, I have been collaborating with people writing another ABI to= ol that would also overlook this kind of difference as well. I wonder how c= onfusing the error reports get.=20 >=20 >> Otherwise, that causes spurious type changes that lead to self >> comparison change down the road. For instance, the following command >> fails: >>=20 >> $ tools/fedabipkgdiff --debug --self-compare -a --from fc36 btrfs-pro= gs >=20 > Shouldn't any tweaking of behaviour happen with abidiff rather than abidw= ? >=20 > We might not end up with stable XML but the finger of blame should be > pointed at the btrfs-progs in any case. >=20 >> This patch thus changes the comparison engine of the IR so that the >> "short, long and long long" modifiers don't change the result of >> comparing integral types that share the same base type when they have >> the same size. >=20 > We don't want this behaviour and can carry a revert patch in AOSP or > work a way to disable it that is less likely to cause merge conflicts > in the future. >=20 > Is there an easy way of putting this under flag control? >=20 > There's also a secondary issue where base types like "int" and "long > int" now want to have the same hash-based type id and we end up with > linear probing and the XML instability that accompanies this. I expect > this was an unintended side-effect, but haven't yet looked into how it > might be resolved. >=20 > Regards, > Giuliano. >=20 >> * include/abg-fwd.h (is_integral_type): Declare new function. >> * include/abg-ir.h (type_decl::get_qualified_name): Add a >> declaration of an implementation of the virtual interface >> get_qualified_name. >> * src/abg-ir-priv.h (integral_type::set_modifiers): Define a new >> setter. >> (integral_type::to_string): Add an "internal" flag. >> * src/abg-ir.cc (operator~, operator&=3D): Declare >> new operators. >> (get_internal_integral_type_name): Define new static function. >> (decl_base::priv::{temporary_internal_qualified_name_, >> internal_qualified_name_}): Define two new data members. >> (get_type_name): For internal name of integral types, use the new >> get_internal_integral_type_name function. >> (is_integral_type): Define new function. >> (integral_type::set_modifiers): Define new member function. >> (operator|, operator&): Fix some indentation. >> (operator~, operator&=3D): Define new operators. >> (parse_integral_type): Fix the logic of this function. Namely, i= t >> wasn't handling parsing "long long" correctly. >> (integral_type::to_string): Add an "internal" flag. >> (equals): In the overload for type_decl, do not take the short, >> long and long long into account when comparing integral types of >> the same size. >> (type_decl::get_qualified_name): Define new method. >> (type_decl::get_pretty_representation): For internal name of >> integral types, use the new get_internal_integral_type_name >> function. >> ({decl,type}_topo_comp::operator()): Use the non-internal pretty >> representation of decls/types for sorting purpose. >> * src/abg-reader.cc (build_type_decl): We don't expect the >> integral type name from abixml to the same as the name of the >> parsed integral type, as the abixml file can be old and have an >> old format. >> * tests/data/test-annotate/libtest23.so.abi: Adjust. >> * tests/data/test-annotate/libtest24-drop-fns-2.so.abi: Adjust. >> * tests/data/test-annotate/libtest24-drop-fns.so.abi: Adjust. >> * tests/data/test-annotate/test0.abi: Adjust. >> * tests/data/test-annotate/test15-pr18892.so.abi: Adjust. >> * tests/data/test-annotate/test17-pr19027.so.abi: Adjust. >> * tests/data/test-annotate/test18-pr19037-libvtkRenderingLIC-6.1.= so.abi: >> Adjust. >> * tests/data/test-annotate/test19-pr19023-libtcmalloc_and_profile= r.so.abi: >> Adjust. >> * tests/data/test-annotate/test20-pr19025-libvtkParallelCore-6.1.= so.abi: >> Adjust. >> * tests/data/test-annotate/test21-pr19092.so.abi: Adjust. >> * tests/data/test-diff-dwarf/PR25058-liblttng-ctl-report-1.txt: >> Adjust. >> * tests/data/test-diff-filter/test41-report-0.txt: Adjust. >> * tests/data/test-diff-pkg/tbb-4.1-9.20130314.fc22.x86_64--tbb-4.= 3-3.20141204.fc23.x86_64-report-0.txt: >> Adjust. >> * tests/data/test-diff-pkg/tbb-4.1-9.20130314.fc22.x86_64--tbb-4.= 3-3.20141204.fc23.x86_64-report-1.txt: >> Adjust. >> * tests/data/test-diff-dwarf/PR25058-liblttng-ctl-report-1.txt: >> Adjust. >> * tests/data/test-read-dwarf/PR22015-libboost_iostreams.so.abi: >> Adjust. >> * tests/data/test-read-dwarf/PR22122-libftdc.so.abi: Adjust. >> * tests/data/test-read-dwarf/PR25007-sdhci.ko.abi: Adjust. >> * tests/data/test-read-dwarf/PR25042-libgdbm-clang-dwarf5.so.6.0.= 0.abi: >> Adjust. >> * tests/data/test-read-dwarf/libtest23.so.abi: Adjust. >> * tests/data/test-read-dwarf/libtest24-drop-fns-2.so.abi: Adjust. >> * tests/data/test-read-dwarf/libtest24-drop-fns.so.abi: Adjust. >> * tests/data/test-read-dwarf/test-PR26568-1.o.abi: Adjust. >> * tests/data/test-read-dwarf/test-PR26568-2.o.abi: Adjust. >> * tests/data/test-read-dwarf/test-libaaudio.so.abi: Adjust. >> * tests/data/test-read-dwarf/test-libandroid.so.abi: Adjust. >> * tests/data/test-read-dwarf/test0.abi: Adjust. >> * tests/data/test-read-dwarf/test0.hash.abi: Adjust. >> * tests/data/test-read-dwarf/test1.hash.abi: Adjust. >> * tests/data/test-read-dwarf/test10-pr18818-gcc.so.abi: Adjust. >> * tests/data/test-read-dwarf/test11-pr18828.so.abi: Adjust. >> * tests/data/test-read-dwarf/test12-pr18844.so.abi: Adjust. >> * tests/data/test-read-dwarf/test15-pr18892.so.abi: Adjust. >> * tests/data/test-read-dwarf/test16-pr18904.so.abi: Adjust. >> * tests/data/test-read-dwarf/test17-pr19027.so.abi: Adjust. >> * tests/data/test-read-dwarf/test18-pr19037-libvtkRenderingLIC-6.= 1.so.abi: >> Adjust. >> * tests/data/test-read-dwarf/test19-pr19023-libtcmalloc_and_profi= ler.so.abi: >> Adjust. >> * tests/data/test-read-dwarf/test20-pr19025-libvtkParallelCore-6.= 1.so.abi: >> Adjust. >> * tests/data/test-read-dwarf/test21-pr19092.so.abi: Adjust. >> * tests/data/test-read-dwarf/test22-pr19097-libstdc++.so.6.0.17.s= o.abi: >> Adjust. >> * tests/data/test-read-dwarf/test9-pr18818-clang.so.abi: Adjust. >> * tests/data/test-read-write/test22.xml: Adjust. >> * tests/data/test-read-write/test23.xml: Adjust. >> * tests/data/test-read-write/test28-without-std-fns-ref.xml: Adju= st. >> * tests/data/test-read-write/test28-without-std-vars-ref.xml: Adj= ust. >>=20 >> Signed-off-by: Dodji Seketeli >> Applied to master. >> --- >> include/abg-fwd.h | 6 + >> include/abg-ir.h | 7 + >> src/abg-ir-priv.h | 11 +- >> src/abg-ir.cc | 302 +- >> src/abg-reader.cc | 3 +- >> tests/data/test-annotate/libtest23.so.abi | 748 +- >> .../test-annotate/libtest24-drop-fns-2.so.abi | 794 +- >> .../test-annotate/libtest24-drop-fns.so.abi | 794 +- >> tests/data/test-annotate/test0.abi | 48 +- >> .../data/test-annotate/test14-pr18893.so.abi | 2472 +- >> .../data/test-annotate/test15-pr18892.so.abi | 12330 +++-- >> .../data/test-annotate/test17-pr19027.so.abi | 2142 +- >> ...st18-pr19037-libvtkRenderingLIC-6.1.so.abi | 11742 +++-- >> ...19-pr19023-libtcmalloc_and_profiler.so.abi | 16174 +++--- >> ...st20-pr19025-libvtkParallelCore-6.1.so.abi | 16864 +++--- >> .../data/test-annotate/test21-pr19092.so.abi | 680 +- >> .../PR25058-liblttng-ctl-report-1.txt | 4 +- >> .../test-PR26739-2-report-0.txt | 10 +- >> .../PR22015-libboost_iostreams.so.abi | 3520 +- >> .../test-read-dwarf/PR22122-libftdc.so.abi | 3929 +- >> .../data/test-read-dwarf/PR25007-sdhci.ko.abi | 9147 ++-- >> .../PR25042-libgdbm-clang-dwarf5.so.6.0.0.abi | 169 +- >> tests/data/test-read-dwarf/libtest23.so.abi | 708 +- >> .../libtest24-drop-fns-2.so.abi | 760 +- >> .../test-read-dwarf/libtest24-drop-fns.so.abi | 760 +- >> .../test-read-dwarf/test-libaaudio.so.abi | 348 +- >> .../test-read-dwarf/test-libandroid.so.abi | 1296 +- >> tests/data/test-read-dwarf/test0.abi | 47 +- >> tests/data/test-read-dwarf/test0.hash.abi | 13 +- >> tests/data/test-read-dwarf/test1.hash.abi | 4 +- >> .../test-read-dwarf/test10-pr18818-gcc.so.abi | 7328 ++- >> .../test-read-dwarf/test11-pr18828.so.abi | 14955 +++--- >> .../test-read-dwarf/test12-pr18844.so.abi | 25236 +++++---- >> .../test-read-dwarf/test14-pr18893.so.abi | 1580 +- >> .../test-read-dwarf/test15-pr18892.so.abi | 11647 +++-- >> .../test-read-dwarf/test16-pr18904.so.abi | 16732 +++--- >> .../test-read-dwarf/test17-pr19027.so.abi | 2056 +- >> ...st18-pr19037-libvtkRenderingLIC-6.1.so.abi | 11520 +++-- >> ...19-pr19023-libtcmalloc_and_profiler.so.abi | 15834 +++--- >> ...st20-pr19025-libvtkParallelCore-6.1.so.abi | 16406 +++--- >> .../test-read-dwarf/test21-pr19092.so.abi | 656 +- >> .../test22-pr19097-libstdc++.so.6.0.17.so.abi | 42542 ++++++++-------- >> .../test9-pr18818-clang.so.abi | 5412 +- >> tests/data/test-read-write/test22.xml | 7 +- >> tests/data/test-read-write/test23.xml | 7 +- >> .../test28-without-std-fns-ref.xml | 648 +- >> .../test28-without-std-vars-ref.xml | 590 +- >> 47 files changed, 129532 insertions(+), 129456 deletions(-) >>=20 >> The patch is too big for the list so I am attaching it gzipped. >>=20 >> Cheers, >>=20 >>=20 >>=20 >> -- >> Dodji >=20