RepoMirrors/libabigail
1
0
Fork 0
mirror of git://sourceware.org/git/libabigail.git synced 2024-12-17 07:24:34 +00:00
Code Issues Packages Projects Releases Wiki Activity
1cfbff1b30
libabigail/include
History
Dodji Seketeli 1cfbff1b30 rhbz1951526 - SELF CHECK FAILED for 'gimp-2.10' 
This is a fix for bug https://bugzilla.redhat.com/show_bug.cgi?id=1951526.

Although it's a patch for one bug, it addresses several different
issues that cause the observed self comparison failure.  As is often
the case on this kind of problems, the failure is difficult to
reproduce on a synthetic test case so I'll explain the root causes in
this commit log.

There are 4 different root causes to this problem.  As I couldn't come
up with a reduced test case for each one of them I am adding the fixes
for those 4 issues in this commit, along with a new regression test
extracted from the initial bugzilla problem report.

So, overall, the symptom we are seeing here is that when we build an
IR for the input binary gimp-2.0, save that IR into abixml, and read
back that abixml into another IR, comparing the two IR shows changes;
it should show no change whatsoever.  This is what we call in
libabigail jargon a self comparison (or self check) failure.

As alluded to in my introduction above, there appear to be 4 different
root causes for that self comparison failure.

1/ The first cause has to do with a situation about two anymous enums
that are (wrongly) considered different from an ABI point of view.
Using the debugging capabilities recently gained by libabigail, I
could notice that the two enums are:

    (gdb) p debug(&l)
    enum __anonymous_enum__ : unnamed-enum-underlying-type-32
    {
      // size in bits: 32
      // translation unit: /usr/src/debug/gimp-2.10.22-2.el9.1.aarch64/app/<artificial>-757de
      // @: 0x698fb68, @canonical: 0

      GIMP_INTERPOLATION_NONE = 0,
      GIMP_INTERPOLATION_LINEAR = 1,
      GIMP_INTERPOLATION_CUBIC = 2,
      GIMP_INTERPOLATION_NOHALO = 3,
      GIMP_INTERPOLATION_LOHALO = 4,
    };

    $1 = (abigail::ir::decl_base *) 0x698fba0
    (gdb) p debug(&r)
    enum __anonymous_enum__ : unnamed-enum-underlying-type-32
    {
      // size in bits: 32
      // translation unit: /usr/src/debug/gimp-2.10.22-2.el9.1.aarch64/app/app.c
      // @: 0xa6d83e8, @canonical: 0

      GIMP_INTERPOLATION_NONE = 0,
      GIMP_INTERPOLATION_LINEAR = 1,
      GIMP_INTERPOLATION_CUBIC = 2,
      GIMP_INTERPOLATION_NOHALO = 3,
      GIMP_INTERPOLATION_LOHALO = 4,
      GIMP_INTERPOLATION_LANCZOS = 3,
    };

    $2 = (abigail::ir::decl_base *) 0xa6d8420
    (gdb)

Note how the second enum has a new enumerator named
'GIMP_INTERPOLATION_LANCZOS', but its value is '3', which is the exact
same value of as the one of the existing enumerator
GIMP_INTERPOLATION_NOHALO.

During type canonicalization of the IR from the input binary,
libabigail (wrongly) considers these two enums as being different.
This leads to the type 'Gimp*' (or anything type indirectly using any
one of the anonymous enums above) coming from one translation unit
being considered different from a type 'Gimp*' coming from another
translation unit, just because their are not using either one version
of the anonymous enum above or the other.

This leads to a *LOT* of spurious type changes from the first IR, that
are saved into abixml.

To fix this first problem, this patch introduces "two modes" of
comparing enums. There is a binary-only mode which only looks
enumerator values, not enumerator names.  And then there is the
source-level mode which looks at both enumerator names and values when
comparing enums.  The former mode is used during type
canonicalization.  However, when a change is detected between two
enums, then the diff-IR built to describe the change is constructed
using the later mode.  Using the later mode allows to describe
precisely things like enumerator insertion/removal by referring to the
names of the added/removed enumerators.

