[PATCH, applied] abixml reader: Fix recursive type definition handling

[PATCH, applied] abixml reader: Fix recursive type definition handling

[Dodji Seketeli]

[-- Attachment #1: Type: text/plain, Size: 6281 bytes --]

Hello,

This should fix self comparison bug https://bugzilla.redhat.com/show_bug.cgi?id=1944088

This arose from a self comparison check failing on the library
libgvpr.so.2 from the graphviz-2.44.0-17.el9.aarch64.rpm package.

Now that we have facilities to see what type (instantiated from the
abixml representation of the libgvpr.so library) exactly the
canonicalization process is failing for, I decided to use it ;-)

I extracted the package and its associating debug info into a
directory named 'extract' and ran abidw --debug-abidiff on it:

    $ build/tool/abidw  --debug-abidiff -d extract/usr/lib/debug extract/usr/lib64/libgvpr.so.2

That yielded the output below:

    error: problem detected with type 'typedef Vmalloc_t' from second corpus
    error: canonical type for type 'typedef Vmalloc_t' of type-id 'type-id-170' changed from 'd72618' to '14a7448'
    error: problem detected with type 'Vmalloc_t*' from second corpus
    error: canonical type for type 'Vmalloc_t*' of type-id 'type-id-188' changed from 'd72ba8' to '14a7968'
    [...]

This tells me that "typedef Vmalloc_t", created from the abixml
compares different from its originating peer that was created from the
binary directly.  The same goes for the pointer type "Vmalloc_t*", etc.

Using the new debugging/logging functionalities from the command line
of the debugger, I could see that in the abixml reader,
build_typedef_decl can fail subtly when the underlying type of the
typedef refers to the typedef itself.  In that case, we need to ensure
that the typedef created by build_typedef_decl is the same one that is
used by the underlying type.  which is not the case at the moment.  At
the moment, the underlying type would create a new typedef beside the
one currently being created by build_typedef_decl.  That leads to more
than one typedef in the system to designate "typedef Vmalloc_t".  And
that wreaks havoc later down the road.

This patch arranges so that build_typedef_decl creates the typedef
"early" before the underlying type is created.  That typedef
temporarily has no underlying type.  It's registered as being the
typedef for the type-id string that identifies it in the abixml.  And
then the function goes to create the underlying type.  This
arrangement ensures that if the underlying type refers to the typedef
being created (via its type-id string), then the typedef that was
created early is effectively re-used.  This ensures that a typedef
which recursively refer to itself is properly represented.  It's only
when the underlying type is fully created that it's added to the
typedef.

Something similar is done for pointer types, in
build_pointer_type_def.

Note that to do this, the patch adjusts the typedef_decl and
pointer_type_def classes so that they can be created with no
underlying/pointed-to types.  The underlying/pointed-to type can thus
be added later.

I believe this patch is the minimal patch necessary to fix this issue.
The graphviz RPM is added to the regression test suite for good
measure.

After visual inspection, I realized that there are other types besides
typedef and pointer types that exhibit the same class of problem even
if they are not involved in this issue on this particular binary.  A
subsequent patch is going to address the problem for those types,
namely, qualified and reference types.

	* include/abg-ir.h (pointer_type_def::pointer_type_def): Declare a
	constructor with no pointed-to type.
	(pointer_type_def::set_pointed_to_type): Declare new method.
	(typedef_decl::typedef_decl): Declare a constructor with no
	underlying type.
	* src/abg-ir.cc (pointer_type_def::pointer_type_def): Define a
	constructor with no pointed-to type.  The pointed-to type can thus
	later be set when it becomes available.
	(pointer_type_def::set_pointed_to_type): Define new method.
	(pointer_type_def::get_qualified_name): Make this work on a
	pointer type that (momentarily) has no pointed-to type.
	(typedef_decl::typedef_decl): Define a constructor with no
	underlying type.
	(typedef_decl::get_size_in_bits): Make this work on a typedef that
	has (momentarily) no underlying type.
	(typedef_decl::set_underlying_type): Update the size and alignment
	of the typedef from its new underlying type.
	* src/abg-reader.cc (build_pointer_type_def): Construct the
	pointer type early /BEFORE/ we even try to construct its
	pointed-to type.  Associate this incomplete type with the type-id.
	Then try to construct the pointed-to type.  During the
	construction of the pointed-to type, if this pointer is needed
	(due to recursion) then the incomplete pointer type can be used,
	leading to just one pointer type used (recursively) as it should
	be.
	(build_typedef_decl): Likewise for building typedef type early
	without its underlying type so that it can used by the underlying
	type if needed.
	* tests/data/test-diff-pkg/graphviz-2.44.0-18.el9.aarch64-self-check-report-0.txt:
	New test reference output.
	* tests/data/test-diff-pkg/graphviz-2.44.0-18.el9.aarch64.rpm: New
	binary test input.
	* tests/data/test-diff-pkg/graphviz-debuginfo-2.44.0-18.el9.aarch64.rpm: Likewise.
	* tests/data/Makefile.am: Add the new test material above to
	source distribution.
	* tests/test-diff-pkg.cc (in_out_specs): Add the test inputs above
	to this test harness.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>

Applied to master.

---
 include/abg-ir.h                              |  13 ++
 src/abg-ir.cc                                 | 129 ++++++++++++++++--
 src/abg-reader.cc                             | 119 +++++++---------
 tests/data/Makefile.am                        |   3 +
 ...4.0-18.el9.aarch64-self-check-report-0.txt |  43 ++++++
 .../graphviz-2.44.0-18.el9.aarch64.rpm        | Bin 0 -> 3443556 bytes
 ...aphviz-debuginfo-2.44.0-18.el9.aarch64.rpm | Bin 0 -> 3541395 bytes
 tests/test-diff-pkg.cc                        |  12 ++
 8 files changed, 238 insertions(+), 81 deletions(-)
 create mode 100644 tests/data/test-diff-pkg/graphviz-2.44.0-18.el9.aarch64-self-check-report-0.txt
 create mode 100644 tests/data/test-diff-pkg/graphviz-2.44.0-18.el9.aarch64.rpm
 create mode 100644 tests/data/test-diff-pkg/graphviz-debuginfo-2.44.0-18.el9.aarch64.rpm


The patch being quite big, I am attaching a gzipped copy of it to this
message.


[-- Attachment #2: 0001-abixml-reader-Fix-recursive-type-definition-handling.patch.gz --]
[-- Type: application/gzip, Size: 7185004 bytes --]

[-- Attachment #3: Type: text/plain, Size: 22 bytes --]


Cheers,

-- 
		Dodji