From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [216.205.24.124]) by sourceware.org (Postfix) with ESMTP id DF62638618D7 for ; Thu, 11 Feb 2021 16:43:54 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org DF62638618D7 Received: from mail-wm1-f72.google.com (mail-wm1-f72.google.com [209.85.128.72]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-357-BZeE7_C6N8OKBLjvGRuTGQ-1; Thu, 11 Feb 2021 11:43:52 -0500 X-MC-Unique: BZeE7_C6N8OKBLjvGRuTGQ-1 Received: by mail-wm1-f72.google.com with SMTP id p8so3461611wmq.7 for ; Thu, 11 Feb 2021 08:43:52 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:subject:organization:date:message-id :user-agent:mime-version; bh=jcmq5TN5T8yceMIW0Snmy7j8ZLQGnxovxySr56MvNu4=; b=mHu0QkHoS/Tc0fYecQFtX5hxp31wIrQtxX86BujoQAYskT83Y7mKFOClzqMQavMQzc 3bc28JDSPa58nEpLme72ZuVdtwxs7VhlgM0tVf4g8KpALH2Ss3bGUFlWZ25y+5qVRyqS yCIhV9EtrKSdb9G/4tGrWtw0wZpCg2LWqrCsvI4WWD/T5CihnplJ4zBfHwXM8w0UpYs5 YYWTpDLA+zTMClDPTWSJoLqrrYSRtR2qlzpYM93pyl9uKrJmvQYugDRozR2RM5x+vrDL tpTGocTxW/KoCqO1a4vqX1OnsPyNFLTeNFZESqPwpAS8kM1XSqi9OrHppGelO0GA4bT7 ahYQ== X-Gm-Message-State: AOAM531tnsMh7T+bRbPFMQuC6YpU5BlwmH220zadcabgYheudse/mZ4H KlMG9MvZuN+9fibcm1luzVVWtSThpmXjs29f7LnhKxwEIayyzEmCrr0w1wY/Ne26jC5NLcukb38 amnU/xR1RSKgVh9Ju2SGgKoBONWDv3dg4p4lMPpUoLOKyTb3HwjEzXpQup8hRV4mjgA0Y X-Received: by 2002:a7b:c397:: with SMTP id s23mr5886001wmj.123.1613061831140; Thu, 11 Feb 2021 08:43:51 -0800 (PST) X-Google-Smtp-Source: ABdhPJx/6f4yAuaek//+ZrftknhVqRuocDWZiB0dSCNYyZn8mKilM2SHt0EPPy7zGYCFuyMkc5v4wA== X-Received: by 2002:a7b:c397:: with SMTP id s23mr5885982wmj.123.1613061830815; Thu, 11 Feb 2021 08:43:50 -0800 (PST) Received: from localhost ([88.120.130.27]) by smtp.gmail.com with ESMTPSA id a132sm11508027wmf.42.2021.02.11.08.43.49 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 11 Feb 2021 08:43:50 -0800 (PST) Received: by localhost (Postfix, from userid 1000) id 1309D581C96; Thu, 11 Feb 2021 17:43:49 +0100 (CET) From: Dodji Seketeli To: libabigail@sourceware.org Subject: [PATCH] Don't consider type name when comparing typedefs Organization: Red Hat / France X-Operating-System: Fedora 34 X-URL: http://www.redhat.com Date: Thu, 11 Feb 2021 17:43:48 +0100 Message-ID: <878s7uftbv.fsf@redhat.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.1 (gnu/linux) MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain X-Spam-Status: No, score=-6.1 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) 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: Thu, 11 Feb 2021 16:43:57 -0000 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 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