From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-io1-xd30.google.com (mail-io1-xd30.google.com [IPv6:2607:f8b0:4864:20::d30]) by sourceware.org (Postfix) with ESMTPS id BF28E3858D1E for ; Wed, 10 Aug 2022 15:24:36 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org BF28E3858D1E Received: by mail-io1-xd30.google.com with SMTP id x64so12433889iof.1 for ; Wed, 10 Aug 2022 08:24:36 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc; bh=98f/nPm2F7m9n7vXmEwD3J2c+xhrHZ2TfddhhouDJT0=; b=vvoADKPEVT4wkLk7GUlusx3ArxYQAtUC+qhSn2PNw9c+M9jmUlmQwxTuWhxHlh2CyV n4921o4nCR7JIWNY0uIkBk85MLFeu0orYD7ktWdVCFUsZNsbD6lZdOyH1YehncYbmCL3 lT+RM6586EltKHtK7RVBhLtrTy6zQadGD7taESJzN9uOw5d8yw5HkRFRFKQWG/1nZDII ocI+uT8ZLzxneP3wkU/BNAY3ICezRuP66gA2XWkQZpRuKA69PKKXnEWoTIjrQPrA3CCL jRK9/RQ7/ckfla7yfrsrfKV4DrABeOeuNhUt46Wo4LBkBLPhceaAc6FOpfc8FiWxiT9v B6Xw== X-Gm-Message-State: ACgBeo2H9/GA79topnenH7Zy9RVTYEc0cZDQ/BHf5MxX8KuXuNwbMGmh yYLKRBkqlENiXeKn3RLlZeswCe+5UoX1sVcJWr4jxg== X-Google-Smtp-Source: AA6agR7lq0tUGTNtlGk4Tll6chAajRUZNbU8hXhGXMXTus2KqUrYO8u6e+rF8zwD4va7Z4q2HEe8goNDO5EWdszQxKc= X-Received: by 2002:a05:6602:2c44:b0:67d:588:f282 with SMTP id x4-20020a0566022c4400b0067d0588f282mr11578397iov.139.1660145075813; Wed, 10 Aug 2022 08:24:35 -0700 (PDT) MIME-Version: 1.0 References: <87sfms91n0.fsf@redhat.com> <87czdw910h.fsf@seketeli.org> In-Reply-To: <87czdw910h.fsf@seketeli.org> From: Giuliano Procida Date: Wed, 10 Aug 2022 16:23:59 +0100 Message-ID: Subject: Re: [PATCH 3/3] ir: Consider integral types of same kind and size as equivalent To: Dodji Seketeli Cc: Dodji Seketeli via Libabigail , Dodji Seketeli , =?UTF-8?Q?Matthias_M=C3=A4nnich?= Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-21.0 required=5.0 tests=BAYES_00, DKIMWL_WL_MED, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, ENV_AND_HDR_SPF_MATCH, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE, USER_IN_DEF_DKIM_WL, USER_IN_DEF_SPF_WL 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: Wed, 10 Aug 2022 15:24:39 -0000 Hi Dodji. On Sat, 23 Jul 2022 at 00:33, Dodji Seketeli wrote: > > Hello, > > 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. While such types may be ABI compatible, they are not API compatible as they impact (at least) C++ overload resolution. 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. > Otherwise, that causes spurious type changes that lead to self > comparison change down the road. For instance, the following command > fails: > > $ tools/fedabipkgdiff --debug --self-compare -a --from fc36 btrfs-progs Shouldn't any tweaking of behaviour happen with abidiff rather than abidw? We might not end up with stable XML but the finger of blame should be pointed at the btrfs-progs in any case. > 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. 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. Is there an easy way of putting this under flag control? 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. Regards, Giuliano. > * 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&=): 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&=): Define new operators. > (parse_integral_type): Fix the logic of this function. Namely, it > 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_profiler.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_profiler.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.so.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: Adjust. > * tests/data/test-read-write/test28-without-std-vars-ref.xml: Adjust. > > 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(-) > > The patch is too big for the list so I am attaching it gzipped. > > Cheers, > > > > -- > Dodji