public inbox for libabigail@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] Don't consider type name when comparing typedefs
@ 2021-02-11 16:43 Dodji Seketeli
  0 siblings, 0 replies; only message in thread
From: Dodji Seketeli @ 2021-02-11 16:43 UTC (permalink / raw)
  To: libabigail

Hello,

This is from a problem report originating from Red Hat bugzilla at
https://bugzilla.redhat.com/show_bug.cgi?id=1925876

I some C programs, the same type can be defined more than once in a
binary, as there is no "One Definition Rule[1]" in C.

    [1]: https://en.wikipedia.org/wiki/One_Definition_Rule

The definition of those two types can be slightly different and yet be
equivalent.

For instance, the data member of a struct S might be defined once as
having a type which is a typedef Foo of, say, "long int" and that
struct S might be defined again elsewhere with a data member of type
typedef Bar of "long int" as well.

With the current code, because Foo and Bar have different names, they
are are going to compare different; so the two struct S are doing to
compare different even though they are equivalent.

Down the road, this is likely going to imply that the several 'struct
S' that are declaration-only will not be resolved as being
declarations of the definition of "struct S", because they is more
than one such definition, and those definitions are different.

This is going to lead to spurious (and hard to debug) type differences
that are going to be detected and reported by libabigail later down
the road.

This patch addresses the problem by not taking typedef names into
account when comparing typedefs before canonicalization.  That allows
the comparison of classes and structs that happens during the
resolution of declaration-only classes to correctly deduce their
equivalence even in cases like the one exposed above.  It thus reduces
the amount of declaration-only classes that are unnecessarily
considered to be different from the definition they ought to equal.

	* include/abg-ir.h (maybe_compare_as_member_decls): Declare new
	function.  Make it a friend of class decl_base.
	* src/abg-dwarf-reader.cc (maybe_canonicalize_type): Don't early
	canonicalize typedefs because they can be "part" of a type that is
	not yet completed, especially considering that class declaration
	resolution is part of type building, stricto sensu.
	* src/abg-ir.cc (maybe_compare_as_member_decls): Factorize this
	out of ...
	(equals): ... the overload for decl_base.  Use it in the overload
	for typedef_decl.
	* tests/data/test-diff-pkg/nmap-7.70-5.el8_testjcc.x86_64-self-check-report-0.txt:
	New test reference output.
	* tests/data/test-diff-pkg/nmap-7.70-5.el8_testjcc.x86_64.rpm: New
	binary input.
	* tests/data/test-diff-pkg/nmap-debuginfo-7.70-5.el8_testjcc.x86_64.rpm: Likewise.
	* tests/data/Makefile.am: Add these new testing material to source
	distribution.
 	* tests/test-diff-pkg.cc (in_out_specs): Add the new test input to
	the harness.
	* tests/data/test-diff-pkg/spice-server-0.12.4-19.el7.x86_64-0.12.8-1.el7.x86_64-report-0.txt:
	Adjust.
	* tests/data/test-diff-suppr/test39-opaque-type-report-0.txt:
	Adjust.

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

Applied to the mainline branch.

---
 include/abg-ir.h                              |  10 ++
 src/abg-dwarf-reader.cc                       |   3 +-
 src/abg-ir.cc                                 | 121 +++++++++++-------
 tests/data/Makefile.am                        |   3 +
 ...el8_testjcc.x86_64-self-check-report-0.txt |   2 +
 .../nmap-7.70-5.el8_testjcc.x86_64.rpm        | Bin 0 -> 6110324 bytes
 ...ap-debuginfo-7.70-5.el8_testjcc.x86_64.rpm | Bin 0 -> 3874760 bytes
 ...l7.x86_64-0.12.8-1.el7.x86_64-report-0.txt |  10 +-
 .../test39-opaque-type-report-0.txt           |   9 +-
 tests/test-diff-pkg.cc                        |  12 ++
 10 files changed, 109 insertions(+), 61 deletions(-)
 create mode 100644 tests/data/test-diff-pkg/nmap-7.70-5.el8_testjcc.x86_64-self-check-report-0.txt
 create mode 100644 tests/data/test-diff-pkg/nmap-7.70-5.el8_testjcc.x86_64.rpm
 create mode 100644 tests/data/test-diff-pkg/nmap-debuginfo-7.70-5.el8_testjcc.x86_64.rpm

The patch is too big to be attached here.  It can be read online at
https://sourceware.org/git/?p=libabigail.git;a=commitdiff;h=77bc4b7b1502b2049b2b754069b5187c768f72c8

Cheers,

-- 
		Dodji


^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2021-02-11 16:43 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-11 16:43 [PATCH] Don't consider type name when comparing typedefs Dodji Seketeli

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).