From patchwork Wed Aug 11 16:02:27 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dodji Seketeli X-Patchwork-Id: 44636 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 649633985452 for ; Wed, 11 Aug 2021 16:04:06 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 649633985452 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1628697846; bh=6RuKq++tPZDqj65tF+xsmhOEfG+nZ9ZPao0OenyyLNc=; h=To:Subject:Date:List-Id:List-Unsubscribe:List-Archive:List-Help: List-Subscribe:From:Reply-To:From; b=I1H1A1WGNau7akNzU0tX8FV1kZ/+kpVkkr43FeRKl/R4ODDcVOCPbMG+ZI7Ztw9sJ A0UxCwlqKoSYBe2vlHYe69dobudG1rwcG633srJbnEqj5kScQM5shPaFDQ50bnGgmk g7jNsHyP5jC8K1Ut9dl4HgvJqlUdbYOVn5LVjhX4= X-Original-To: libabigail@sourceware.org Delivered-To: libabigail@sourceware.org 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 18C12398243C for ; Wed, 11 Aug 2021 16:02:37 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 18C12398243C Received: from mail-wr1-f70.google.com (mail-wr1-f70.google.com [209.85.221.70]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-599-qGprvNA1M0ST2pkUXj8R3w-1; Wed, 11 Aug 2021 12:02:33 -0400 X-MC-Unique: qGprvNA1M0ST2pkUXj8R3w-1 Received: by mail-wr1-f70.google.com with SMTP id l12-20020a5d6d8c0000b029015488313d96so896508wrs.15 for ; Wed, 11 Aug 2021 09:02:33 -0700 (PDT) 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=6RuKq++tPZDqj65tF+xsmhOEfG+nZ9ZPao0OenyyLNc=; b=l+On6aOGlsVI7nK3w0+3R0WeUqqJrP2LCnqXx87SXCIdCCHn7nMTZkB6tKIfpXVZI/ ScnzfP/K7e6j9LNZENFxi1LKRdTlwpH44SXj05Wu5FE9DIs6PdaUY4T7wAbsNj9CNJwX JCx91/EPySiJ2Rlqy8Lmg33htGt6grnpGQ+S3T0zkCIDfNXjKJk4F+I5e1AaASr32djQ izp7LmAUnDaRkxvXzWb+qwNadj111LzanfUhWLrFTZuFrKIl+bxqulw39W/AXXCXQVcX o+5G3W/fEH5WvIIqdtkGS067dKaMkGCJuX9mREqMWQLKeoXKZvwycUyBRg0RzTh7QSJ9 uB6w== X-Gm-Message-State: AOAM533Omhx+O0dKFmONzYHkgPLNyomwm/TtUmRSKzPZvww/IVHxAz+v o+S+PCHTp4LJcOkB37lLi8Ysp72u8M/J04AYefBRgIhRCkwyJOtmFoeDLep3VnypB/jM797oRMy tCm1x+WYlgDuPMuMAEYPupumemv16RhXyVoCw9b4r13WQlJiYejVIWBEZ9N62QfRyyZCl X-Received: by 2002:a05:600c:19c6:: with SMTP id u6mr29003612wmq.154.1628697752130; Wed, 11 Aug 2021 09:02:32 -0700 (PDT) X-Google-Smtp-Source: ABdhPJxSV2n7Hv3KRkMy0RwZBbUO8SAEQY+sdoY+pucLqWhZ4D5u/VZRaN28YAtuG42JZkOBPLM++g== X-Received: by 2002:a05:600c:19c6:: with SMTP id u6mr29003590wmq.154.1628697751908; Wed, 11 Aug 2021 09:02:31 -0700 (PDT) Received: from localhost ([88.120.130.27]) by smtp.gmail.com with ESMTPSA id l7sm24927694wmj.9.2021.08.11.09.02.31 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 11 Aug 2021 09:02:31 -0700 (PDT) Received: by localhost (Postfix, from userid 1000) id 2FE145800FB; Wed, 11 Aug 2021 18:02:28 +0200 (CEST) To: libabigail@sourceware.org Subject: [PATCH 1/8, applied] ir: Improve the debugging facilities Organization: Red Hat / France X-Operating-System: Fedora 35 X-URL: http://www.redhat.com Date: Wed, 11 Aug 2021 18:02:27 +0200 Message-ID: <877dgsdl64.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 X-Spam-Status: No, score=-12.2 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_NONE, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) 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-Patchwork-Original-From: Dodji Seketeli via Libabigail From: Dodji Seketeli Reply-To: Dodji Seketeli Errors-To: libabigail-bounces+patchwork=sourceware.org@sourceware.org Sender: "Libabigail" Hello, While looking at something else, I stumbled across some minor issues in the debugging facilities I use to track self comparison problems. I added a missing ABG_RETURN macro in the stack of equals() function to better detect when there is a change, under the debugger. I also fixed get_debug_representation() to properly display the class/enum name (as expected) rather their pretty representation. * src/abg-ir.cc (maybe_compare_as_member_decls): Add a missing ABG_RETURN (get_debug_representation): Display the name of class and enums, not their pretty representation. Signed-off-by: Dodji Seketeli Applied to master. --- src/abg-ir.cc | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/abg-ir.cc b/src/abg-ir.cc index 87b3b182..39540582 100644 --- a/src/abg-ir.cc +++ b/src/abg-ir.cc @@ -4706,7 +4706,7 @@ maybe_compare_as_member_decls(const decl_base& l, *k |= LOCAL_NON_TYPE_CHANGE_KIND; } } - return result; + ABG_RETURN(result); } /// Compares two instances of @ref decl_base. @@ -8522,7 +8522,7 @@ get_debug_representation(const type_or_decl_base* artifact) if (c) { class_decl *clazz = is_class_type(c); - string name = c->get_pretty_representation(/*internal=*/false, true); + string name = c->get_qualified_name(); std::ostringstream o; o << name; @@ -8535,8 +8535,7 @@ get_debug_representation(const type_or_decl_base* artifact) o << " "; if (b->get_is_virtual()) o << "virtual "; - o << b->get_base_class()->get_pretty_representation(/*internal=*/false, - /*qualified=*/true) + o << b->get_base_class()->get_qualified_name() << std::endl; } } @@ -8582,7 +8581,7 @@ get_debug_representation(const type_or_decl_base* artifact) } else if (const enum_type_decl* e = is_enum_type(artifact)) { - string name = e->get_pretty_representation(/*internal=*/true, true); + string name = e->get_qualified_name(); std::ostringstream o; o << name << " : " From patchwork Wed Aug 11 16:03:33 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dodji Seketeli X-Patchwork-Id: 44637 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id AC2EF3985448 for ; Wed, 11 Aug 2021 16:04:09 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org AC2EF3985448 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1628697849; bh=7y+z+vbm6eQlCrGbteu1B1ZAZBiBUjNlMF78Ft0nyY8=; h=To:Subject:Date:List-Id:List-Unsubscribe:List-Archive:List-Help: List-Subscribe:From:Reply-To:From; b=wNC570+VSiXJnrWwGbBIR+ntOzvTofuKi2Eg3on2cjqMoxprRx/UGW08o/507E+gi kJUiqQ2aU3c5qesVqtXxM7DDxWM7fx0c1DRelJu3uXNvaAAUM9Fbg1abxfAswYXtxY s+J1DViiUdiNWYC53+A+VN4wfMw4k1PvhqJ/UCzg= X-Original-To: libabigail@sourceware.org Delivered-To: libabigail@sourceware.org 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 E8542383A832 for ; Wed, 11 Aug 2021 16:03:40 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org E8542383A832 Received: from mail-wm1-f69.google.com (mail-wm1-f69.google.com [209.85.128.69]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-211-ExfOt0iSMXGYuisb9A7MMw-1; Wed, 11 Aug 2021 12:03:39 -0400 X-MC-Unique: ExfOt0iSMXGYuisb9A7MMw-1 Received: by mail-wm1-f69.google.com with SMTP id b3-20020a1c80030000b02902e6a7296cb3so1046783wmd.5 for ; Wed, 11 Aug 2021 09:03:39 -0700 (PDT) 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=7y+z+vbm6eQlCrGbteu1B1ZAZBiBUjNlMF78Ft0nyY8=; b=IFojhYeKspQlOM0F+aPRGzM5Ahth+TOBR1nr4eLhTlcl5Fg5d3tpGl9rIarvLROYxW fJmaY1qCsvPEABsowWVeAWgM/j8EGH88pjLkJ1fvtHisWTaobpTJR2u55NXx3vhWK5vF sa8IpK9I0PlhqhPX3DYYrsudQjRYMwpVcqrR0uJO7SJ6LM9LkNTJccJshDvkwz09Cdp/ NihkbnIp3KWaRrUq7+vyi/D2yBhkJ1ZGNREUzgfv/LqCtfX3wWzcwSLobOzAWyRzveWB 7y/4uABYbiye+xfNJ1S+DD44IdxbZwb13wj4dQ8tKd7fcPttaFyzxVvPF+ze2HRxgeJH bIRg== X-Gm-Message-State: AOAM530dXNoOGdqbzVfw3316awFwR2k2fbro4/zlFeDPAkj9oQPoxAkx Hhl6DzffnLXRgM4wY3OHTrB8nL5rHvPCZN8jfS1YIXb+J4mdiwVNkZQcuenLND03jenwQJyzkeZ /xCDPAbszhBRIN7GCZGiQsGsURZl4Fd6/DKZk2zplZ6zKQqBZg3r8ZktJ1bu9L4KbVEgM X-Received: by 2002:a1c:4c02:: with SMTP id z2mr17096024wmf.62.1628697817997; Wed, 11 Aug 2021 09:03:37 -0700 (PDT) X-Google-Smtp-Source: ABdhPJwndJjgIgagTUUS3ASJE+S5QVxULA3H4cJJkEBc1qj+50bebJUDHkvo8BtWfdkUTtMN3sOhvQ== X-Received: by 2002:a1c:4c02:: with SMTP id z2mr17096006wmf.62.1628697817809; Wed, 11 Aug 2021 09:03:37 -0700 (PDT) Received: from localhost ([88.120.130.27]) by smtp.gmail.com with ESMTPSA id h11sm19940677wrq.64.2021.08.11.09.03.37 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 11 Aug 2021 09:03:37 -0700 (PDT) Received: by localhost (Postfix, from userid 1000) id 0FACD5800FB; Wed, 11 Aug 2021 18:03:33 +0200 (CEST) To: libabigail@sourceware.org Subject: [PATCH 2/8, applied] ir: Tighten the test for anonymous data member Organization: Red Hat / France X-Operating-System: Fedora 35 X-URL: http://www.redhat.com Date: Wed, 11 Aug 2021 18:03:33 +0200 Message-ID: <8735rgdl4a.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 X-Spam-Status: No, score=-12.2 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_NONE, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) 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-Patchwork-Original-From: Dodji Seketeli via Libabigail From: Dodji Seketeli Reply-To: Dodji Seketeli Errors-To: libabigail-bounces+patchwork=sourceware.org@sourceware.org Sender: "Libabigail" Hello, In is_anonymous_data_member(), we only test that the name of the data member is empty; we forget to test that decl_base::get_is_anonymous() is true. This might make us wrongly think that a data member is anonymous in cases like in the equals() function for var_decl, where we temporarily set the name of the compared var_decl to "" before invoking the decl_base::operator==. We do this to perform the comparison by not taking into account the name of the variable. This hasn't yet happened on the binaries of the regression test suite, but it's definitely wrong so I am fixing it here. * src/abg-ir.cc: (is_anonymous_data_member): Consider decl_base::get_is_anonymous as well. Signed-off-by: Dodji Seketeli Applied to master. --- src/abg-ir.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/src/abg-ir.cc b/src/abg-ir.cc index 39540582..8e00eabf 100644 --- a/src/abg-ir.cc +++ b/src/abg-ir.cc @@ -5474,6 +5474,7 @@ bool is_anonymous_data_member(const var_decl& d) { return (is_data_member(d) + && d.get_is_anonymous() && d.get_name().empty() && is_class_or_union_type(d.get_type())); } From patchwork Wed Aug 11 16:04:23 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dodji Seketeli X-Patchwork-Id: 44638 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 92FE1383A832 for ; Wed, 11 Aug 2021 16:04:44 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 92FE1383A832 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1628697884; bh=OJQl/dN7dIi69FoFlcppyBLIX/48hWJLFro+ZGblHrg=; h=To:Subject:Date:List-Id:List-Unsubscribe:List-Archive:List-Help: List-Subscribe:From:Reply-To:From; b=MRb7lpgwhkh2yxqjY5Llo7aSuJ+lNucKs/CYVXSA6V+C48NGS8Y1+OWiguhjgpGmr qHtvAEdPFGUITvbTpYVpbn//P3uEROCNOJAMSX37cMdFBLcf2bBwRGLKv809dIoWUc kdhzZy9FFo+tvRjZwMExPck/Cdq/I15mO5EndjGc= X-Original-To: libabigail@sourceware.org Delivered-To: libabigail@sourceware.org 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 EC306385841C for ; Wed, 11 Aug 2021 16:04:30 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org EC306385841C Received: from mail-wm1-f70.google.com (mail-wm1-f70.google.com [209.85.128.70]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-53-I-MPlsY7MeeLr5x-PudPWw-1; Wed, 11 Aug 2021 12:04:29 -0400 X-MC-Unique: I-MPlsY7MeeLr5x-PudPWw-1 Received: by mail-wm1-f70.google.com with SMTP id c2-20020a7bc8420000b0290238db573ab7so2251302wml.5 for ; Wed, 11 Aug 2021 09:04:29 -0700 (PDT) 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=OJQl/dN7dIi69FoFlcppyBLIX/48hWJLFro+ZGblHrg=; b=C86BetyVw1WPFsPUV2+/f7ZF9G9vqNWBP8vKZkkKtTfZfG3pZyeZ1CGMt28XJxJ6vp UrdKEUj321t2b4paPszhLhgFzR79QojEEIAVFjsZf3iGG4qWLglhXSJyRpoK/rCQVznE j2ZdeZOKVABc9MYW9XsQyU2zLRtf3xfuvtjx4lYQULzgUNC6s93MYV1BL9YUyvh8HsZc HhC5HSxafdRt5hY2d15fBVwxCwefjkP0smYa79qr0XGHXPf2HDXIDti4wDmz2/25JLMe BVSBDn9rNtxw8um5WfmdEnPojQL3Ihv3feZ+ANbIWatkjQfHuy8dwep6J0cw/XdoxeSf MPvw== X-Gm-Message-State: AOAM533zGKaoFEJlfp22cpLVD2DGbtDIfO+gdJnFpYWgwfKHV67Ibj+3 tAN7ne0QU7h2dQOEtx7/ePAoRM7wvjLvY6bnvmJOULK6PCo7IGfNydAtIfzL07hUMqkfSMt6azf eQdQed0MdFfZxviHq4sqF4nQbPUuVw1+3BvsrDcTFn46Vtj/7pw9G8ZfT+pfnw/pZqL37 X-Received: by 2002:a1c:27c2:: with SMTP id n185mr27956414wmn.20.1628697867944; Wed, 11 Aug 2021 09:04:27 -0700 (PDT) X-Google-Smtp-Source: ABdhPJzOvssxJd4uAOMl0FQZLLUEIbgf4/8xUdvDPnSAGozVPqVWEc/h0SutNPnQHjVzc8I8H4TmJg== X-Received: by 2002:a1c:27c2:: with SMTP id n185mr27956401wmn.20.1628697867723; Wed, 11 Aug 2021 09:04:27 -0700 (PDT) Received: from localhost ([88.120.130.27]) by smtp.gmail.com with ESMTPSA id l38sm5902519wmp.15.2021.08.11.09.04.26 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 11 Aug 2021 09:04:27 -0700 (PDT) Received: by localhost (Postfix, from userid 1000) id 5898A5800FB; Wed, 11 Aug 2021 18:04:23 +0200 (CEST) To: libabigail@sourceware.org Subject: [PATCH 3/8, applied] ir: Tighten type comparison optimization for Linux kernel binaries Organization: Red Hat / France X-Operating-System: Fedora 35 X-URL: http://www.redhat.com Date: Wed, 11 Aug 2021 18:04:23 +0200 Message-ID: <87y298c6ig.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 X-Spam-Status: No, score=-12.2 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_NONE, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) 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-Patchwork-Original-From: Dodji Seketeli via Libabigail From: Dodji Seketeli Reply-To: Dodji Seketeli Errors-To: libabigail-bounces+patchwork=sourceware.org@sourceware.org Sender: "Libabigail" Hello, types_defined_same_linux_kernel_corpus_public() performs an optimization while comparing two types in the context of the Linux kernel. If two types of the same kind and name are defined in the same corpus and in the same file, then they ought to be equal. For two anonymous classes that have naming typedefs, the function forgets to ensure that the naming typedefs have the same name. I have no binary that exhibits the potential issue, but I stumbled upon the problem while looking at something else that uncovered the problem. This change doesn't impact any of the binaries of the regression suite at the moment, though. Fixed thus. * src/abg-ir.cc (types_defined_same_linux_kernel_corpus_public): Ensure that anonymous classes with naming typedefs have identical typedef names. Signed-off-by: Dodji Seketeli Applied to master. --- src/abg-ir.cc | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/abg-ir.cc b/src/abg-ir.cc index 8e00eabf..f7739186 100644 --- a/src/abg-ir.cc +++ b/src/abg-ir.cc @@ -13161,6 +13161,16 @@ types_defined_same_linux_kernel_corpus_public(const type_base& t1, || (c2 && c2->get_is_anonymous() && !c2->get_naming_typedef())) return false; + // Two anonymous classes with naming typedefs should have the same + // typedef name. + if (c1 + && c2 + && c1->get_is_anonymous() && c1->get_naming_typedef() + && c2->get_is_anonymous() && c2->get_naming_typedef()) + if (c1->get_naming_typedef()->get_name() + != c2->get_naming_typedef()->get_name()) + return false; + // Two anonymous enum types cannot be eligible to this optimization. if (const enum_type_decl *e1 = is_enum_type(&t1)) if (const enum_type_decl *e2 = is_enum_type(&t2)) From patchwork Wed Aug 11 16:08:40 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dodji Seketeli X-Patchwork-Id: 44639 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 905683982413 for ; Wed, 11 Aug 2021 16:09:06 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 905683982413 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1628698146; bh=J523LUzNZSu82XqfQSqxgKuXT1Bz7PWitpLK1YXmzuo=; h=To:Subject:Date:List-Id:List-Unsubscribe:List-Archive:List-Help: List-Subscribe:From:Reply-To:From; b=W3bbQOqWZ0Zor+KTz+lGSbI4RPxjvlcC9UPXpYG1sqh4IanmuhpJQHNat8Fpp5gCU dzbkNTKBzAzITt7iPXEZZNSEl4Oem9WVh4OU0ZESIunMr6/8/GGvLW78oYr7cCVXHS 4FgB2kv/YRyxyLqrxqmwiNpvM1UyZMvPZ2AFYeOY= X-Original-To: libabigail@sourceware.org Delivered-To: libabigail@sourceware.org Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTP id B712B385841C for ; Wed, 11 Aug 2021 16:08:51 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org B712B385841C Received: from mail-wr1-f71.google.com (mail-wr1-f71.google.com [209.85.221.71]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-2-9c304nvIMmO8UdrShKFUDw-1; Wed, 11 Aug 2021 12:08:47 -0400 X-MC-Unique: 9c304nvIMmO8UdrShKFUDw-1 Received: by mail-wr1-f71.google.com with SMTP id o11-20020a5d474b0000b02901533f8ed22cso887619wrs.22 for ; Wed, 11 Aug 2021 09:08:47 -0700 (PDT) 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=J523LUzNZSu82XqfQSqxgKuXT1Bz7PWitpLK1YXmzuo=; b=Nw1pc94QAn71CtdFe/1ZZ8L07XUvD54psYiLowB8zVs/WRZD8xTAuVFJrpBXoGNIGu NiesXgOkPkzbDd8Ig7qnqvAe23YBqG+i+yCT+y67aRVZCUzO8i9IieDvdsxbCHy4rlc+ zqnWu0Ob8zdR+Scx3s0+qbUkyWhoabFeG3BbWfkXfY0/XSeeE1hqmsq5eZhvRaEqPEDn BJpdIRzMdH+38XHqRqumD7Epm6a8xjU5cRa6swmBr7buSekhA3niUobmVVWilb6c7F/V aC5HsF8Fag4SdEJHlGJtIh8NM/6+FDvgxiR3AbEnq/7/H8dYF8DRLpGhRLPtQG5uI4TM ML8g== X-Gm-Message-State: AOAM530OOnrlObvqB7FV6YOSAN2V73wijqc3iRWU6CX3KGTNK8svYEuw n0fAHxXgr+HmLnjicLVoye6xSNYo880DQjb3bRTGV7mcg9MZOEZvArjTB5EOv460Woe0tf4DBm5 GdanwhdSbaZvmnogfrvJVF76SfrYBEKR0sk4b6E1rpzTNSfMAb5GRA4qaLsTVN0ihFiJC X-Received: by 2002:a5d:5382:: with SMTP id d2mr10420858wrv.352.1628698125221; Wed, 11 Aug 2021 09:08:45 -0700 (PDT) X-Google-Smtp-Source: ABdhPJywyvcD6Fi9ulZPXIVtFkQaqdFO7lFag85Tb7KRIZv5NyL6fEMEWE5wCckTe9hUp+ikRL/7yQ== X-Received: by 2002:a5d:5382:: with SMTP id d2mr10420743wrv.352.1628698124229; Wed, 11 Aug 2021 09:08:44 -0700 (PDT) Received: from localhost ([88.120.130.27]) by smtp.gmail.com with ESMTPSA id i9sm6689454wre.36.2021.08.11.09.08.43 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 11 Aug 2021 09:08:43 -0700 (PDT) Received: by localhost (Postfix, from userid 1000) id 92F2D5800FB; Wed, 11 Aug 2021 18:08:40 +0200 (CEST) To: libabigail@sourceware.org Subject: [PATCH 5/8, applied] Bug 27236 - Fix the canonical type propagation optimization Organization: Red Hat / France X-Operating-System: Fedora 35 X-URL: http://www.redhat.com Date: Wed, 11 Aug 2021 18:08:40 +0200 Message-ID: <87pmukc6bb.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 X-Spam-Status: No, score=-12.3 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_NONE, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) 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-Patchwork-Original-From: Dodji Seketeli via Libabigail From: Dodji Seketeli Reply-To: Dodji Seketeli Errors-To: libabigail-bounces+patchwork=sourceware.org@sourceware.org Sender: "Libabigail" Hello, While working on another bug, it turned out the initial fix for the bug https://sourceware.org/bugzilla/show_bug.cgi?id=27236 was just papering over the real issue. I think the real issue is that "canonical type propagation" optimization was being done even in cases where it shouldn't have been done. This patch recognizes the limits of that optimization and avoid performing it when we are off limits. So here is what that optimization is. The text below is also present in the comments in the source code. I am putting it here to explain the context. During the canonicalization of a type T (which doesn't yet have a canonical type), T is compared structurally (member-wise) against a type C which already has a canonical type. The comparison expression is C == T. During that structural comparison, if a subtype of C (which also already has a canonical type) is structurally compared to a subtype of T (which doesn't yet have a canonical type) and if they are equal, then we can deduce that the canonical type of the subtype of C is the canonical type of the subtype of C. Thus, we can canonicalize the sub-type of the T, during the canonicalization of T itself. That canonicalization of the sub-type of T is what we call "propagating the canonical type of the sub-type of C onto the sub-type of T". It's also called "on-the-fly canonicalization". It's on the fly because it happens during a comparison -- which itself happens during the canonicalization of T. So this is the general description of the "canonical type propagation optimization". Now we must recognize the limits of that optimization. Said otherwise, there is a case when a type is *NOT* eligible to this canonical type propagation optimization. The reason why a type is deemed NON-eligible to the canonical type propagation optimization is that it "depends" on a recursively present type. Let me explain. Suppose we have a type T that has sub-types named ST0 and ST1. Suppose ST1 itself has a sub-type that is T itself. In this case, we say that T is a recursive type, because it has T (itself) as one of its sub-types: T +-- ST0 | +-- ST1 | + | | | +-- T | +-- ST2 ST1 is said to "depend" on T because it has T as a sub-type. But because T is recursive, then ST1 is said to depend on a recursive type. Notice however that ST0 does not depend on any recursive type. Now suppose we are comparing T to a type T' that has the same structure with sub-types ST0', ST1' and ST2'. During the comparison of ST1 against ST1', their sub-type T is compared against T'. Because T (resp. T') is a recursive type that is already being compared, the comparison of T against T' (as a subtypes of ST1 and ST1') returns true, meaning they are considered equal. This is done so that we don't enter an infinite recursion. That means ST1 is also deemed equal to ST1'. If we are in the course of the canonicalization of T' and thus if T (as well as as all of its sub-types) is already canonicalized, then the canonical type propagation optimization will make us propagate the canonical type of ST1 onto ST1'. So the canonical type of ST1' will be equal to the canonical type of ST1 as a result of that optmization. But then, later down the road, when ST2 is compared against ST2', let's suppose that we find out that they are different. Meaning that ST2 != ST2'. This means that T != T', i.e, the canonicalization of T' failed for now. But most importantly, it means that the propagation of the canonical type of ST1 to ST1' must now be invalidated. Meaning, ST1' must now be considered as not having any canonical type. In other words, during type canonicalization, if ST1' depends on a recursive type T', its propagated canonical type must be invalidated (set to nullptr) if T' appears to be different from T, a.k.a, the canonicalization of T' temporarily failed. This means that any sub-type that depends on recursive types and that has been the target of the canonical type propagation optimization must be tracked. If the dependant recursive type fails its canonicalization, then the sub-type being compared must have its propagated canonical type cleared. In other words, its propagated canonical type must be cancelled. This concept of cancelling the propagated canonical type when needed is what this patch introduces. New data members have been introduced to the environment::priv private structure. Those are to keep track of the stack of sub-types being compared so that we can detect if a candidate to the canonical type propagation optimization depends on a recursive type. There is also a data structure in there to track the targets of the canonical type propagation optimization that "might" need to see their propagated canonical types be cancelled. Then new functions have been introduced to detect when a type depends on a recursive type, to cancel or confirm propagated canonical types etc. In abg-ir.cc, The RETURN* macros used in the equals() overloads have been factorized using the newly introduced function templates return_comparison_result(). This now contains the book keeping that was previously done (in the RETURN* macros) to detect recursive cycles in the comparison, as well as triggering the canonical type propagation. This i also where the logic of properly limiting the optimization is implemented now. * include/abg-ir.h (pointer_set): This typedef is now for an unordered_set rather than an unordered_set. (environment::priv_): Make this public so that code in free form function from abg-ir.cc can access it. * src/abg-ir-priv.h (struct type_base::priv): Move this private structure here, from abg-ir.cc. (type_base::priv::{depends_on_recursive_type_, canonical_type_propagated_}): Added these two new data members. (type_base::priv::priv): Initialize the two new data members. (type_base::priv::{depends_on_recursive_type, set_depends_on_recursive_type, set_does_not_depend_on_recursive_type, canonical_type_propagated, set_canonical_type_propagated, clear_propagated_canonical_type}): Define new member functions. (struct environment::priv): Move this struct here, from abg-ir.cc. (environment::priv::{types_with_non_confirmed_propagated_ct_, left_type_comp_operands_, right_type_comp_operands_}): New data members. (environment::priv::{mark_dependant_types, mark_dependant_types_compared_until, confirm_ct_propagation, collect_types_that_depends_on, cancel_ct_propagation, remove_from_types_with_non_confirmed_propagated_ct}): New member functions. * src/abg-ir.cc (struct environment::priv, struct) (type_base::priv, struct class_or_union::priv): Move these struct to include/abg-ir-priv.h. (push_composite_type_comparison_operands) (pop_composite_type_comparison_operands) (mark_dependant_types_compared_until) (maybe_cancel_propagated_canonical_type): Define new functions. (notify_equality_failed, mark_types_as_being_compared): Re-indent. (is_comparison_cycle_detected, return_comparison_result): Define new function templates. (RETURN_TRUE_IF_COMPARISON_CYCLE_DETECTED): Define new macro. (equals(const function_type& l, const function_type& r)): Redefine the RETURN macro using the new return_comparison_result function template. Use the new RETURN_TRUE_IF_COMPARISON_CYCLE_DETECTED and mark_types_as_being_compared functions. (equals(const class_or_union& l, const class_or_union&, change_kind*)): Likewise. (equals(const class_decl& l, const class_decl&, change_kind*)): Likewise. Because this uses another equal() function to compare the class_or_union part the type, ensure that no canonical type propagation occurs at that point. (types_are_being_compared): Remove as it's not used anymore. (maybe_propagate_canonical_type): Use the new environment::priv::propagate_ct() function here. (method_matches_at_least_one_in_vector): Ensure the right-hand-side operand of the equality stays on the right. This is important because the equals() functions expect that. * src/abg-reader.cc (build_type): Ensure all types are canonicalized. * tests/data/test-diff-dwarf/PR25058-liblttng-ctl-report-1.txt: Adjust. * tests/data/test-diff-pkg/nss-3.23.0-1.0.fc23.x86_64-report-0.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/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/test-libaaudio.so.abi: Likewise. Signed-off-by: Dodji Seketeli Applied to master. --- include/abg-ir.h | 8 +- src/abg-ir-priv.h | 661 ++++++++++++++++ src/abg-ir.cc | 705 ++++++++++-------- src/abg-reader.cc | 2 + .../PR25058-liblttng-ctl-report-1.txt | 7 +- .../nss-3.23.0-1.0.fc23.x86_64-report-0.txt | 6 +- ...l7.x86_64-0.12.8-1.el7.x86_64-report-2.txt | 86 ++- ...bb-4.3-3.20141204.fc23.x86_64-report-0.txt | 41 +- ...bb-4.3-3.20141204.fc23.x86_64-report-1.txt | 2 +- .../test-read-dwarf/test-libaaudio.so.abi | 12 +- 10 files changed, 1196 insertions(+), 334 deletions(-) diff --git a/include/abg-ir.h b/include/abg-ir.h index defc7e64..8979f10c 100644 --- a/include/abg-ir.h +++ b/include/abg-ir.h @@ -95,8 +95,8 @@ namespace ir // Inject some std types in here. using std::unordered_map; -/// A convenience typedef fo r an ordered set of size_t. -typedef unordered_set pointer_set; +/// A convenience typedef for an unordered set of pointer values +typedef unordered_set pointer_set; /// Functor to hash a canonical type by using its pointer value. struct canonical_type_hash @@ -133,17 +133,17 @@ typedef vector type_base_sptrs_type; /// that you can de-allocate the environment instance. class environment { +public: struct priv; std::unique_ptr priv_; -public: - /// A convenience typedef for a map of canonical types. The key is /// the pretty representation string of a particular type and the /// value is the vector of canonical types that have the same pretty /// representation string. typedef std::unordered_map > canonical_types_map_type; + environment(); virtual ~environment(); diff --git a/src/abg-ir-priv.h b/src/abg-ir-priv.h index 9a2b3870..9285dd73 100644 --- a/src/abg-ir-priv.h +++ b/src/abg-ir-priv.h @@ -151,6 +151,667 @@ struct translation_unit::priv {return types_;} }; // end translation_unit::priv +// + +/// Definition of the private data of @ref type_base. +struct type_base::priv +{ + size_t size_in_bits; + size_t alignment_in_bits; + type_base_wptr canonical_type; + // The data member below holds the canonical type that is managed by + // the smart pointer referenced by the canonical_type data member + // above. We are storing this underlying (naked) pointer here, so + // that users can access it *fast*. Otherwise, accessing + // canonical_type above implies creating a shared_ptr, and that has + // been measured to be slow for some performance hot spots. + type_base* naked_canonical_type; + // Computing the representation of a type again and again can be + // costly. So we cache the internal and non-internal type + // representation strings here. + interned_string internal_cached_repr_; + interned_string cached_repr_; + // The next two data members are used while comparing types during + // canonicalization. They are useful for the "canonical type + // propagation" (aka on-the-fly-canonicalization) optimization + // implementation. + + // The set of canonical recursive types this type depends on. + unordered_set depends_on_recursive_type_; + bool canonical_type_propagated_; + + priv() + : size_in_bits(), + alignment_in_bits(), + naked_canonical_type(), + canonical_type_propagated_(false) + {} + + priv(size_t s, + size_t a, + type_base_sptr c = type_base_sptr()) + : size_in_bits(s), + alignment_in_bits(a), + canonical_type(c), + naked_canonical_type(c.get()), + canonical_type_propagated_(false) + {} + + /// Test if the current type depends on recursive type comparison. + /// + /// A recursive type T is a type T which has a sub-type that is T + /// (recursively) itself. + /// + /// So this function tests if the current type has a recursive + /// sub-type or is a recursive type itself. + /// + /// @return true if the current type depends on a recursive type. + bool + depends_on_recursive_type() const + {return !depends_on_recursive_type_.empty();} + + /// Test if the current type depends on a given recursive type. + /// + /// A recursive type T is a type T which has a sub-type that is T + /// (recursively) itself. + /// + /// So this function tests if the current type depends on a given + /// recursive type. + /// + /// @param dependant the type we want to test if the current type + /// depends on. + /// + /// @return true if the current type depends on the recursive type + /// @dependant. + bool + depends_on_recursive_type(const type_base* dependant) const + { + return + (depends_on_recursive_type_.find(reinterpret_cast(dependant)) + != depends_on_recursive_type_.end()); + } + + /// Set the flag that tells if the current type depends on a given + /// recursive type. + /// + /// A recursive type T is a type T which has asub-type that is T + /// (recursively) itself. + /// + /// So this function tests if the current type depends on a + /// recursive type. + /// + /// @param t the recursive type that current type depends on. + void + set_depends_on_recursive_type(const type_base * t) + {depends_on_recursive_type_.insert(reinterpret_cast(t));} + + /// Unset the flag that tells if the current type depends on a given + /// recursive type. + /// + /// A recursive type T is a type T which has asub-type that is T + /// (recursively) itself. + /// + /// So this function flags the current type as not being dependant + /// on a given recursive type. + /// + /// + /// @param t the recursive type to consider. + void + set_does_not_depend_on_recursive_type(const type_base *t) + {depends_on_recursive_type_.erase(reinterpret_cast(t));} + + /// Flag the current type as not being dependant on any recursive type. + void + set_does_not_depend_on_recursive_type() + {depends_on_recursive_type_.clear();} + + /// Test if the type carries a canonical type that is the result of + /// maybe_propagate_canonical_type(), aka, "canonical type + /// propagation optimization". + /// + /// @return true iff the current type carries a canonical type that + /// is the result of canonical type propagation. + bool + canonical_type_propagated() + {return canonical_type_propagated_;} + + /// Set the flag that says if the type carries a canonical type that + /// is the result of maybe_propagate_canonical_type(), aka, + /// "canonical type propagation optimization". + /// + /// @param f true iff the current type carries a canonical type that + /// is the result of canonical type propagation. + void + set_canonical_type_propagated(bool f) + {canonical_type_propagated_ = f;} + + /// If the current canonical type was set as the result of the + /// "canonical type propagation optimization", then clear it. + void + clear_propagated_canonical_type() + { + if (canonical_type_propagated_) + { + canonical_type.reset(); + naked_canonical_type = nullptr; + set_canonical_type_propagated(false); + } + } +}; // end struct type_base::priv + +// +/// The private data of the @ref environment type. +struct environment::priv +{ + config config_; + canonical_types_map_type canonical_types_; + mutable vector sorted_canonical_types_; + type_base_sptr void_type_; + type_base_sptr variadic_marker_type_; + unordered_set classes_being_compared_; + unordered_set fn_types_being_compared_; + vector extra_live_types_; + interned_string_pool string_pool_; + // The two vectors below represent the stack of left and right + // operands of the current type comparison operation that is + // happening during type canonicalization. + // + // Basically, that stack of operand looks like below. + // + // First, suppose we have a type T_L that has two sub-types as this: + // + // T_L + // | + // +-- L_OP0 + // | + // +-- L_OP1 + // + // Now suppose that we have another type T_R that has two sub-types + // as this: + // + // T_R + // | + // +-- R_OP0 + // | + // +-- R_OP1 + // + // Now suppose that we compare T_L against T_R. We are going to + // have a stack of pair of types. Each pair of types represents + // two (sub) types being compared against each other. + // + // On the stack, we will thus first have the pair (T_L, T_R) + // being compared. Then, we will have the pair (L_OP0, R_OP0) + // being compared, and then the pair (L_OP1, R_OP1) being + // compared. Like this: + // + // | T_L | L_OP0 | L_OP1 | <-- this goes into left_type_comp_operands_; + // -------- ------------- + // | T_R | R_OP0 | R_OP1 | <-- this goes into right_type_comp_operands_; + // + // This "stack of operands of the current type comparison, during + // type canonicalization" is used in the context of the @ref + // OnTheFlyCanonicalization optimization. It's used to detect if a + // sub-type of the type being canonicalized depends on a recursive + // type. + vector left_type_comp_operands_; + vector right_type_comp_operands_; + // Vector of types that protentially received propagated canonical types. + // If the canonical type propagation is confirmed, the potential + // canonical types must be promoted as canonical types. Otherwise if + // the canonical type propagation is cancelled, the canonical types + // must be cleared. + pointer_set types_with_non_confirmed_propagated_ct_; +#ifdef WITH_DEBUG_SELF_COMPARISON + // This is used for debugging purposes. + // When abidw is used with the option --debug-abidiff, some + // libabigail internals need to get a hold on the initial binary + // input of abidw, as well as as the abixml file that represents the + // ABI of that binary. + // + // So this one is the corpus for the input binary. + corpus_wptr first_self_comparison_corpus_; + // This one is the corpus for the ABIXML file representing the + // serialization of the input binary. + corpus_wptr second_self_comparison_corpus_; + // This is also used for debugging purposes, when using + // 'abidw --debug-abidiff '. It holds the set of mapping of + // an abixml (canonical) type and its type-id. + unordered_map type_id_canonical_type_map_; + // Likewise. It holds a map that associates the pointer to a type read from + // abixml and the type-id string it corresponds to. + unordered_map pointer_type_id_map_; +#endif + bool canonicalization_is_done_; + bool do_on_the_fly_canonicalization_; + bool decl_only_class_equals_definition_; + bool use_enum_binary_only_equality_; +#ifdef WITH_DEBUG_SELF_COMPARISON + bool self_comparison_debug_on_; +#endif + + priv() + : canonicalization_is_done_(), + do_on_the_fly_canonicalization_(true), + decl_only_class_equals_definition_(false), + use_enum_binary_only_equality_(true) +#ifdef WITH_DEBUG_SELF_COMPARISON + , + self_comparison_debug_on_(false) +#endif + {} + + /// Push a pair of operands on the stack of operands of the current + /// type comparison, during type canonicalization. + /// + /// For more information on this, please look at the description of + /// the right_type_comp_operands_ data member. + /// + /// @param left the left-hand-side comparison operand to push. + /// + /// @param right the right-hand-side comparison operand to push. + void + push_composite_type_comparison_operands(const type_base* left, + const type_base* right) + { + ABG_ASSERT(left && right); + + left_type_comp_operands_.push_back(left); + right_type_comp_operands_.push_back(right); + } + + /// Pop a pair of operands from the stack of operands to the current + /// type comparison. + /// + /// For more information on this, please look at the description of + /// the right_type_comp_operands_ data member. + /// + /// @param left the left-hand-side comparison operand we expect to + /// pop from the top of the stack. If this doesn't match the + /// operand found on the top of the stack, the function aborts. + /// + /// @param right the right-hand-side comparison operand we expect to + /// pop from the bottom of the stack. If this doesn't match the + /// operand found on the top of the stack, the function aborts. + void + pop_composite_type_comparison_operands(const type_base* left, + const type_base* right) + { + const type_base *t = left_type_comp_operands_.back(); + ABG_ASSERT(t == left); + t = right_type_comp_operands_.back(); + ABG_ASSERT(t == right); + + left_type_comp_operands_.pop_back(); + right_type_comp_operands_.pop_back(); + } + + /// Mark all the types that comes after a certain one as NOT being + /// eligible for the canonical type propagation optimization. + /// + /// @param type the type that represents the "marker type". All + /// types after this one will be marked as being NON-eligible to + /// the canonical type propagation optimization. + /// + /// @param types the set of types to consider. In that vector, all + /// types that come after @p type are going to be marked as being + /// non-eligible to the canonical type propagation optimization. + /// + /// @return true iff the operation was successful. + bool + mark_dependant_types(const type_base* type, + vector& types) + { + bool found = false; + for (auto t : types) + { + if (!found + && (reinterpret_cast(t) + == reinterpret_cast(type))) + { + found = true; + continue; + } + else if (found) + t->priv_->set_depends_on_recursive_type(type); + } + return found; + } + + /// In the stack of the current types being compared (as part of + /// type canonicalization), mark all the types that comes after a + /// certain one as NOT being eligible to the canonical type + /// propagation optimization. + /// + /// For a starter, please read about the @ref + /// OnTheFlyCanonicalization, aka, "canonical type propagation + /// optimization". + /// + /// To implement that optimization, we need, among other things to + /// maintain stack of the types (and their sub-types) being + /// currently compared as part of type canonicalization. + /// + /// Note that we only consider the type that is the right-hand-side + /// operand of the comparison because it's that one that is being + /// canonicalized and thus, that is not yet canonicalized. + /// + /// The reason why a type is deemed NON-eligible to the canonical + /// type propagation optimization is that it "depends" on + /// recursively present type. Let me explain. + /// + /// Suppose we have a type T that has sub-types named ST0 and ST1. + /// Suppose ST1 itself has a sub-type that is T itself. In this + /// case, we say that T is a recursive type, because it has T + /// (itself) as one of its sub-types: + /// + /// T + /// +-- ST0 + /// | + /// +-- ST1 + /// + + /// | + /// +-- T + /// + /// ST1 is said to "depend" on T because it has T as a sub-type. + /// But because T is recursive, then ST1 is said to depend on a + /// recursive type. Notice however that ST0 does not depend on any + /// recursive type. + /// + /// When we are at the point of comparing the sub-type T of ST1 + /// against its counterpart, the stack of the right-hand-side + /// operands of the type canonicalization is going to look like + /// this: + /// + /// | T | ST1 | + /// + /// We don't add the type T to the stack as we detect that T was + /// already in there (recursive cycle). + /// + /// So, this function will basically mark ST1 as being NON-eligible + /// to being the target of canonical type propagation. + /// + /// @param right the right-hand-side operand of the type comparison. + /// + /// @return true iff the operation was successful. + bool + mark_dependant_types_compared_until(const type_base* right) + { + bool result = false; + + result |= + mark_dependant_types(right, + right_type_comp_operands_); + return result; + } + + /// Propagate the canonical type of a type to another one. + /// + /// @param src the type to propagate the canonical type from. + /// + /// @param dest the type to propagate the canonical type of @p src + /// to. + /// + /// @return bool iff the canonical was propagated. + bool + propagate_ct(const type_base& src, const type_base& dest) + { + type_base_sptr canonical = src.get_canonical_type(); + ABG_ASSERT(canonical); + dest.priv_->canonical_type = canonical; + dest.priv_->naked_canonical_type = canonical.get(); + dest.priv_->set_canonical_type_propagated(true); + return true; + } + + /// Mark a set of types that have been the target of canonical type + /// propagation and that depend on a recursive type as being + /// permanently canonicalized. + /// + /// To understand the sentence above, please read the description of + /// type canonicalization and especially about the "canonical type + /// propagation optimization" at @ref OnTheFlyCanonicalization, in + /// the src/abg-ir.cc file. + void + confirm_ct_propagation(const type_base* dependant_type) + { + pointer_set to_remove; + for (auto i : types_with_non_confirmed_propagated_ct_) + { + type_base *t = reinterpret_cast(i); + ABG_ASSERT(t->priv_->depends_on_recursive_type()); + t->priv_->set_does_not_depend_on_recursive_type(dependant_type); + if (!t->priv_->depends_on_recursive_type()) + to_remove.insert(i); + } + + for (auto i : to_remove) + types_with_non_confirmed_propagated_ct_.erase(i); + } + + /// Collect the types that depends on a given "target" type. + /// + /// Walk a set of types and if they depend directly or indirectly on + /// a "target" type, then collect them into a set. + /// + /// @param target the target type to consider. + /// + /// @param types the types to walk to detect those who depend on @p + /// target. + /// + /// @return true iff one or more type from @p types is found to + /// depend on @p target. + bool + collect_types_that_depends_on(const type_base *target, + const pointer_set& types, + pointer_set& collected) + { + bool result = false; + for (const auto i : types) + { + type_base *t = reinterpret_cast(i); + if (t->priv_->depends_on_recursive_type(target)) + { + collected.insert(i); + collect_types_that_depends_on(t, types, collected); + result = true; + } + } + return result; + } + + /// Reset the canonical type (set it nullptr) of a set of types that + /// have been the target of canonical type propagation and that + /// depend on a given recursive type. + /// + /// Once the canonical type of a type in that set is reset, the type + /// is marked as non being dependant on a recursive type anymore. + /// + /// To understand the sentences above, please read the description + /// of type canonicalization and especially about the "canonical + /// type propagation optimization" at @ref OnTheFlyCanonicalization, + /// in the src/abg-ir.cc file. + /// + /// @param target if a type which has been subject to the canonical + /// type propagation optimizationdepends on a this target type, then + /// cancel its canonical type. + void + cancel_ct_propagation(const type_base* target) + { + pointer_set to_remove; + collect_types_that_depends_on(target, + types_with_non_confirmed_propagated_ct_, + to_remove); + + for (auto i : to_remove) + { + type_base *t = reinterpret_cast(i); + ABG_ASSERT(t->priv_->depends_on_recursive_type()); + type_base_sptr canonical = t->priv_->canonical_type.lock(); + if (canonical) + { + t->priv_->clear_propagated_canonical_type(); + t->priv_->set_does_not_depend_on_recursive_type(); + } + } + + for (auto i : to_remove) + types_with_non_confirmed_propagated_ct_.erase(i); + } + + /// Remove a given type from the set of types that have been + /// non-confirmed subjects of the canonical type propagation + /// optimization. + /// + /// @param dependant the dependant type to consider. + void + remove_from_types_with_non_confirmed_propagated_ct(const type_base* dependant) + { + uintptr_t i = reinterpret_cast(dependant); + types_with_non_confirmed_propagated_ct_.erase(i); + } + +};// end struct environment::priv + +// +struct class_or_union::priv +{ + typedef_decl_wptr naming_typedef_; + member_types member_types_; + data_members data_members_; + data_members non_static_data_members_; + member_functions member_functions_; + // A map that associates a linkage name to a member function. + string_mem_fn_sptr_map_type mem_fns_map_; + // A map that associates function signature strings to member + // function. + string_mem_fn_ptr_map_type signature_2_mem_fn_map_; + member_function_templates member_function_templates_; + member_class_templates member_class_templates_; + + priv() + {} + + priv(class_or_union::member_types& mbr_types, + class_or_union::data_members& data_mbrs, + class_or_union::member_functions& mbr_fns) + : member_types_(mbr_types), + data_members_(data_mbrs), + member_functions_(mbr_fns) + { + for (data_members::const_iterator i = data_members_.begin(); + i != data_members_.end(); + ++i) + if (!get_member_is_static(*i)) + non_static_data_members_.push_back(*i); + } + + /// Mark a class or union or union as being currently compared using + /// the class_or_union== operator. + /// + /// Note that is marking business is to avoid infinite loop when + /// comparing a class or union or union. If via the comparison of a + /// data member or a member function a recursive re-comparison of + /// the class or union is attempted, the marking business help to + /// detect that infinite loop possibility and avoid it. + /// + /// @param klass the class or union or union to mark as being + /// currently compared. + void + mark_as_being_compared(const class_or_union& klass) const + { + const environment* env = klass.get_environment(); + ABG_ASSERT(env); + env->priv_->classes_being_compared_.insert(&klass); + } + + /// Mark a class or union as being currently compared using the + /// class_or_union== operator. + /// + /// Note that is marking business is to avoid infinite loop when + /// comparing a class or union. If via the comparison of a data + /// member or a member function a recursive re-comparison of the + /// class or union is attempted, the marking business help to detect + /// that infinite loop possibility and avoid it. + /// + /// @param klass the class or union to mark as being currently + /// compared. + void + mark_as_being_compared(const class_or_union* klass) const + {mark_as_being_compared(*klass);} + + /// Mark a class or union as being currently compared using the + /// class_or_union== operator. + /// + /// Note that is marking business is to avoid infinite loop when + /// comparing a class or union. If via the comparison of a data + /// member or a member function a recursive re-comparison of the + /// class or union is attempted, the marking business help to detect + /// that infinite loop possibility and avoid it. + /// + /// @param klass the class or union to mark as being currently + /// compared. + void + mark_as_being_compared(const class_or_union_sptr& klass) const + {mark_as_being_compared(*klass);} + + /// If the instance of class_or_union has been previously marked as + /// being compared -- via an invocation of mark_as_being_compared() + /// this method unmarks it. Otherwise is has no effect. + /// + /// This method is not thread safe because it uses the static data + /// member classes_being_compared_. If you wish to use it in a + /// multi-threaded environment you should probably protect the + /// access to that static data member with a mutex or somesuch. + /// + /// @param klass the instance of class_or_union to unmark. + void + unmark_as_being_compared(const class_or_union& klass) const + { + const environment* env = klass.get_environment(); + ABG_ASSERT(env); + env->priv_->classes_being_compared_.erase(&klass); + } + + /// If the instance of class_or_union has been previously marked as + /// being compared -- via an invocation of mark_as_being_compared() + /// this method unmarks it. Otherwise is has no effect. + /// + /// @param klass the instance of class_or_union to unmark. + void + unmark_as_being_compared(const class_or_union* klass) const + { + if (klass) + return unmark_as_being_compared(*klass); + } + + /// Test if a given instance of class_or_union is being currently + /// compared. + /// + ///@param klass the class or union to test. + /// + /// @return true if @p klass is being compared, false otherwise. + bool + comparison_started(const class_or_union& klass) const + { + const environment* env = klass.get_environment(); + ABG_ASSERT(env); + return env->priv_->classes_being_compared_.count(&klass); + } + + /// Test if a given instance of class_or_union is being currently + /// compared. + /// + ///@param klass the class or union to test. + /// + /// @return true if @p klass is being compared, false otherwise. + bool + comparison_started(const class_or_union* klass) const + { + if (klass) + return comparison_started(*klass); + return false; + } +}; // end struct class_or_union::priv + } // end namespace ir } // end namespace abigail diff --git a/src/abg-ir.cc b/src/abg-ir.cc index 0eb42f38..7d575259 100644 --- a/src/abg-ir.cc +++ b/src/abg-ir.cc @@ -264,6 +264,118 @@ has_generic_anonymous_internal_type_name(const decl_base *d); static interned_string get_generic_anonymous_internal_type_name(const decl_base *d); +void +push_composite_type_comparison_operands(const type_base& left, + const type_base& right); + +void +pop_composite_type_comparison_operands(const type_base& left, + const type_base& right); + +bool +mark_dependant_types_compared_until(const type_base &r); + +/// Push a pair of operands on the stack of operands of the current +/// type comparison, during type canonicalization. +/// +/// For more information on this, please look at the description of +/// the environment::priv::right_type_comp_operands_ data member. +/// +/// @param left the left-hand-side comparison operand to push. +/// +/// @param right the right-hand-side comparison operand to push. +void +push_composite_type_comparison_operands(const type_base& left, + const type_base& right) +{ + const environment * env = left.get_environment(); + env->priv_->push_composite_type_comparison_operands(&left, &right); +} + +/// Pop a pair of operands from the stack of operands to the current +/// type comparison. +/// +/// For more information on this, please look at the description of +/// the environment::privright_type_comp_operands_ data member. +/// +/// @param left the left-hand-side comparison operand we expect to +/// pop from the top of the stack. If this doesn't match the +/// operand found on the top of the stack, the function aborts. +/// +/// @param right the right-hand-side comparison operand we expect to +/// pop from the bottom of the stack. If this doesn't match the +/// operand found on the top of the stack, the function aborts. +void +pop_composite_type_comparison_operands(const type_base& left, + const type_base& right) +{ + const environment * env = left.get_environment(); + env->priv_->pop_composite_type_comparison_operands(&left, &right); +} + +/// In the stack of the current types being compared (as part of type +/// canonicalization), mark all the types that comes after a certain +/// one as NOT being eligible to the canonical type propagation +/// optimization. +/// +/// For a starter, please read about the @ref +/// OnTheFlyCanonicalization, aka, "canonical type propagation +/// optimization". +/// +/// To implement that optimization, we need, among other things to +/// maintain stack of the types (and their sub-types) being +/// currently compared as part of type canonicalization. +/// +/// Note that we only consider the type that is the right-hand-side +/// operand of the comparison because it's that one that is being +/// canonicalized and thus, that is not yet canonicalized. +/// +/// The reason why a type is deemed NON-eligible to the canonical +/// type propagation optimization is that it "depends" on +/// recursively present type. Let me explain. +/// +/// Suppose we have a type T that has sub-types named ST0 and ST1. +/// Suppose ST1 itself has a sub-type that is T itself. In this +/// case, we say that T is a recursive type, because it has T +/// (itself) as one of its sub-types: +/// +/// T +/// +-- ST0 +/// | +/// +-- ST1 +/// + +/// | +/// +-- T +/// +/// ST1 is said to "depend" on T because it has T as a sub-type. +/// But because T is recursive, then ST1 is said to depend on a +/// recursive type. Notice however that ST0 does not depend on any +/// recursive type. +/// +/// When we are at the point of comparing the sub-type T of ST1 +/// against its counterpart, the stack of the right-hand-side +/// operands of the type canonicalization is going to look like +/// this: +/// +/// | T | ST1 | +/// +/// We don't add the type T to the stack as we detect that T was +/// already in there (recursive cycle). +/// +/// So, this function will basically mark ST1 as being NON-eligible +/// to being the target of canonical type propagation, by marking ST1 +/// as being dependant on T. +/// +/// @param right the right-hand-side operand of the type comparison. +/// +/// @return true iff the operation was successful. +bool +mark_dependant_types_compared_until(const type_base &r) +{ + const environment * env = r.get_environment(); + return env->priv_->mark_dependant_types_compared_until(&r); +} + /// @brief the location of a token represented in its simplest form. /// Instances of this type are to be stored in a sorted vector, so the /// type must have proper relational operators. @@ -723,7 +835,7 @@ notify_equality_failed(const type_or_decl_base *l __attribute__((unused)), { \ if (value == false) \ notify_equality_failed(l, r); \ - return value; \ + return value; \ } while (false) #else // WITH_DEBUG_SELF_COMPARISON @@ -752,6 +864,159 @@ try_canonical_compare(const T *l, const T *r) return equals(*l, *r, 0); } +/// Detect if a recursive comparison cycle is detected while +/// structurally comparing two types (a.k.a member-wise comparison). +/// +/// @param l the left-hand-side operand of the current comparison. +/// +/// @param r the right-hand-side operand of the current comparison. +/// +/// @return true iff a comparison cycle is detected. +template +bool +is_comparison_cycle_detected(T& l, T& r) +{ + bool result = (l.priv_->comparison_started(l) + || l.priv_->comparison_started(r)); + return result ; +} + +/// This macro is to be used while comparing composite types that +/// might recursively refer to themselves. Comparing two such types +/// might get us into a cyle. +/// +/// Practically, if we detect that we are already into comparing 'l' +/// and 'r'; then, this is a cycle. +// +/// To break the cycle, we assume the result of the comparison is true +/// for now. Comparing the other sub-types of l & r will tell us later +/// if l & r are actually different or not. +/// +/// In the mean time, returning true from this macro should not be +/// used to propagate the canonical type of 'l' onto 'r' as we don't +/// know yet if l equals r. All the types that depend on l and r +/// can't (and that are in the comparison stack currently) can't have +/// their canonical type propagated either. So this macro disallows +/// canonical type propagation for those types that depend on a +/// recursively defined sub-type for now. +/// +/// @param l the left-hand-side operand of the comparison. +#define RETURN_TRUE_IF_COMPARISON_CYCLE_DETECTED(l, r) \ + do \ + { \ + if (is_comparison_cycle_detected(l, r)) \ + { \ + mark_dependant_types_compared_until(r); \ + return true; \ + } \ + } \ + while(false) + + +/// Mark a pair of types as being compared. +/// +/// This is helpful to later detect recursive cycles in the comparison +/// stack. +/// +/// @param l the left-hand-side operand of the comparison. +/// +/// @parm r the right-hand-side operand of the comparison. +template +void +mark_types_as_being_compared(T& l, T&r) +{ + l.priv_->mark_as_being_compared(l); + l.priv_->mark_as_being_compared(r); + push_composite_type_comparison_operands(l, r); +} + +/// Return the result of the comparison of two (sub) types. +/// +/// The function does the necessary book keeping before returning the +/// result of the comparison of two (sub) types. +/// +/// The book-keeping done is in the following +/// areas: +/// +/// * Management of the Canonical Type Propagation optimization +/// * type comparison cycle detection +/// +/// @param l the left-hand-side operand of the type comparison +/// +/// @param r the right-hand-side operand of the type comparison +/// +/// @param value the result of the comparison of @p l and @p r. +/// +/// @return the value @p value. +template +bool +return_comparison_result(T& l, T& r, bool value) +{ + if (value == true) + maybe_propagate_canonical_type(l, r); + l.priv_->unmark_as_being_compared(l); + l.priv_->unmark_as_being_compared(r); + + pop_composite_type_comparison_operands(l, r); + const environment* env = l.get_environment(); + if (env->do_on_the_fly_canonicalization()) + // We are instructed to perform the "canonical type propagation" + // optimization, making 'r' to possibly get the canonical type of + // 'l' if it has one. This mostly means that we are currently + // canonicalizing the type that contain the subtype provided in + // the 'r' argument. + { + if (value == true + && is_type(&r)->priv_->depends_on_recursive_type() + && !env->priv_->right_type_comp_operands_.empty() + && is_type(&r)->priv_->canonical_type_propagated()) + { + // Track the object 'r' for which the propagated canonical + // type might be re-initialized if the current comparison + // eventually fails. + env->priv_->types_with_non_confirmed_propagated_ct_.insert + (reinterpret_cast(is_type(&r))); + } + else if (value == true && env->priv_->right_type_comp_operands_.empty()) + { + // The type provided in the 'r' argument is the type that is + // being canonicalized; 'r' is not a mere subtype being + // compared, it's the whole type being canonicalized. And + // its canonicalization has just succeeded. So let's + // confirm the "canonical type propagation" of all the + // sub-types that were compared during the comparison of + // 'r'. + env->priv_->confirm_ct_propagation(&r); + if (is_type(&r)->priv_->depends_on_recursive_type()) + { + is_type(&r)->priv_->set_does_not_depend_on_recursive_type(); + env->priv_->remove_from_types_with_non_confirmed_propagated_ct(&r); + } + } + else if (value == false) + { + // The comparison of the current sub-type failed. So all + // the types in + // env->prix_->types_with_non_confirmed_propagated_ct_ + // should see their tentatively propagated canonical type + // cancelled. + env->priv_->cancel_ct_propagation(&r); + if (is_type(&r)->priv_->depends_on_recursive_type()) + { + // The right-hand-side operand cannot carry any tentative + // canonical type at this point. + type_base* canonical_type = r.get_naked_canonical_type(); + ABG_ASSERT(canonical_type == nullptr); + // Reset the marking of the right-hand-side operand as it no + // longer carries a tentative canonical type that might be + // later cancelled. + is_type(&r)->priv_->set_does_not_depend_on_recursive_type(); + } + } + } + ABG_RETURN(value); +} + /// Getter of all types types sorted by their pretty representation. /// /// @return a sorted vector of all types sorted by their pretty @@ -2836,57 +3101,6 @@ dm_context_rel::~dm_context_rel() typedef unordered_map interned_string_bool_map_type; -/// The private data of the @ref environment type. -struct environment::priv -{ - config config_; - canonical_types_map_type canonical_types_; - mutable vector sorted_canonical_types_; - type_base_sptr void_type_; - type_base_sptr variadic_marker_type_; - unordered_set classes_being_compared_; - unordered_set fn_types_being_compared_; - vector extra_live_types_; - interned_string_pool string_pool_; -#ifdef WITH_DEBUG_SELF_COMPARISON - // This is used for debugging purposes. - // When abidw is used with the option --debug-abidiff, some - // libabigail internals need to get a hold on the initial binary - // input of abidw, as well as as the abixml file that represents the - // ABI of that binary. - // - // So this one is the corpus for the input binary. - corpus_wptr first_self_comparison_corpus_; - // This one is the corpus for the ABIXML file representing the - // serialization of the input binary. - corpus_wptr second_self_comparison_corpus_; - // This is also used for debugging purposes, when using - // 'abidw --debug-abidiff '. It holds the set of mapping of - // an abixml (canonical) type and its type-id. - unordered_map type_id_canonical_type_map_; - // Likewise. It holds a map that associates the pointer to a type read from - // abixml and the type-id string it corresponds to. - unordered_map pointer_type_id_map_; -#endif - bool canonicalization_is_done_; - bool do_on_the_fly_canonicalization_; - bool decl_only_class_equals_definition_; - bool use_enum_binary_only_equality_; -#ifdef WITH_DEBUG_SELF_COMPARISON - bool self_comparison_debug_on_; -#endif - - priv() - : canonicalization_is_done_(), - do_on_the_fly_canonicalization_(true), - decl_only_class_equals_definition_(false), - use_enum_binary_only_equality_(true) -#ifdef WITH_DEBUG_SELF_COMPARISON - , - self_comparison_debug_on_(false) -#endif - {} -};// end struct environment::priv /// Default constructor of the @ref environment type. environment::environment() @@ -13086,44 +13300,7 @@ global_scope::~global_scope() { } -// - -/// Definition of the private data of @ref type_base. -struct type_base::priv -{ - size_t size_in_bits; - size_t alignment_in_bits; - type_base_wptr canonical_type; - // The data member below holds the canonical type that is managed by - // the smart pointer referenced by the canonical_type data member - // above. We are storing this underlying (naked) pointer here, so - // that users can access it *fast*. Otherwise, accessing - // canonical_type above implies creating a shared_ptr, and that has - // been measured to be slow for some performance hot spots. - type_base* naked_canonical_type; - // Computing the representation of a type again and again can be - // costly. So we cache the internal and non-internal type - // representation strings here. - interned_string internal_cached_repr_; - interned_string cached_repr_; - - priv() - : size_in_bits(), - alignment_in_bits(), - naked_canonical_type() - {} - - priv(size_t s, - size_t a, - type_base_sptr c = type_base_sptr()) - : size_in_bits(s), - alignment_in_bits(a), - canonical_type(c), - naked_canonical_type(c.get()) - {} -}; // end struct type_base::priv - -static void +static bool maybe_propagate_canonical_type(const type_base& lhs_type, const type_base& rhs_type); @@ -18411,21 +18588,10 @@ equals(const function_type& l, const function_type& r, change_kind* k) { -#define RETURN(value) \ - do { \ - l.priv_->unmark_as_being_compared(l); \ - l.priv_->unmark_as_being_compared(r); \ - if (value == true) \ - maybe_propagate_canonical_type(l, r); \ - ABG_RETURN(value); \ - } while(0) - - if (l.priv_->comparison_started(l) - || l.priv_->comparison_started(r)) - return true; +#define RETURN(value) return return_comparison_result(l, r, value) - l.priv_->mark_as_being_compared(l); - l.priv_->mark_as_being_compared(r); + RETURN_TRUE_IF_COMPARISON_CYCLE_DETECTED(l, r); + mark_types_as_being_compared(l, r); bool result = true; @@ -19956,145 +20122,6 @@ function_decl::parameter::get_pretty_representation(bool internal, // // -struct class_or_union::priv -{ - typedef_decl_wptr naming_typedef_; - member_types member_types_; - data_members data_members_; - data_members non_static_data_members_; - member_functions member_functions_; - // A map that associates a linkage name to a member function. - string_mem_fn_sptr_map_type mem_fns_map_; - // A map that associates function signature strings to member - // function. - string_mem_fn_ptr_map_type signature_2_mem_fn_map_; - member_function_templates member_function_templates_; - member_class_templates member_class_templates_; - - priv() - {} - - priv(class_or_union::member_types& mbr_types, - class_or_union::data_members& data_mbrs, - class_or_union::member_functions& mbr_fns) - : member_types_(mbr_types), - data_members_(data_mbrs), - member_functions_(mbr_fns) - { - for (data_members::const_iterator i = data_members_.begin(); - i != data_members_.end(); - ++i) - if (!get_member_is_static(*i)) - non_static_data_members_.push_back(*i); - } - - /// Mark a class or union or union as being currently compared using - /// the class_or_union== operator. - /// - /// Note that is marking business is to avoid infinite loop when - /// comparing a class or union or union. If via the comparison of a - /// data member or a member function a recursive re-comparison of - /// the class or union is attempted, the marking business help to - /// detect that infinite loop possibility and avoid it. - /// - /// @param klass the class or union or union to mark as being - /// currently compared. - void - mark_as_being_compared(const class_or_union& klass) const - { - const environment* env = klass.get_environment(); - ABG_ASSERT(env); - env->priv_->classes_being_compared_.insert(&klass); - } - - /// Mark a class or union as being currently compared using the - /// class_or_union== operator. - /// - /// Note that is marking business is to avoid infinite loop when - /// comparing a class or union. If via the comparison of a data - /// member or a member function a recursive re-comparison of the - /// class or union is attempted, the marking business help to detect - /// that infinite loop possibility and avoid it. - /// - /// @param klass the class or union to mark as being currently - /// compared. - void - mark_as_being_compared(const class_or_union* klass) const - {mark_as_being_compared(*klass);} - - /// Mark a class or union as being currently compared using the - /// class_or_union== operator. - /// - /// Note that is marking business is to avoid infinite loop when - /// comparing a class or union. If via the comparison of a data - /// member or a member function a recursive re-comparison of the - /// class or union is attempted, the marking business help to detect - /// that infinite loop possibility and avoid it. - /// - /// @param klass the class or union to mark as being currently - /// compared. - void - mark_as_being_compared(const class_or_union_sptr& klass) const - {mark_as_being_compared(*klass);} - - /// If the instance of class_or_union has been previously marked as - /// being compared -- via an invocation of mark_as_being_compared() - /// this method unmarks it. Otherwise is has no effect. - /// - /// This method is not thread safe because it uses the static data - /// member classes_being_compared_. If you wish to use it in a - /// multi-threaded environment you should probably protect the - /// access to that static data member with a mutex or somesuch. - /// - /// @param klass the instance of class_or_union to unmark. - void - unmark_as_being_compared(const class_or_union& klass) const - { - const environment* env = klass.get_environment(); - ABG_ASSERT(env); - env->priv_->classes_being_compared_.erase(&klass); - } - - /// If the instance of class_or_union has been previously marked as - /// being compared -- via an invocation of mark_as_being_compared() - /// this method unmarks it. Otherwise is has no effect. - /// - /// @param klass the instance of class_or_union to unmark. - void - unmark_as_being_compared(const class_or_union* klass) const - { - if (klass) - return unmark_as_being_compared(*klass); - } - - /// Test if a given instance of class_or_union is being currently - /// compared. - /// - ///@param klass the class or union to test. - /// - /// @return true if @p klass is being compared, false otherwise. - bool - comparison_started(const class_or_union& klass) const - { - const environment* env = klass.get_environment(); - ABG_ASSERT(env); - return env->priv_->classes_being_compared_.count(&klass); - } - - /// Test if a given instance of class_or_union is being currently - /// compared. - /// - ///@param klass the class or union to test. - /// - /// @return true if @p klass is being compared, false otherwise. - bool - comparison_started(const class_or_union* klass) const - { - if (klass) - return comparison_started(*klass); - return false; - } -}; // end struct class_or_union::priv /// A Constructor for instances of @ref class_or_union /// @@ -21030,12 +21057,7 @@ class_or_union::operator==(const class_or_union& other) const bool equals(const class_or_union& l, const class_or_union& r, change_kind* k) { -#define RETURN(value) \ - do { \ - l.priv_->unmark_as_being_compared(l); \ - l.priv_->unmark_as_being_compared(r); \ - ABG_RETURN(value); \ - } while(0) +#define RETURN(value) return return_comparison_result(l, r, value); // if one of the classes is declaration-only, look through it to // get its definition. @@ -21116,12 +21138,9 @@ equals(const class_or_union& l, const class_or_union& r, change_kind* k) } } - if (l.priv_->comparison_started(l) - || l.priv_->comparison_started(r)) - return true; + RETURN_TRUE_IF_COMPARISON_CYCLE_DETECTED(l, r); - l.priv_->mark_as_being_compared(l); - l.priv_->mark_as_being_compared(r); + mark_types_as_being_compared(l, r); bool val = *def1 == *def2; if (!val) @@ -21142,12 +21161,9 @@ equals(const class_or_union& l, const class_or_union& r, change_kind* k) if (types_defined_same_linux_kernel_corpus_public(l, r)) return true; - if (l.priv_->comparison_started(l) - || l.priv_->comparison_started(r)) - return true; + RETURN_TRUE_IF_COMPARISON_CYCLE_DETECTED(l, r); - l.priv_->mark_as_being_compared(l); - l.priv_->mark_as_being_compared(r); + mark_types_as_being_compared(l, r); bool result = true; @@ -21329,39 +21345,11 @@ copy_member_function(const class_or_union_sptr& t, const method_decl* method) // -/// Test if two @ref class_decl or @ref function_type are already -/// being compared. -/// -/// @param lhs_type the first type that would be involved in a -/// potential comparison. -/// -/// @param rhs_type the second type that would involved in a potential -/// comparison. -/// -/// @return true iff @p lhs_type and @p rhs_type are being compared. -static bool -types_are_being_compared(const type_base& lhs_type, - const type_base& rhs_type) -{ - type_base *l = &const_cast(lhs_type); - type_base *r = &const_cast(rhs_type); - - if (class_or_union *l_cou = is_class_or_union_type(l)) - if (class_or_union *r_cou = is_class_or_union_type(r)) - return (l_cou->priv_->comparison_started(*l_cou) - || l_cou->priv_->comparison_started(*r_cou)); - - if (function_type *l_fn_type = is_function_type(l)) - if (function_type *r_fn_type = is_function_type(r)) - return (l_fn_type->priv_->comparison_started(*l_fn_type) - || l_fn_type->priv_->comparison_started(*r_fn_type)); - - return false; -} - /// @defgroup OnTheFlyCanonicalization On-the-fly Canonicalization /// @{ /// +/// This optimization is also known as "canonical type propagation". +/// /// During the canonicalization of a type T (which doesn't yet have a /// canonical type), T is compared structurally (member-wise) against /// a type C which already has a canonical type. The comparison @@ -21382,6 +21370,70 @@ types_are_being_compared(const type_base& lhs_type, /// For now this on-the-fly canonicalization only happens when /// comparing @ref class_decl and @ref function_type. /// +/// Note however that there is a case when a type is *NOT* eligible to +/// this canonical type propagation optimization. +/// +/// The reason why a type is deemed NON-eligible to the canonical type +/// propagation optimization is that it "depends" on recursively +/// present type. Let me explain. +/// +/// Suppose we have a type T that has sub-types named ST0 and ST1. +/// Suppose ST1 itself has a sub-type that is T itself. In this case, +/// we say that T is a recursive type, because it has T (itself) as +/// one of its sub-types: +/// +/// T +/// +-- ST0 +/// | +/// +-- ST1 +/// | + +/// | | +/// | +-- T +/// | +/// +-- ST2 +/// +/// ST1 is said to "depend" on T because it has T as a sub-type. But +/// because T is recursive, then ST1 is said to depend on a recursive +/// type. Notice however that ST0 does not depend on any recursive +/// type. +/// +/// Now suppose we are comparing T to a type T' that has the same +/// structure with sub-types ST0', ST1' and ST2'. During the +/// comparison of ST1 against ST1', their sub-type T is compared +/// against T'. Because T (resp. T') is a recursive type that is +/// already being compared, the comparison of T against T' (as a +/// subtypes of ST1 and ST1') returns true, meaning they are +/// considered equal. This is done so that we don't enter an infinite +/// recursion. +/// +/// That means ST1 is also deemed equal to ST1'. If we are in the +/// course of the canonicalization of T' and thus if T (as well as as +/// all of its sub-types) is already canonicalized, then the canonical +/// type propagation optimization will make us propagate the canonical +/// type of ST1 onto ST1'. So the canonical type of ST1' will be +/// equal to the canonical type of ST1 as a result of that +/// optmization. +/// +/// But then, later down the road, when ST2 is compared against ST2', +/// let's suppose that we find out that they are different. Meaning +/// that ST2 != ST2'. This means that T != T', i.e, the +/// canonicalization of T' failed for now. But most importantly, it +/// means that the propagation of the canonical type of ST1 to ST1' +/// must now be invalidated. Meaning, ST1' must now be considered as +/// not having any canonical type. +/// +/// In other words, during type canonicalization, if ST1' depends on a +/// recursive type T', its propagated canonical type must be +/// invalidated (set to nullptr) if T' appears to be different from T, +/// a.k.a, the canonicalization of T' temporarily failed. +/// +/// This means that any sub-type that depends on recursive types and +/// that has been the target of the canonical type propagation +/// optimization must be tracked. If the dependant recursive type +/// fails its canonicalization, then the sub-type being compared must +/// have its propagated canonical type cleared. In other words, its +/// propagated canonical type must be cancelled. +/// /// @} @@ -21392,22 +21444,17 @@ types_are_being_compared(const type_base& lhs_type, /// @param lhs_type the type which canonical type to propagate. /// /// @param rhs_type the type which canonical type to set. -static void +static bool maybe_propagate_canonical_type(const type_base& lhs_type, const type_base& rhs_type) { - if (const environment *env = lhs_type.get_environment()) if (env->do_on_the_fly_canonicalization()) if (type_base_sptr canonical_type = lhs_type.get_canonical_type()) - if (!rhs_type.get_canonical_type() - && !types_are_being_compared(lhs_type, rhs_type)) - { - const_cast(rhs_type).priv_->canonical_type = - canonical_type; - const_cast(rhs_type).priv_->naked_canonical_type = - canonical_type.get(); - } + if (!rhs_type.get_canonical_type()) + if (env->priv_->propagate_ct(lhs_type, rhs_type)) + return true; + return false; } // @@ -22573,12 +22620,41 @@ method_matches_at_least_one_in_vector(const method_decl_sptr& method, for (class_decl::member_functions::const_iterator i = fns.begin(); i != fns.end(); ++i) - if (methods_equal_modulo_elf_symbol(*i, method)) + // Note that the comparison must be done in this order: method == + // *i This is to keep the consistency of the comparison. It's + // important especially when doing type canonicalization. The + // already canonicalize type is the left operand, and the type + // being canonicalized is the right operand. This comes from the + // code in type_base::get_canonical_type_for(). + if (methods_equal_modulo_elf_symbol(method, *i)) return true; return false; } +/// Cancel the canonical type that was propagated. +/// +/// If we are in the process of comparing a type for the purpose of +/// canonicalization, and if that type has been the target of the +/// canonical type propagation optimization, then clear the propagated +/// canonical type. See @ref OnTheFlyCanonicalization for more about +/// the canonical type optimization +/// +/// @param t the type to consider. +static bool +maybe_cancel_propagated_canonical_type(const class_or_union& t) +{ + const environment* env = t.get_environment(); + if (env && env->do_on_the_fly_canonicalization()) + if (is_type(&t)->priv_->canonical_type_propagated()) + { + is_type(&t)->priv_->clear_propagated_canonical_type(); + env->priv_->remove_from_types_with_non_confirmed_propagated_ct(&t); + return true; + } + return false; +} + /// Compares two instances of @ref class_decl. /// /// If the two intances are different, set a bitfield to give some @@ -22608,9 +22684,8 @@ equals(const class_decl& l, const class_decl& r, change_kind* k) static_cast(r), k); - if (l.class_or_union::priv_->comparison_started(l) - || l.class_or_union::priv_->comparison_started(r)) - return true; + RETURN_TRUE_IF_COMPARISON_CYCLE_DETECTED(static_cast(l), + static_cast(r)); bool result = true; if (!equals(static_cast(l), @@ -22622,17 +22697,19 @@ equals(const class_decl& l, const class_decl& r, change_kind* k) return result; } - l.class_or_union::priv_->mark_as_being_compared(l); - l.class_or_union::priv_->mark_as_being_compared(r); + mark_types_as_being_compared(static_cast(l), + static_cast(r)); + +#define RETURN(value) \ + return return_comparison_result(static_cast(l), \ + static_cast(r), \ + value); -#define RETURN(value) \ - do { \ - l.class_or_union::priv_->unmark_as_being_compared(l); \ - l.class_or_union::priv_->unmark_as_being_compared(r); \ - if (value == true) \ - maybe_propagate_canonical_type(l, r); \ - ABG_RETURN(value); \ - } while(0) + // If comparing the class_or_union 'part' of the type led to + // canonical type propagation, then cancel that because it's too + // early to do that at this point. We still need to compare bases + // virtual members. + maybe_cancel_propagated_canonical_type(r); // Compare bases. if (l.get_base_specifiers().size() != r.get_base_specifiers().size()) @@ -23739,8 +23816,6 @@ equals(const union_decl& l, const union_decl& r, change_kind* k) bool result = equals(static_cast(l), static_cast(r), k); - if (result == true) - maybe_propagate_canonical_type(l, r); ABG_RETURN(result); } diff --git a/src/abg-reader.cc b/src/abg-reader.cc index 4d38d53d..63216029 100644 --- a/src/abg-reader.cc +++ b/src/abg-reader.cc @@ -5675,6 +5675,8 @@ build_type(read_context& ctxt, } #endif + if (t) + ctxt.maybe_canonicalize_type(t,/*force_delay=*/false ); return t; } diff --git a/tests/data/test-diff-dwarf/PR25058-liblttng-ctl-report-1.txt b/tests/data/test-diff-dwarf/PR25058-liblttng-ctl-report-1.txt index 506e5412..90248ab7 100644 --- a/tests/data/test-diff-dwarf/PR25058-liblttng-ctl-report-1.txt +++ b/tests/data/test-diff-dwarf/PR25058-liblttng-ctl-report-1.txt @@ -89,7 +89,12 @@ Variables changes summary: 0 Removed, 0 Changed, 3 Added variables parameter 1 of type 'lttng_action*' has sub-type changes: in pointed to type 'struct lttng_action': type size hasn't changed - 2 data member changes: + 3 data member changes: + type of 'action_validate_cb validate' changed: + underlying type 'bool (lttng_action*)*' changed: + in pointed to type 'function type bool (lttng_action*)': + parameter 1 of type 'lttng_action*' has sub-type changes: + pointed to type 'struct lttng_action' changed, as being reported type of 'action_serialize_cb serialize' changed: underlying type 'typedef ssize_t (lttng_action*, char*)*' changed: in pointed to type 'function type typedef ssize_t (lttng_action*, char*)': diff --git a/tests/data/test-diff-pkg/nss-3.23.0-1.0.fc23.x86_64-report-0.txt b/tests/data/test-diff-pkg/nss-3.23.0-1.0.fc23.x86_64-report-0.txt index f62905de..b09b0091 100644 --- a/tests/data/test-diff-pkg/nss-3.23.0-1.0.fc23.x86_64-report-0.txt +++ b/tests/data/test-diff-pkg/nss-3.23.0-1.0.fc23.x86_64-report-0.txt @@ -68,7 +68,7 @@ ================ end of changes of 'libssl3.so'=============== ================ changes of 'libsmime3.so'=============== - Functions changes summary: 0 Removed, 1 Changed (127 filtered out), 0 Added functions + Functions changes summary: 0 Removed, 1 Changed (146 filtered out), 0 Added functions Variables changes summary: 0 Removed, 0 Changed, 0 Added variable 1 function with some indirect sub-type change: @@ -82,12 +82,12 @@ type of 'NSSCMSContent content' changed: underlying type 'union NSSCMSContentUnion' at cmst.h:118:1 changed: type size hasn't changed - 1 data member changes (3 filtered): + 1 data member changes (4 filtered): type of 'NSSCMSEncryptedData* encryptedData' changed: in pointed to type 'typedef NSSCMSEncryptedData' at cmst.h:65:1: underlying type 'struct NSSCMSEncryptedDataStr' at cmst.h:468:1 changed: type size hasn't changed - 1 data member changes (1 filtered): + 1 data member changes (2 filtered): type of 'NSSCMSAttribute** unprotectedAttr' changed: in pointed to type 'NSSCMSAttribute*': in pointed to type 'typedef NSSCMSAttribute' at cmst.h:69:1: diff --git a/tests/data/test-diff-pkg/spice-server-0.12.4-19.el7.x86_64-0.12.8-1.el7.x86_64-report-2.txt b/tests/data/test-diff-pkg/spice-server-0.12.4-19.el7.x86_64-0.12.8-1.el7.x86_64-report-2.txt index 749146d2..5567354a 100644 --- a/tests/data/test-diff-pkg/spice-server-0.12.4-19.el7.x86_64-0.12.8-1.el7.x86_64-report-2.txt +++ b/tests/data/test-diff-pkg/spice-server-0.12.4-19.el7.x86_64-0.12.8-1.el7.x86_64-report-2.txt @@ -29,7 +29,87 @@ in pointed to type 'typedef QXLState' at spice-qxl.h:35:1: underlying type 'struct QXLState' at reds.h:93:1 changed: type size hasn't changed - 1 data member change: + 2 data member changes: + type of 'QXLInterface* qif' changed: + in pointed to type 'typedef QXLInterface' at spice-qxl.h:33:1: + underlying type 'struct QXLInterface' at spice.h:230:1 changed: + type size hasn't changed + 15 data member changes: + type of 'void (QXLInstance*, QXLWorker*)* attache_worker' changed: + in pointed to type 'function type void (QXLInstance*, QXLWorker*)': + parameter 1 of type 'QXLInstance*' has sub-type changes: + in pointed to type 'typedef QXLInstance' at spice-qxl.h:34:1: + underlying type 'struct QXLInstance' changed, as being reported + type of 'void (QXLInstance*, int)* set_compression_level' changed: + in pointed to type 'function type void (QXLInstance*, int)': + parameter 1 of type 'QXLInstance*' has sub-type changes: + in pointed to type 'typedef QXLInstance' at spice-qxl.h:34:1: + underlying type 'struct QXLInstance' changed, as being reported + type of 'void (QXLInstance*, typedef uint32_t)* set_mm_time' changed: + in pointed to type 'function type void (QXLInstance*, typedef uint32_t)': + parameter 1 of type 'QXLInstance*' has sub-type changes: + in pointed to type 'typedef QXLInstance' at spice-qxl.h:34:1: + underlying type 'struct QXLInstance' changed, as being reported + type of 'void (QXLInstance*, QXLDevInitInfo*)* get_init_info' changed: + in pointed to type 'function type void (QXLInstance*, QXLDevInitInfo*)': + parameter 1 of type 'QXLInstance*' has sub-type changes: + in pointed to type 'typedef QXLInstance' at spice-qxl.h:34:1: + underlying type 'struct QXLInstance' changed, as being reported + type of 'int (QXLInstance*, QXLCommandExt*)* get_command' changed: + in pointed to type 'function type int (QXLInstance*, QXLCommandExt*)': + parameter 1 of type 'QXLInstance*' has sub-type changes: + in pointed to type 'typedef QXLInstance' at spice-qxl.h:34:1: + underlying type 'struct QXLInstance' changed, as being reported + type of 'int (QXLInstance*)* req_cmd_notification' changed: + in pointed to type 'function type int (QXLInstance*)': + parameter 1 of type 'QXLInstance*' has sub-type changes: + in pointed to type 'typedef QXLInstance' at spice-qxl.h:34:1: + underlying type 'struct QXLInstance' changed, as being reported + type of 'void (QXLInstance*, struct QXLReleaseInfoExt)* release_resource' changed: + in pointed to type 'function type void (QXLInstance*, struct QXLReleaseInfoExt)': + parameter 1 of type 'QXLInstance*' has sub-type changes: + in pointed to type 'typedef QXLInstance' at spice-qxl.h:34:1: + underlying type 'struct QXLInstance' changed, as being reported + type of 'int (QXLInstance*, QXLCommandExt*)* get_cursor_command' changed: + in pointed to type 'function type int (QXLInstance*, QXLCommandExt*)': + parameter 1 of type 'QXLInstance*' has sub-type changes: + in pointed to type 'typedef QXLInstance' at spice-qxl.h:34:1: + underlying type 'struct QXLInstance' changed, as being reported + type of 'int (QXLInstance*)* req_cursor_notification' changed: + in pointed to type 'function type int (QXLInstance*)': + parameter 1 of type 'QXLInstance*' has sub-type changes: + in pointed to type 'typedef QXLInstance' at spice-qxl.h:34:1: + underlying type 'struct QXLInstance' changed, as being reported + type of 'void (QXLInstance*, typedef uint32_t)* notify_update' changed: + in pointed to type 'function type void (QXLInstance*, typedef uint32_t)': + parameter 1 of type 'QXLInstance*' has sub-type changes: + in pointed to type 'typedef QXLInstance' at spice-qxl.h:34:1: + underlying type 'struct QXLInstance' changed, as being reported + type of 'int (QXLInstance*)* flush_resources' changed: + in pointed to type 'function type int (QXLInstance*)': + parameter 1 of type 'QXLInstance*' has sub-type changes: + in pointed to type 'typedef QXLInstance' at spice-qxl.h:34:1: + underlying type 'struct QXLInstance' changed, as being reported + type of 'void (QXLInstance*, typedef uint64_t)* async_complete' changed: + in pointed to type 'function type void (QXLInstance*, typedef uint64_t)': + parameter 1 of type 'QXLInstance*' has sub-type changes: + in pointed to type 'typedef QXLInstance' at spice-qxl.h:34:1: + underlying type 'struct QXLInstance' changed, as being reported + type of 'void (QXLInstance*, typedef uint32_t, QXLRect*, typedef uint32_t)* update_area_complete' changed: + in pointed to type 'function type void (QXLInstance*, typedef uint32_t, QXLRect*, typedef uint32_t)': + parameter 1 of type 'QXLInstance*' has sub-type changes: + in pointed to type 'typedef QXLInstance' at spice-qxl.h:34:1: + underlying type 'struct QXLInstance' changed, as being reported + type of 'void (QXLInstance*, typedef uint8_t, uint8_t*)* set_client_capabilities' changed: + in pointed to type 'function type void (QXLInstance*, typedef uint8_t, uint8_t*)': + parameter 1 of type 'QXLInstance*' has sub-type changes: + in pointed to type 'typedef QXLInstance' at spice-qxl.h:34:1: + underlying type 'struct QXLInstance' changed, as being reported + type of 'int (QXLInstance*, VDAgentMonitorsConfig*)* client_monitors_config' changed: + in pointed to type 'function type int (QXLInstance*, VDAgentMonitorsConfig*)': + parameter 1 of type 'QXLInstance*' has sub-type changes: + in pointed to type 'typedef QXLInstance' at spice-qxl.h:34:1: + underlying type 'struct QXLInstance' changed, as being reported type of 'RedDispatcher* dispatcher' changed: in pointed to type 'typedef RedDispatcher' at red_worker.h:87:1: underlying type 'struct RedDispatcher' at red_dispatcher.c:53:1 changed: @@ -310,7 +390,9 @@ in pointed to type 'typedef QXLState' at spice-qxl.h:35:1: underlying type 'struct QXLState' at reds.h:93:1 changed: type size hasn't changed - 1 data member change: + 2 data member changes: + type of 'QXLInterface* qif' changed: + pointed to type 'typedef QXLInterface' changed at spice.h:102:1, as reported earlier type of 'RedDispatcher* dispatcher' changed: pointed to type 'struct RedDispatcher' changed, as being reported type of 'RedDispatcher* red_dispatcher' changed: diff --git a/tests/data/test-diff-pkg/tbb-4.1-9.20130314.fc22.x86_64--tbb-4.3-3.20141204.fc23.x86_64-report-0.txt b/tests/data/test-diff-pkg/tbb-4.1-9.20130314.fc22.x86_64--tbb-4.3-3.20141204.fc23.x86_64-report-0.txt index 54ee13d5..95e58609 100644 --- a/tests/data/test-diff-pkg/tbb-4.1-9.20130314.fc22.x86_64--tbb-4.3-3.20141204.fc23.x86_64-report-0.txt +++ b/tests/data/test-diff-pkg/tbb-4.1-9.20130314.fc22.x86_64--tbb-4.3-3.20141204.fc23.x86_64-report-0.txt @@ -1,5 +1,5 @@ ================ changes of 'libtbb.so.2'=============== - Functions changes summary: 0 Removed, 18 Changed (92 filtered out), 17 Added functions + Functions changes summary: 0 Removed, 19 Changed (92 filtered out), 17 Added functions Variables changes summary: 0 Removed, 0 Changed, 0 Added variable Function symbols changes summary: 0 Removed, 0 Added function symbol not referenced by debug info Variable symbols changes summary: 3 Removed, 0 Added variable symbols not referenced by debug info @@ -24,7 +24,7 @@ [A] 'method void tbb::internal::concurrent_queue_base_v8::move_content(tbb::internal::concurrent_queue_base_v8&)' {_ZN3tbb8internal24concurrent_queue_base_v812move_contentERS1_} [A] 'method void tbb::task_group_context::capture_fp_settings()' {_ZN3tbb18task_group_context19capture_fp_settingsEv} - 18 functions with some indirect sub-type change: + 19 functions with some indirect sub-type change: [C] 'method void tbb::filter::set_end_of_input(int)' at pipeline.cpp:700:1 has some indirect sub-type changes: implicit parameter 0 of type 'tbb::filter*' has sub-type changes: @@ -112,6 +112,43 @@ class tbb::internal::mutex_copy_deprecated_and_disabled at tbb_stddef.h:334:1 no data member change (1 filtered); + [C] 'method void tbb::internal::task_scheduler_observer_v3::observe(bool)' at observer_proxy.cpp:351:1 has some indirect sub-type changes: + implicit parameter 0 of type 'tbb::internal::task_scheduler_observer_v3*' has sub-type changes: + in pointed to type 'class tbb::internal::task_scheduler_observer_v3' at task_scheduler_observer.h:40:1: + type size hasn't changed + 1 member function deletion: + 'method virtual tbb::internal::task_scheduler_observer_v3::~task_scheduler_observer_v3(int)' at task_scheduler_observer.h:94:1 + 1 member function insertion: + 'method virtual tbb::internal::task_scheduler_observer_v3::~task_scheduler_observer_v3(int)' at task_scheduler_observer.h:86:1 + no member function changes (2 filtered); + 1 data member change: + type of 'tbb::internal::observer_proxy* my_proxy' changed: + in pointed to type 'class tbb::internal::observer_proxy' at observer_proxy.h:104:1: + type size hasn't changed + 1 data member changes (3 filtered): + type of 'tbb::internal::observer_list* my_list' changed: + in pointed to type 'class tbb::internal::observer_list' at observer_proxy.h:34:1: + type size hasn't changed + 1 data member changes (2 filtered): + type of 'tbb::internal::arena* my_arena' changed: + in pointed to type 'class tbb::internal::arena' at arena.h:160:1: + type size hasn't changed + 1 base class deletion: + struct tbb::internal::padded at tbb_stddef.h:261:1 + 1 base class insertion: + struct tbb::internal::padded at tbb_stddef.h:251:1 + 1 data member change: + type of 'tbb::internal::arena_slot my_slots[1]' changed: + array element type 'struct tbb::internal::arena_slot' changed: + type size hasn't changed + 2 base class deletions: + struct tbb::internal::padded at tbb_stddef.h:261:1 + struct tbb::internal::padded at tbb_stddef.h:261:1 + 2 base class insertions: + struct tbb::internal::padded at tbb_stddef.h:251:1 + struct tbb::internal::padded at tbb_stddef.h:251:1 + type size hasn't changed + [C] 'function void tbb::internal::throw_exception_v4(tbb::internal::exception_id)' at tbb_misc.cpp:119:1 has some indirect sub-type changes: parameter 1 of type 'enum tbb::internal::exception_id' has sub-type changes: type size hasn't changed diff --git a/tests/data/test-diff-pkg/tbb-4.1-9.20130314.fc22.x86_64--tbb-4.3-3.20141204.fc23.x86_64-report-1.txt b/tests/data/test-diff-pkg/tbb-4.1-9.20130314.fc22.x86_64--tbb-4.3-3.20141204.fc23.x86_64-report-1.txt index b76b389e..dc0afab9 100644 --- a/tests/data/test-diff-pkg/tbb-4.1-9.20130314.fc22.x86_64--tbb-4.3-3.20141204.fc23.x86_64-report-1.txt +++ b/tests/data/test-diff-pkg/tbb-4.1-9.20130314.fc22.x86_64--tbb-4.3-3.20141204.fc23.x86_64-report-1.txt @@ -1,5 +1,5 @@ ================ changes of 'libtbb.so.2'=============== - Functions changes summary: 0 Removed, 16 Changed (94 filtered out), 17 Added functions + Functions changes summary: 0 Removed, 16 Changed (95 filtered out), 17 Added functions Variables changes summary: 0 Removed, 0 Changed, 0 Added variable Function symbols changes summary: 0 Removed, 0 Added function symbol not referenced by debug info Variable symbols changes summary: 3 Removed, 0 Added variable symbols not referenced by debug info diff --git a/tests/data/test-read-dwarf/test-libaaudio.so.abi b/tests/data/test-read-dwarf/test-libaaudio.so.abi index dd98f75d..8a71b8d1 100644 --- a/tests/data/test-read-dwarf/test-libaaudio.so.abi +++ b/tests/data/test-read-dwarf/test-libaaudio.so.abi @@ -99,7 +99,7 @@ - + @@ -107,7 +107,7 @@ - + @@ -119,7 +119,7 @@ - + @@ -127,8 +127,8 @@ - - + + @@ -136,7 +136,7 @@ - + From patchwork Wed Aug 11 16:09:58 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dodji Seketeli X-Patchwork-Id: 44640 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 6D7C93985440 for ; Wed, 11 Aug 2021 16:10:19 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 6D7C93985440 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1628698219; bh=MsTnqzToYnB+ui7FzBgrfB2vgaqvxPCoTdkNW5I7Mx8=; h=To:Subject:Date:List-Id:List-Unsubscribe:List-Archive:List-Help: List-Subscribe:From:Reply-To:From; b=RXKKh2l5t9DyNnomQOeysMIV8vzKe5WU4ymKuPJzFes9zpHRbjKFoInKbqi8wOY2L zKvyblBkqQZKWFDY/K3WvWjYMRwC72WP4ZcLxIVdIlTIxkfHed2z6AyinVmRcutwEd eCDg0Ex5xjxMlAPwqLeNpck88K7e6KFYkfJakPDU= X-Original-To: libabigail@sourceware.org Delivered-To: libabigail@sourceware.org Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTP id 0363C385841C for ; Wed, 11 Aug 2021 16:10:05 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 0363C385841C Received: from mail-wr1-f70.google.com (mail-wr1-f70.google.com [209.85.221.70]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-126-FLk5ym4AOUu8TZvIW_vMxQ-1; Wed, 11 Aug 2021 12:10:04 -0400 X-MC-Unique: FLk5ym4AOUu8TZvIW_vMxQ-1 Received: by mail-wr1-f70.google.com with SMTP id m5-20020a5d6a050000b0290154e83dce73so888475wru.19 for ; Wed, 11 Aug 2021 09:10:04 -0700 (PDT) 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=MsTnqzToYnB+ui7FzBgrfB2vgaqvxPCoTdkNW5I7Mx8=; b=DoBLpJzw+VnYhnUdMHlxmmncGiHgeCW8Q8rC/GuXbpoWb05ZpgdI+OjZI8+CBZGRka XaFLUyY2CrP6v+RB8gazuSRRsQGUd8qKdEUPb9SWczMmSFcH1bomShVYXAROUfD15NAe 2sC/lDt3FIoosn8tG/uylADAG5y2r8ixyo2Jl7aiVCaw7VXE2bg7LrSfsYBEUjRtoMWE YQ29wJ3xq9T/SdelpMmXlNOJcyLqe6qpd3VXwIUdZ9llpwCioVWIZFJnVOhNClEaeZTc EdgoXXevogrfzI0qG/Jv30u9GPIj0CB4rs5DVNsNKN3WWxbKfol0/vcnprNs8SchwpFK cFCQ== X-Gm-Message-State: AOAM533zbHxG10LYDk3XWZWTNnxwdDmWuX6vpK252UDRuyhdJPlkkc72 VAS80PXKTngRUjSAyMn8GPYzKODzxxbzzU/T2pBEH9u9zWAe1xQ9PrmwS2mFD9HxWJp9qj/w9N1 LLfD/Ug1tSU/Ur6CUA3pogbHmw3i25XCifyJMAN46MTZoL1mj51Udw5R53R0GwUxKfjc6 X-Received: by 2002:a5d:5550:: with SMTP id g16mr38211182wrw.336.1628698202739; Wed, 11 Aug 2021 09:10:02 -0700 (PDT) X-Google-Smtp-Source: ABdhPJzY+sAPzUkjf+gyBJnUNRdC7M3KZg5bwiMprNYdrcD2LkGvShgY45zaQZXvYUm3QbVf4g98hw== X-Received: by 2002:a5d:5550:: with SMTP id g16mr38211127wrw.336.1628698202358; Wed, 11 Aug 2021 09:10:02 -0700 (PDT) Received: from localhost ([88.120.130.27]) by smtp.gmail.com with ESMTPSA id 9sm24705564wmf.34.2021.08.11.09.10.01 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 11 Aug 2021 09:10:01 -0700 (PDT) Received: by localhost (Postfix, from userid 1000) id BCB355800FB; Wed, 11 Aug 2021 18:09:58 +0200 (CEST) To: libabigail@sourceware.org Subject: [PATCH 6/8, applied] Bug 27236 - Allow updating classes from abixml Organization: Red Hat / France X-Operating-System: Fedora 35 X-URL: http://www.redhat.com Date: Wed, 11 Aug 2021 18:09:58 +0200 Message-ID: <87lf58c695.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 X-Spam-Status: No, score=-12.3 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_NONE, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) 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-Patchwork-Original-From: Dodji Seketeli via Libabigail From: Dodji Seketeli Reply-To: Dodji Seketeli Errors-To: libabigail-bounces+patchwork=sourceware.org@sourceware.org Sender: "Libabigail" Hello, Some classes can be defined piece-wise, in some rare cases in the abixml. build_class_decl is currently preventing that to happen, leading to some spurious self comparison errors. Fixed thus. * src/abg-reader.cc (build_class_decl): Keep going when the class has already been built. The rest of the code knows how to add new stuff. * tests/data/test-abidiff/test-PR18791-report0.txt: Adjust. Signed-off-by: Dodji Seketeli Applied to master. --- src/abg-reader.cc | 7 +- .../test-abidiff/test-PR18791-report0.txt | 131 +++++++++++++++++- 2 files changed, 132 insertions(+), 6 deletions(-) diff --git a/src/abg-reader.cc b/src/abg-reader.cc index 63216029..b115304f 100644 --- a/src/abg-reader.cc +++ b/src/abg-reader.cc @@ -4535,14 +4535,13 @@ build_class_decl(read_context& ctxt, ABG_ASSERT(!id.empty()); + class_decl_sptr previous_definition, previous_declaration; if (type_base_sptr t = ctxt.get_type_decl(id)) { - class_decl_sptr result = is_class_type(t); - ABG_ASSERT(result); - return result; + previous_definition = is_class_type(t); + ABG_ASSERT(previous_definition); } - class_decl_sptr previous_definition, previous_declaration; const vector *types_ptr = 0; if (!is_anonymous && !previous_definition) types_ptr = ctxt.get_all_type_decls(id); diff --git a/tests/data/test-abidiff/test-PR18791-report0.txt b/tests/data/test-abidiff/test-PR18791-report0.txt index 5de96dc6..6c236f92 100644 --- a/tests/data/test-abidiff/test-PR18791-report0.txt +++ b/tests/data/test-abidiff/test-PR18791-report0.txt @@ -1,4 +1,4 @@ -Functions changes summary: 1 Removed, 35 Changed, 1 Added functions +Functions changes summary: 1 Removed, 60 Changed, 1 Added functions Variables changes summary: 0 Removed, 0 Changed, 0 Added variable 1 Removed function: @@ -9,7 +9,7 @@ Variables changes summary: 0 Removed, 0 Changed, 0 Added variable [A] 'method void std::__cxx11::_List_base >::_M_clear()' -35 functions with some indirect sub-type change: +60 functions with some indirect sub-type change: [C] 'method bool sigc::connection::block(bool)' has some indirect sub-type changes: implicit parameter 0 of type 'sigc::connection*' has sub-type changes: @@ -193,6 +193,26 @@ Variables changes summary: 0 Removed, 0 Changed, 0 Added variable implicit parameter 0 of type 'sigc::internal::signal_impl*' has sub-type changes: pointed to type 'struct sigc::internal::signal_impl' changed, as reported earlier + [C] 'method void sigc::internal::slot_rep::disconnect()' has some indirect sub-type changes: + implicit parameter 0 of type 'sigc::internal::slot_rep*' has sub-type changes: + pointed to type 'struct sigc::internal::slot_rep' changed, as reported earlier + + [C] 'method void sigc::internal::trackable_callback_list::add_callback(void*)' has some indirect sub-type changes: + implicit parameter 0 of type 'sigc::internal::trackable_callback_list*' has sub-type changes: + pointed to type 'struct sigc::internal::trackable_callback_list' changed, as reported earlier + + [C] 'method void sigc::internal::trackable_callback_list::clear()' has some indirect sub-type changes: + implicit parameter 0 of type 'sigc::internal::trackable_callback_list*' has sub-type changes: + pointed to type 'struct sigc::internal::trackable_callback_list' changed, as reported earlier + + [C] 'method void sigc::internal::trackable_callback_list::remove_callback(void*)' has some indirect sub-type changes: + implicit parameter 0 of type 'sigc::internal::trackable_callback_list*' has sub-type changes: + pointed to type 'struct sigc::internal::trackable_callback_list' changed, as reported earlier + + [C] 'method sigc::internal::trackable_callback_list::~trackable_callback_list(int)' has some indirect sub-type changes: + implicit parameter 0 of type 'sigc::internal::trackable_callback_list*' has sub-type changes: + pointed to type 'struct sigc::internal::trackable_callback_list' changed, as reported earlier + [C] 'method void sigc::signal_base::block(bool)' has some indirect sub-type changes: implicit parameter 0 of type 'sigc::signal_base*' has sub-type changes: in pointed to type 'struct sigc::signal_base': @@ -281,3 +301,110 @@ Variables changes summary: 0 Removed, 0 Changed, 0 Added variable implicit parameter 0 of type 'sigc::signal_base*' has sub-type changes: pointed to type 'struct sigc::signal_base' changed, as reported earlier + [C] 'method void sigc::slot_base::add_destroy_notify_callback(void*)' has some indirect sub-type changes: + implicit parameter 0 of type 'const sigc::slot_base*' has sub-type changes: + in pointed to type 'const sigc::slot_base': + unqualified underlying type 'class sigc::slot_base' changed, as reported earlier + + [C] 'method bool sigc::slot_base::block(bool)' has some indirect sub-type changes: + implicit parameter 0 of type 'sigc::slot_base*' has sub-type changes: + pointed to type 'class sigc::slot_base' changed, as reported earlier + + [C] 'method void sigc::slot_base::disconnect()' has some indirect sub-type changes: + implicit parameter 0 of type 'sigc::slot_base*' has sub-type changes: + pointed to type 'class sigc::slot_base' changed, as reported earlier + + [C] 'method bool sigc::slot_base::operator bool()' has some indirect sub-type changes: + implicit parameter 0 of type 'const sigc::slot_base*' has sub-type changes: + in pointed to type 'const sigc::slot_base': + unqualified underlying type 'class sigc::slot_base' changed, as reported earlier + + [C] 'method sigc::slot_base& sigc::slot_base::operator=(const sigc::slot_base&)' has some indirect sub-type changes: + return type changed: + referenced type 'class sigc::slot_base' changed, as reported earlier + implicit parameter 0 of type 'sigc::slot_base*' has sub-type changes: + pointed to type 'class sigc::slot_base' changed, as reported earlier + parameter 1 of type 'const sigc::slot_base&' has sub-type changes: + in referenced type 'const sigc::slot_base': + unqualified underlying type 'class sigc::slot_base' changed, as reported earlier + + [C] 'method void sigc::slot_base::remove_destroy_notify_callback(void*)' has some indirect sub-type changes: + implicit parameter 0 of type 'const sigc::slot_base*' has sub-type changes: + in pointed to type 'const sigc::slot_base': + unqualified underlying type 'class sigc::slot_base' changed, as reported earlier + + [C] 'method void sigc::slot_base::set_parent(void*)' has some indirect sub-type changes: + implicit parameter 0 of type 'const sigc::slot_base*' has sub-type changes: + in pointed to type 'const sigc::slot_base': + unqualified underlying type 'class sigc::slot_base' changed, as reported earlier + + [C] 'method sigc::slot_base::slot_base(sigc::slot_base::rep_type*)' has some indirect sub-type changes: + implicit parameter 0 of type 'sigc::slot_base*' has sub-type changes: + pointed to type 'class sigc::slot_base' changed, as reported earlier + parameter 1 of type 'sigc::slot_base::rep_type*' has sub-type changes: + pointed to type 'typedef sigc::slot_base::rep_type' changed, as reported earlier + + [C] 'method sigc::slot_base::slot_base()' has some indirect sub-type changes: + implicit parameter 0 of type 'sigc::slot_base*' has sub-type changes: + pointed to type 'class sigc::slot_base' changed, as reported earlier + + [C] 'method sigc::slot_base::slot_base(const sigc::slot_base&)' has some indirect sub-type changes: + implicit parameter 0 of type 'sigc::slot_base*' has sub-type changes: + pointed to type 'class sigc::slot_base' changed, as reported earlier + parameter 1 of type 'const sigc::slot_base&' has sub-type changes: + in referenced type 'const sigc::slot_base': + unqualified underlying type 'class sigc::slot_base' changed, as reported earlier + + [C] 'method bool sigc::slot_base::unblock()' has some indirect sub-type changes: + implicit parameter 0 of type 'sigc::slot_base*' has sub-type changes: + pointed to type 'class sigc::slot_base' changed, as reported earlier + + [C] 'method sigc::slot_base::~slot_base(int)' has some indirect sub-type changes: + implicit parameter 0 of type 'sigc::slot_base*' has sub-type changes: + pointed to type 'class sigc::slot_base' changed, as reported earlier + + [C] 'method void sigc::trackable::add_destroy_notify_callback(void*)' has some indirect sub-type changes: + implicit parameter 0 of type 'const sigc::trackable*' has sub-type changes: + in pointed to type 'const sigc::trackable': + unqualified underlying type 'struct sigc::trackable' changed, as reported earlier + + [C] 'method sigc::internal::trackable_callback_list* sigc::trackable::callback_list()' has some indirect sub-type changes: + return type changed: + pointed to type 'struct sigc::internal::trackable_callback_list' changed, as reported earlier + implicit parameter 0 of type 'const sigc::trackable*' has sub-type changes: + in pointed to type 'const sigc::trackable': + unqualified underlying type 'struct sigc::trackable' changed, as reported earlier + + [C] 'method void sigc::trackable::notify_callbacks()' has some indirect sub-type changes: + implicit parameter 0 of type 'sigc::trackable*' has sub-type changes: + pointed to type 'struct sigc::trackable' changed, as reported earlier + + [C] 'method sigc::trackable& sigc::trackable::operator=(const sigc::trackable&)' has some indirect sub-type changes: + return type changed: + referenced type 'struct sigc::trackable' changed, as reported earlier + implicit parameter 0 of type 'sigc::trackable*' has sub-type changes: + pointed to type 'struct sigc::trackable' changed, as reported earlier + parameter 1 of type 'const sigc::trackable&' has sub-type changes: + in referenced type 'const sigc::trackable': + unqualified underlying type 'struct sigc::trackable' changed, as reported earlier + + [C] 'method void sigc::trackable::remove_destroy_notify_callback(void*)' has some indirect sub-type changes: + implicit parameter 0 of type 'const sigc::trackable*' has sub-type changes: + in pointed to type 'const sigc::trackable': + unqualified underlying type 'struct sigc::trackable' changed, as reported earlier + + [C] 'method sigc::trackable::trackable(const sigc::trackable&)' has some indirect sub-type changes: + implicit parameter 0 of type 'sigc::trackable*' has sub-type changes: + pointed to type 'struct sigc::trackable' changed, as reported earlier + parameter 1 of type 'const sigc::trackable&' has sub-type changes: + in referenced type 'const sigc::trackable': + unqualified underlying type 'struct sigc::trackable' changed, as reported earlier + + [C] 'method sigc::trackable::trackable()' has some indirect sub-type changes: + implicit parameter 0 of type 'sigc::trackable*' has sub-type changes: + pointed to type 'struct sigc::trackable' changed, as reported earlier + + [C] 'method sigc::trackable::~trackable(int)' has some indirect sub-type changes: + implicit parameter 0 of type 'sigc::trackable*' has sub-type changes: + pointed to type 'struct sigc::trackable' changed, as reported earlier + From patchwork Wed Aug 11 16:10:33 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dodji Seketeli X-Patchwork-Id: 44641 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 759C43982404 for ; Wed, 11 Aug 2021 16:10:53 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 759C43982404 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1628698253; bh=C4Yy5oIjFFTx3NdSS8WF1wkXOdycU791lesX1km/wRM=; h=To:Subject:Date:List-Id:List-Unsubscribe:List-Archive:List-Help: List-Subscribe:From:Reply-To:From; b=c1VFRGmYNPEpS3YyBZ+tMK1mE5t+BkPrJAaZL/nSYxm1kPuiB3fPLAim8gVGPMv+0 KEcXZw5EJc+ECKG/anEpYQGljduEMHBzdaArB4qxsWKY/z61SSLvfcxPRKOMkd2Pei YDJU2s3HDI8VHkzUiB7u4j9eCodbywd6DER/5FM0= X-Original-To: libabigail@sourceware.org Delivered-To: libabigail@sourceware.org Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTP id 778AC3982413 for ; Wed, 11 Aug 2021 16:10:40 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 778AC3982413 Received: from mail-wm1-f71.google.com (mail-wm1-f71.google.com [209.85.128.71]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-152-ZhacxstrM0WICkKADSgrfA-1; Wed, 11 Aug 2021 12:10:39 -0400 X-MC-Unique: ZhacxstrM0WICkKADSgrfA-1 Received: by mail-wm1-f71.google.com with SMTP id f25-20020a1c6a190000b029024fa863f6b0so2268573wmc.1 for ; Wed, 11 Aug 2021 09:10:38 -0700 (PDT) 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=C4Yy5oIjFFTx3NdSS8WF1wkXOdycU791lesX1km/wRM=; b=ZYA+GPnPkavCkXc8FC10u9LsGHFQxBxbLlqbhNMIZTuJ/Tr6OnvZftScAgPuEx5ZXt sKJrZ4whWmYeyTsyNQPC+k/N1OgUTzCuGO6chToMym9PeQH16fgEDu3bk4nfCUMstcET rOVZjTj4h+3ShpcVvg24x4OOrUVocDha6xE2tQsTbdiN8xa8gOmtjLAoclKpxUlY8PcY xq7FSgcCQg9CRGjZVwDg8Ceej+BVbnZvbWsH/bnlqFxfkH5X+M1fH4n8ZWbUHL2lZ57h Pxg0/8+HdfnUp5zmCNQBpo/ZrQA40ICKWb1Gdhl32BQKX6+As0Ylb/pWAvBh+Hz8uI4s xvWg== X-Gm-Message-State: AOAM530wIpSNELSeo30LPyM/r5gDW6vII9zjhCw6Z6kMn8PD4ynEM8eF yTy4LeqTsP6uuNreQioTqlLT717pHLns/LgNyw00dxXMldXU/S1xsF7ofV/sN7O00ahrzAbMFkM GPo3eyHiD3F6PrX7YdVQn312ujE0ZptvHo7rO4mdxDw5JZ74hTftFXflbbLdJNvqRLQyT X-Received: by 2002:a7b:c083:: with SMTP id r3mr29400567wmh.65.1628698237501; Wed, 11 Aug 2021 09:10:37 -0700 (PDT) X-Google-Smtp-Source: ABdhPJy4BO08RUbIuLNAM7Zp7M//PLhuP/DztqZofPHFcdR1TdUiunExnfQn7RM1H82hGIaA1Szsxg== X-Received: by 2002:a7b:c083:: with SMTP id r3mr29400538wmh.65.1628698237221; Wed, 11 Aug 2021 09:10:37 -0700 (PDT) Received: from localhost ([88.120.130.27]) by smtp.gmail.com with ESMTPSA id p2sm27820805wrr.21.2021.08.11.09.10.36 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 11 Aug 2021 09:10:36 -0700 (PDT) Received: by localhost (Postfix, from userid 1000) id D29DE5800FB; Wed, 11 Aug 2021 18:10:33 +0200 (CEST) To: libabigail@sourceware.org Subject: [PATCH 7/8, applied] Bug 27236 - Don't forget to emit some referenced types Organization: Red Hat / France X-Operating-System: Fedora 35 X-URL: http://www.redhat.com Date: Wed, 11 Aug 2021 18:10:33 +0200 Message-ID: <87h7fwc686.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 X-Spam-Status: No, score=-12.3 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_NONE, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) 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-Patchwork-Original-From: Dodji Seketeli via Libabigail From: Dodji Seketeli Reply-To: Dodji Seketeli Errors-To: libabigail-bounces+patchwork=sourceware.org@sourceware.org Sender: "Libabigail" Hello, Since we arranged to only emit referenced types in translation units where they belong, it appears that in some cases we forget to emit some referenced types. This is because some referenced types might belong to a translation unit that is *already* emitted by the time we detect that a type is referenced. To fix this correctly, we should probably have a pass that walks the corpus to detect referenced types, so that we have their set even before we start emitting translation units. But for now, the patch just detects when we are emitting the last translation unit. In that case all the non-emitted referenced types are emitted. It doesn't seem to be an issue if those don't belong to that translation unit, compared to their original (from the DWARF) type. * include/abg-writer.h (write_translation_unit): Add a new parameter that says if we are emitting the last TU. * src/abg-writer.cc (write_translation_unit::{type_is_emitted, decl_only_type_is_emitted}): Constify these methods. (write_context::has_non_emitted_referenced_types): Define new member function using the const methods above. (write_translation_unit): When emitting the last TU, emit all the referenced types. (write_corpus): Set signal when emitting the last translation unit. Signed-off-by: Dodji Seketeli Applied to master. --- include/abg-writer.h | 3 +- src/abg-writer.cc | 103 ++++++++++++++++++++++++++++++++----------- 2 files changed, 79 insertions(+), 27 deletions(-) diff --git a/include/abg-writer.h b/include/abg-writer.h index 70d118a5..b6bdcb21 100644 --- a/include/abg-writer.h +++ b/include/abg-writer.h @@ -100,7 +100,8 @@ set_ostream(write_context& ctxt, ostream& os); bool write_translation_unit(write_context& ctxt, const translation_unit& tu, - const unsigned indent); + const unsigned indent, + bool last = true); bool write_corpus_to_archive(const corpus& corp, diff --git a/src/abg-writer.cc b/src/abg-writer.cc index ee24c17d..bf460eed 100644 --- a/src/abg-writer.cc +++ b/src/abg-writer.cc @@ -505,6 +505,30 @@ public: get_referenced_non_canonical_types() const {return m_referenced_non_canonical_types_set;} + /// Test if there are non emitted referenced types. + /// + /// @return true iff there are non emitted referenced types. + bool + has_non_emitted_referenced_types() const + { + for (const auto t : get_referenced_types()) + if (!type_is_emitted(t) + && !decl_only_type_is_emitted(t)) + return false; + + for (const auto t : get_referenced_non_canonical_types()) + if (!type_is_emitted(t) + && !decl_only_type_is_emitted(t)) + return false; + + for (const auto t : get_referenced_non_canonical_types()) + if (!type_is_emitted(t) + && !decl_only_type_is_emitted(t)) + return false; + + return true; + } + /// Record a given type as being referenced by a pointer, a /// reference or a typedef type that is being emitted to the XML /// output. @@ -708,7 +732,7 @@ public: /// @return true if the type has already been emitted, false /// otherwise. bool - type_is_emitted(const type_base *t) + type_is_emitted(const type_base *t) const { return m_emitted_type_set.find(t) != m_emitted_type_set.end(); } @@ -720,7 +744,7 @@ public: /// @return true if the type has already been emitted, false /// otherwise. bool - type_is_emitted(const type_base_sptr& t) + type_is_emitted(const type_base_sptr& t) const {return type_is_emitted(t.get());} /// Test if the name of a given decl has been written out to the XML @@ -783,7 +807,7 @@ public: /// @return true iff the declaration-only class @p t has been /// emitted. bool - decl_only_type_is_emitted(const type_base* t) + decl_only_type_is_emitted(const type_base* t) const { type_ptr_set_type::const_iterator i = m_emitted_decl_only_set.find(t); if (i == m_emitted_decl_only_set.end()) @@ -798,7 +822,7 @@ public: /// @return true iff the declaration-only class @p t has been /// emitted. bool - decl_only_type_is_emitted(const type_base_sptr& t) + decl_only_type_is_emitted(const type_base_sptr& t) const {return decl_only_type_is_emitted(t.get());} /// Record a declaration as emitted in the abixml output. @@ -2228,12 +2252,31 @@ write_canonical_types_of_scope(const scope_decl &scope, /// @param indent how many indentation spaces to use during the /// serialization. /// +/// @param is_last If true, it means the TU to emit is the last one of +/// the corpus. If this is the case, all the remaining referenced +/// types that were not emitted are going to be emitted here, +/// irrespective of if they belong to this TU or not. This is quite a +/// hack. Ideally, we should have a pass that walks all the TUs, +/// detect their non-emitted referenced types, before hand. Then, +/// when we start emitting the TUs, we know for each TU which +/// non-emitted referenced type should be emitted. As we don't yet +/// have such a pass, we do our best for now. +/// /// @return true upon successful completion, false otherwise. bool -write_translation_unit(write_context& ctxt, - const translation_unit& tu, - const unsigned indent) +write_translation_unit(write_context& ctxt, + const translation_unit& tu, + const unsigned indent, + bool is_last) { + if (tu.is_empty() && !is_last) + return false; + + if (is_last + && tu.is_empty() + && ctxt.has_non_emitted_referenced_types()) + return false; + ostream& o = ctxt.get_ostream(); const config& c = ctxt.get_config(); @@ -2259,7 +2302,7 @@ write_translation_unit(write_context& ctxt, << translation_unit_language_to_string(tu.get_language()) <<"'"; - if (tu.is_empty()) + if (tu.is_empty() && !is_last) { o << "/>\n"; return true; @@ -2304,18 +2347,20 @@ write_translation_unit(write_context& ctxt, // So this map of type -> string is to contain the referenced types // we need to emit. type_ptr_set_type referenced_types_to_emit; - for (type_ptr_set_type::const_iterator i = ctxt.get_referenced_types().begin(); i != ctxt.get_referenced_types().end(); ++i) - if (!ctxt.type_is_emitted(*i) - && !ctxt.decl_only_type_is_emitted(*i) - // Ensure that the referenced type is emitted in the - // translation that it belongs to. - && ((*i)->get_translation_unit()->get_absolute_path() - == tu.get_absolute_path())) + { + if (!ctxt.type_is_emitted(*i) + && !ctxt.decl_only_type_is_emitted(*i) + // Ensure that the referenced type is emitted in the + // translation that it belongs to. + && (is_last + || ((*i)->get_translation_unit()->get_absolute_path() + == tu.get_absolute_path()))) referenced_types_to_emit.insert(*i); + } for (fn_type_ptr_set_type::const_iterator i = ctxt.get_referenced_function_types().begin(); @@ -2325,8 +2370,9 @@ write_translation_unit(write_context& ctxt, && !ctxt.decl_only_type_is_emitted(*i) // Ensure that the referenced type is emitted in the // translation that it belongs to. - && ((*i)->get_translation_unit()->get_absolute_path() - == tu.get_absolute_path())) + && (is_last + || ((*i)->get_translation_unit()->get_absolute_path() + == tu.get_absolute_path()))) // A referenced type that was not emitted at all must be // emitted now. referenced_types_to_emit.insert(*i); @@ -2339,8 +2385,9 @@ write_translation_unit(write_context& ctxt, && !ctxt.decl_only_type_is_emitted(*i) // Ensure that the referenced type is emitted in the // translation that it belongs to. - && ((*i)->get_translation_unit()->get_absolute_path() - == tu.get_absolute_path())) + && (is_last + || ((*i)->get_translation_unit()->get_absolute_path() + == tu.get_absolute_path()))) // A referenced type that was not emitted at all must be // emitted now. referenced_types_to_emit.insert(*i); @@ -2399,8 +2446,9 @@ write_translation_unit(write_context& ctxt, && !ctxt.decl_only_type_is_emitted(*i) // Ensure that the referenced type is emitted in the // translation that it belongs to. - && ((*i)->get_translation_unit()->get_absolute_path() - == tu.get_absolute_path())) + && (is_last + || ((*i)->get_translation_unit()->get_absolute_path() + == tu.get_absolute_path()))) // A referenced type that was not emitted at all must be // emitted now. referenced_types_to_emit.insert(*i); @@ -2413,8 +2461,9 @@ write_translation_unit(write_context& ctxt, && !ctxt.decl_only_type_is_emitted(*i) // Ensure that the referenced type is emitted in the // translation that it belongs to. - && ((*i)->get_translation_unit()->get_absolute_path() - == tu.get_absolute_path())) + && (is_last + || ((*i)->get_translation_unit()->get_absolute_path() + == tu.get_absolute_path()))) // A referenced type that was not emitted at all must be // emitted now. referenced_types_to_emit.insert(*i); @@ -4453,14 +4502,16 @@ write_corpus(write_context& ctxt, } // Now write the translation units. + unsigned nb_tus = corpus->get_translation_units().size(), n = 0; for (translation_units::const_iterator i = corpus->get_translation_units().begin(); i != corpus->get_translation_units().end(); - ++i) + ++i, ++n) { translation_unit& tu = **i; - if (!tu.is_empty()) - write_translation_unit(ctxt, tu, get_indent_to_level(ctxt, indent, 1)); + write_translation_unit(ctxt, tu, + get_indent_to_level(ctxt, indent, 1), + n == nb_tus - 1); } do_indent_to_level(ctxt, indent, 0);