Re: [PATCH 3/3] ir: Consider integral types of same kind and size as equivalent

[Giuliano Procida <gprocida@google.com>]

Hi Dodji.

On Sat, 23 Jul 2022 at 00:33, Dodji Seketeli <dodji@seketeli.org> 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 <dodji@redhat.com>
> 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