2/ The second root cause is that a struct, say, 'struct _GimpImage'
from a translation unit is considered different from a 'struct
_GimpImage' because the DWARF reader wrongly assign them different sizes.

Here is what it looks like in the debugger:

    (gdb) p debug(&l)
    struct _GimpImage
    {  // size in bits: 384
       // definition point: ../../app/core/gimpimage.h:39:1
       // translation unit: /usr/src/debug/gimp-2.10.22-2.el9.1.aarch64/app/<artificial>-757de
       // @: 0x69b9d10, @canonical: 0

      GimpViewable parent_instance;'
      Gimp* gimp;'
      GimpImagePrivate* priv;'
    };

    $8 = (abigail::ir::type_base *) 0x69b9d10
    (gdb) p debug(&r)
    struct _GimpImage
    {  // size in bits: 0
       // definition point: :0:0
       // translation unit: /usr/src/debug/gimp-2.10.22-2.el9.1.aarch64/app/<artificial>-8813f
       // @: 0x6ac7a50, @canonical: 0

    };

Notice how the second 'struct _GimpImage' has a size of zero.

This is because when reading the DWARF, we first encounter the DIE for
the first' struct _GimpImage' and we properly build a type for it,
along with its declaration.  Then when we encounter another DIE
defining 'struct _GimpImage' again, from a different translation unit,
the DWARF reader recognizes that it's a DIE for a declaration of
'struct _GimpImage' and fails to re-use the previous definition for
'struct _GimpImage'.  So it wrongly builds declaration-only 'struct
_GimpImage' for it, hence the second struct _GimpImage with a zero
size.

Here again that creates spurious changes (after type canonicalization)
in types using struct _GimpImage.  And that is a lot of types,
including things like 'Gimp*' and the like.

The fix for this root cause issue is to change
add_or_update_class_type in the DWARF reader to recognize that we are
seeing a type declaration for which there was already a definition and
return that definition instead of creating a new declaration.

3/ The third root cause is better explained with a "screen shot".
Consider these two 'versions' of the same struct _GdkDevice from two
different translation units:

    struct _GdkDevice
    {  // size in bits: 576
       // definition point: /usr/include/gtk-2.0/gdk/gdkinput.h:98:1
       // translation unit: /usr/src/debug/gimp-2.10.22-2.el9.1.aarch64/app/<artificial>-2d0352
       // @: 0x8820530, @canonical: 0

      GObject parent_instance;'
      gchar* name; // uses canonical type '@0x6892980'
      GdkInputSource source;'
      GdkInputMode mode;'
      gboolean has_cursor; // uses canonical type '@0x688dd00'
      gint num_axes; // uses canonical type '@0x688dd00'
      GdkDeviceAxis* axes;'
      gint num_keys; // uses canonical type '@0x688dd00'
      GdkDeviceKey* keys;'
    };

    $9 = (abigail::ir::type_base *) 0x8820530
    (gdb) p debug(&r)
    struct _GdkDevice
    {  // size in bits: 576
       // definition point: /usr/include/gtk-2.0/gdk/gdkinput.h:98:1
       // translation unit: /usr/src/debug/gimp-2.10.22-2.el9.1.aarch64/app/<artificial>-1fdb18
       // @: 0x7cd71e0, @canonical: 0

      GObject parent_instance;'
      gchar* _g_sealed__name; // uses canonical type '@0x6892980'
      GdkInputSource _g_sealed__source;'
      GdkInputMode _g_sealed__mode;'
      gboolean _g_sealed__has_cursor; // uses canonical type '@0x688dd00'
      gint _g_sealed__num_axes; // uses canonical type '@0x688dd00'
      GdkDeviceAxis* _g_sealed__axes;'
      gint _g_sealed__num_keys; // uses canonical type '@0x688dd00'
      GdkDeviceKey* _g_sealed__keys;'
    };

    $10 = (abigail::ir::type_base *) 0x7cd71e0
    (gdb)

Notice how the name of the second data member 'name' was changed to
'_g_sealed_name'.  A similar scheme happens to several other data
member names.  The offsets and types of the struct _GdkDevice haven't
changed however.  So from an ABI standpoint, the two versions of that
struct are equal.  Libabigail consider them different however.
Because that type is used by tons of other types of the binary being
analyzed, this leads to lots of spurious canonical type difference
that shouldn't be there.

These three issues are magnified by the fact that the gimp binary is
compiled using "link time optimization".  That brings in a lot more
opportunities to see these underlying issues that have been there for
a long time.

4/ The fourth and last root cause issue.  When the abixml writer emits
a translation unit (TU), it keeps track of the 'non-emitted referred
to type' of the currently emitted translation unit and emits them at
the end of each TU.  For instance, if the type 'Gimp*' (pointer to
Gimp) was emitted, and yet the referred-to type 'Gimp' wasn't emitted,
the TU writer makes sure to emit the referred-to 'Gimp' type at the
end of the TU.  This has been going on for quite some time now.

The problem however is that although the non-emitted referred-to type
was referred to in this current TU, it might no have been *DEFINED* in
this TU.  In that case, it should not be emitted in this TU.

Otherwise, the TU where that type is defined in the abixml might
appear different from where it is defined in the initial binary,
leading to self comparison failures down the road.

This patch ensures that a non-emitted referred-to type is always
emitted in the TU it belongs to.

5/ After doing all this, it appears that we were forgetting to emit
some function types that were defined in TUs emitted earlier and yet
were being referred-to later.  Looking closer, I realized that we
should just emit function types seen in a given TU, regardless of the
referred-to relation.

The problem with that is that function types are special in libabigail
because there are two situation in which they are created.

Basically, a function type is created by the DWARF DIE
DW_TAG_subroutine_type.  This is for instance how pointer to functions
are represented in DWARF, namely, by a DW_TAG_pointer_type that points
to a DW_TAG_subroutine_type.  That is represented in the libabigail ir
by an instance of the abigail::ir::function_type type.  This is
represented in abixml as a 'function-type' XML element.

But then, libabigail considers that all decls have a type.  This
applies obviously for variables or data member.  Right.  But then,
libabigail considers that a function is also a decl, which has a type.
And the type of a function is a function type, represented by the same
abigail::ir::function_type.  A practical difference with the former
situation is that function decls are *NOT* represented in abixml using
a 'function-type' element.  Instead a 'function-decl' XML element uses
return type and parameter elements to represent the types involved
with a function decl.

Said otherwise, the former 'function type' concept used to represent
the type of functions in the libabigail IR is artificial.  This
artificial-ness was not explicitly expressed in libabigail.  This
patch now expresses that artificial-ness for function types.  So the
abixml writer now just decide to not emit artificial function types,
and instead, emit all the non-artificial function types instead.

This addresses this last issues by being able to emit all
non-artificial function types defined in a given TU, without having to
bother with the fact that they are referred-to or not.

Together, fixing these 5 problems fixes this reported problem.

The changes to the reference test outputs are adjustments needed
because of the abixml output indeed changes.

	* include/abg-ir.h
	(environment::use_enum_binary_only_equality): Declare
	accessors.  (type_or_decl_base::{s,g}et_is_artificial):
	Likewise.  (decl_base::{s,g}et_is_artificial): Remove
	accessors.  * src/abg-ir.cc
	(environment::priv::use_enum_binary_only_equality): Define new
	data member.
	(environment::priv::use_enum_binary_only_equality): Define
	accessors.  (type_or_decl_base::priv::is_artificial_): Define
	new data member.  It has actually moved here from
	decl_base::priv::is_artificial_.
	(type_or_decl_base::priv::priv): Initialize it.
	(type_or_decl_base::{g,s}et_is_artificial): Define accessors.
	(decl_base::is_artificial_): Move this to
	type_or_decl_base::is_artificial_.
	(maybe_adjust_canonical_type): In a given class of equivalence
	of function types, if there is one non-artificial function
	type, then the entire class of equivalence is considered
	non-artificial; so flag the canonical function type as being
	non-artificial.  (is_enumerator_present_in_enum): Define new
	static function.  (equals): Re-arrange the overload for enums
	so the order of the enumerators doesn't count in the
	comparison.  Also, two enums with different numbers of
	enumerators can still be equal, with the right redundancy.  In
	the overload for var_decl, avoid taking into account the names
	of data members in the comparison.
	(enum_type_decl::enumerator::operator==): In the binary-level
	comparison mode, only compare the value of enumerators, not
	their name.  * src/abg-comparison.cc (compute_diff): In the
	overload for enum_type_decl, if the enums compare different
	using binary-level comparison, then use source-level
	comparison to build the diff-IR.  * src/abg-dwarf-reader.cc
	(read_context::compare_before_canonicalisation): Compare enums
	using binary-level comparison.  (add_or_update_class_type): If
	we are looking at the definition of an existing declaration
	that has been already defined then use the previous
	definition, in case we are going to need to update the
	definition.  Also, update the size only if it's needed.
	(build_function_type): By default, consider the newly built
	function type as artificial.  (build_ir_node_from_die): When
	looking at a DW_TAG_subroutine_type DIE, consider the built
	function type as non-artificial.  * src/abg-reader.cc
	(read_context::maybe_check_abixml_canonical_type_stability):
	Don't consider declaration-only classes in an ODR context
	because they don't have canonical types.
	(build_function_decl): Flag the function type of the function
	as artificial.  (build_class_decl): Make sure to reuse class
	types that were already created.  * src/abg-writer.cc
	(write_translation_unit): Allow emitting empty classes.  Make
	sure referenced types are emitting in the translation unit
	where they belong.  Avoid emitting artificial function types.
	*
	tests/data/test-alt-dwarf-file/rhbz1951526/rhbz1951526-report-0.txt:
	New test reference output.  *
	tests/data/test-alt-dwarf-file/rhbz1951526/usr/bin/gimp-2.10:
	New reference test binary input.  *
	tests/data/test-alt-dwarf-file/rhbz1951526/usr/lib/debug/.dwz/gimp-2.10.22-2.el9.1.aarch64:
	Likewise.  *
	tests/data/test-alt-dwarf-file/rhbz1951526/usr/lib/debug/usr/bin/gimp-2.10-2.10.22-2.el9.1.aarch64.debug:
	Likewise.  * tests/data/Makefile.am: Add the new test files to
	source directory.  * tests/test-alt-dwarf-file.cc: Add the new
	test inputs to this test harness.  *
	tests/data/test-abidiff/test-PR18791-report0.txt: Adjust.  *
	tests/data/test-abidiff/test-enum0-report.txt: Likewise.  *
	tests/data/test-annotate/libtest23.so.abi: Likewise.  *
	tests/data/test-annotate/libtest24-drop-fns-2.so.abi:
	Likewise.  *
	tests/data/test-annotate/libtest24-drop-fns.so.abi: Likewise.
	* tests/data/test-annotate/test-anonymous-members-0.o.abi:
	Likewise.  * tests/data/test-annotate/test13-pr18894.so.abi:
	Likewise.  * tests/data/test-annotate/test14-pr18893.so.abi:
	Likewise.  * tests/data/test-annotate/test15-pr18892.so.abi:
	Likewise.  * tests/data/test-annotate/test17-pr19027.so.abi:
	Likewise.  *
	tests/data/test-annotate/test18-pr19037-libvtkRenderingLIC-6.1.so.abi:
	Likewise.  *
	tests/data/test-annotate/test19-pr19023-libtcmalloc_and_profiler.so.abi:
	Likewise.  *
	tests/data/test-annotate/test20-pr19025-libvtkParallelCore-6.1.so.abi:
	Likewise.  * tests/data/test-annotate/test21-pr19092.so.abi:
	Likewise.  *
	tests/data/test-diff-dwarf-abixml/test0-pr19026-libvtkIOSQL-6.1.so.1.abi:
	Likewise.  *
	tests/data/test-diff-dwarf/PR25058-liblttng-ctl-report-1.txt:
	Likewise.  * tests/data/test-diff-dwarf/test6-report.txt:
	Likewise.  *
	tests/data/test-diff-filter/test31-pr18535-libstdc++-report-0.txt:
	Likewise.  *
	tests/data/test-diff-filter/test31-pr18535-libstdc++-report-1.txt:
	Likewise.  * tests/data/test-diff-filter/test8-report.txt:
	Likewise.  *
	tests/data/test-diff-pkg/spice-server-0.12.4-19.el7.x86_64-0.12.8-1.el7.x86_64-report-2.txt:
	Likewise.  *
	tests/data/test-diff-pkg/spice-server-0.12.4-19.el7.x86_64-0.12.8-1.el7.x86_64-report-3.txt:
	Likewise.  *
	tests/data/test-diff-pkg/tbb-4.1-9.20130314.fc22.x86_64--tbb-4.3-3.20141204.fc23.x86_64-report-0.txt:
	Likewise.  *
	tests/data/test-diff-pkg/tbb-4.1-9.20130314.fc22.x86_64--tbb-4.3-3.20141204.fc23.x86_64-report-1.txt:
	Likewise.  *
	tests/data/test-read-dwarf/PR22015-libboost_iostreams.so.abi:
	Likewise.  *
	tests/data/test-read-dwarf/PR22122-libftdc.so.abi: Likewise.
	* tests/data/test-read-dwarf/PR25007-sdhci.ko.abi: Likewise.
	*
	tests/data/test-read-dwarf/PR25042-libgdbm-clang-dwarf5.so.6.0.0.abi:
	Likewise.  *
	tests/data/test-read-dwarf/PR26261/PR26261-exe.abi: Likewise.
	* tests/data/test-read-dwarf/libtest23.so.abi: Likewise.  *
	tests/data/test-read-dwarf/libtest24-drop-fns-2.so.abi:
	Likewise.  *
	tests/data/test-read-dwarf/libtest24-drop-fns.so.abi:
	Likewise.  *
	tests/data/test-read-dwarf/test-libandroid.so.abi: Likewise.
	* tests/data/test-read-dwarf/test10-pr18818-gcc.so.abi:
	Likewise.  * tests/data/test-read-dwarf/test12-pr18844.so.abi:
	Likewise.  * tests/data/test-read-dwarf/test13-pr18894.so.abi:
	Likewise.  * tests/data/test-read-dwarf/test14-pr18893.so.abi:
	Likewise.  * tests/data/test-read-dwarf/test15-pr18892.so.abi:
	Likewise.  * tests/data/test-read-dwarf/test16-pr18904.so.abi:
	Likewise.  * tests/data/test-read-dwarf/test17-pr19027.so.abi:
	Likewise.  *
	tests/data/test-read-dwarf/test18-pr19037-libvtkRenderingLIC-6.1.so.abi:
	Likewise.  *
	tests/data/test-read-dwarf/test19-pr19023-libtcmalloc_and_profiler.so.abi:
	Likewise.  *
	tests/data/test-read-dwarf/test20-pr19025-libvtkParallelCore-6.1.so.abi:
	Likewise.  * tests/data/test-read-dwarf/test21-pr19092.so.abi:
	Likewise.  *
	tests/data/test-read-dwarf/test22-pr19097-libstdc++.so.6.0.17.so.abi:
	Likewise.  *
	tests/data/test-read-dwarf/test9-pr18818-clang.so.abi:
	Likewise.  *
	tests/data/test-read-write/test28-without-std-fns-ref.xml:
	Likewise.  *
	tests/data/test-read-write/test28-without-std-vars-ref.xml:
	Likewise.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
2021-06-09 18:43:02 +02:00
..
abg-comp-filter.h Re-license the project to Apache v2 With LLVM Exception 2020-12-02 11:49:13 +01:00
abg-comparison.h Bug 27569 - abidiff misses a function parameter addition 2021-03-29 12:34:48 +02:00
abg-config.h Re-license the project to Apache v2 With LLVM Exception 2020-12-02 11:49:13 +01:00
abg-corpus.h abg-corpus: remove symbol maps and their setters 2021-04-02 15:48:01 +02:00
abg-cxx-compat.h abg-cxx-compat: add simplified version of std::optional 2021-03-09 10:41:10 +01:00
abg-diff-utils.h Drop unneccessary includes of abg-cxx-compat.h 2020-12-15 09:23:44 +01:00
abg-dwarf-reader.h dwarf reader: drop (now) unused code related to symbol table reading 2021-04-02 15:48:23 +02:00
abg-fwd.h Introduce artificial locations 2021-05-25 12:31:14 +02:00
abg-hash.h Re-license the project to Apache v2 With LLVM Exception 2020-12-02 11:49:13 +01:00
abg-ini.h Drop unneccessary includes of abg-cxx-compat.h 2020-12-15 09:23:44 +01:00
abg-interned-str.h Drop unneccessary includes of abg-cxx-compat.h 2020-12-15 09:23:44 +01:00
abg-ir.h rhbz1951526 - SELF CHECK FAILED for 'gimp-2.10' 2021-06-09 18:43:02 +02:00
abg-libxml-utils.h reader: Use xmlFirstElementChild and xmlNextElementSibling rather than xml::advance_to_next_sibling_element 2021-05-03 17:15:22 +02:00
abg-reader.h Detect abixml canonical type instability during abidw --debug-abidiff 2021-05-25 12:24:26 +02:00
abg-regex.h Drop unneccessary includes of abg-cxx-compat.h 2020-12-15 09:23:44 +01:00
abg-reporter.h Drop unneccessary includes of abg-cxx-compat.h 2020-12-15 09:23:44 +01:00
abg-sptr-utils.h Drop unneccessary includes of abg-cxx-compat.h 2020-12-15 09:23:44 +01:00
abg-suppression.h Drop unneccessary includes of abg-cxx-compat.h 2020-12-15 09:23:44 +01:00
abg-tools-utils.h Bug 27512 - Remove broken zip-archive support 2021-03-19 10:52:57 +01:00
abg-traverse.h Re-license the project to Apache v2 With LLVM Exception 2020-12-02 11:49:13 +01:00
abg-version.h.in Re-license the project to Apache v2 With LLVM Exception 2020-12-02 11:49:13 +01:00
abg-viz-common.h Re-license the project to Apache v2 With LLVM Exception 2020-12-02 11:49:13 +01:00
abg-viz-dot.h Re-license the project to Apache v2 With LLVM Exception 2020-12-02 11:49:13 +01:00
abg-viz-svg.h Re-license the project to Apache v2 With LLVM Exception 2020-12-02 11:49:13 +01:00
abg-workers.h Drop unneccessary includes of abg-cxx-compat.h 2020-12-15 09:23:44 +01:00
abg-writer.h Detect abixml canonical type instability during abidw --debug-abidiff 2021-05-25 12:24:26 +02:00
Makefile.am Bug 27512 - Remove broken zip-archive support 2021-03-19 10:52:57 +01:00