From patchwork Thu Sep 7 14:05:06 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dodji Seketeli X-Patchwork-Id: 75456 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 D2BEF3858C2B for ; Thu, 7 Sep 2023 14:05:32 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org D2BEF3858C2B DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1694095532; bh=3uFJtZ5dl3r3CVXoy6syES+qcsStZE2umbEdwfV5zkg=; h=To:Cc:Subject:References:Date:In-Reply-To:List-Id: List-Unsubscribe:List-Archive:List-Help:List-Subscribe:From: Reply-To:From; b=vpescVbRFmVw09jKo28FzlnlOUvP2i7/LUr2lT7xCPApt8zFiN/6oja/5nqBF6zEX vVzXl+tDrKtjT88oZLLHOfVWYQC6EVQTD9ZVEMZ3FLUtZnJeSOD06faQToBSfl6pNT zXPXyDtDf+6bomOaZqR+Xr4TiFNy/Zg8RA2lhRlQ= 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.129.124]) by sourceware.org (Postfix) with ESMTPS id CAB873858D1E for ; Thu, 7 Sep 2023 14:05:21 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org CAB873858D1E Received: from mail-qt1-f200.google.com (mail-qt1-f200.google.com [209.85.160.200]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-261-mNuMh6esOA2bHgQHq1gBXg-1; Thu, 07 Sep 2023 10:05:15 -0400 X-MC-Unique: mNuMh6esOA2bHgQHq1gBXg-1 Received: by mail-qt1-f200.google.com with SMTP id d75a77b69052e-41272f89564so11463891cf.3 for ; Thu, 07 Sep 2023 07:05:14 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1694095512; x=1694700312; h=mime-version:user-agent:message-id:in-reply-to:date:references :organization:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=3uFJtZ5dl3r3CVXoy6syES+qcsStZE2umbEdwfV5zkg=; b=ZyS/S/+gq3nVK+LIDdF6+Y199M96VlSBebYcnPdBgI+XtzfHDITZIF1lpbhMo0P/Kv OViVxH1ODlUkaO2PWhy3kaiyT0W5hNQFSkvWSTxHr7qtlzbfJQ5KWwi5FzgV4okJAwoR Xs/hXjwJ3XvtTt9Y2sYWbrwRQFCo3BEJScdu7iODaPKn2ycSa++cDroJZGWh26KMLP5w OLHhtoOEsHM9drbVaRGKhpEajiG4UKND6OoTgsjR2zRhsVcuxKR4oSNbh3aS2b1CDVKU WxbQMQF9CndgOQzKOrFcrMC1SFU1UgIO0IpsX5H483CPIkbe+BPC1wlCjyD2sKWJvGK+ O1DA== X-Gm-Message-State: AOJu0YxFjTURlUf5t8AybrmfM0aQC5n+10EV1sPHe3j6ebjFWbyodcDM nDCnEetJl/1IUPkpL6O23aqwC35MhVkPZTUJMNGiwY5oH1nI/0EgrnFh/hSPm5SeE9cvCa7nYNW IfPZDAJdnS26SIu/yubCdmJBknIxE X-Received: by 2002:ac8:7f45:0:b0:411:fca1:76d with SMTP id g5-20020ac87f45000000b00411fca1076dmr21067135qtk.50.1694095510425; Thu, 07 Sep 2023 07:05:10 -0700 (PDT) X-Google-Smtp-Source: AGHT+IFnFP17Gg/yn/qwxnCdwfdGyjHRxyKl1Z2iZDxurO1ZKxUCtA45Riir+2RlAUT0F++6sFNcPQ== X-Received: by 2002:ac8:7f45:0:b0:411:fca1:76d with SMTP id g5-20020ac87f45000000b00411fca1076dmr21067056qtk.50.1694095509432; Thu, 07 Sep 2023 07:05:09 -0700 (PDT) Received: from localhost ([88.120.130.27]) by smtp.gmail.com with ESMTPSA id ay22-20020a05622a229600b003eabcc29132sm6169285qtb.29.2023.09.07.07.05.08 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 07 Sep 2023 07:05:08 -0700 (PDT) Received: by localhost (Postfix, from userid 1000) id 5D74EB4FD2; Thu, 7 Sep 2023 16:05:06 +0200 (CEST) To: Dodji Seketeli Cc: libabigail@sourceware.org Subject: [PATCH 12/16] ir: Avoid forgetting potential seemingly duplicated member functions Organization: Red Hat / France References: <87il8mglc1.fsf@redhat.com> X-Operating-System: CentOS Stream release 9 X-URL: http://www.redhat.com Date: Thu, 07 Sep 2023 16:05:06 +0200 In-Reply-To: <87il8mglc1.fsf@redhat.com> (Dodji Seketeli's message of "Thu, 07 Sep 2023 15:32:46 +0200") Message-ID: <871qfaf59p.fsf@redhat.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.2 (gnu/linux) MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-Spam-Status: No, score=-11.9 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_NONE, RCVD_IN_MSPIKE_H4, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_NONE, TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: libabigail@sourceware.org X-Mailman-Version: 2.1.30 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 some rare cases expressed in DWARF from C++, a member function can belong to both a declaration-only class (having no data member) in one translation unit, and to a fully defined class in another translation unit. In those cases, the two classes are represented and considered different. But then, consider a destructor which mangled name is "D1". D1 is represented in both the decl-only class and the fully-defined class. In the former case, its "this pointer" points to a decl-only class, while in the later case its "this pointer" points a fully-defined class. So, D1 coming from the former case will compare different from the D1 coming from the later case, because of the spurious difference "decl-only class" versus "fully-defined class". This is in the context of a self-comparison. One way to fix this self-comparison subtle change is to give the former D1 and the later D1 two different "function ID"s. Today, the function ID is just the mangled name, D1. This patch is defining a more involved ID which makes the difference between a member function on a decl-only class and a member function on a fully-defined class. Note that the only member functions that matter are virtual member functions because they are the only member functions that are considered when comparing two classes. This fixes the self-comparison error found on the binary gcc-gnat-12.3.1-1.fc37.x86_64/usr/libexec/gcc/x86_64-redhat-linux/12/gnat1 while running the self-comparison command below: $ fedabipkgdiff --self-compare -a --from fc37 gcc-gnat * src/abg-ir.cc (is_declaration_only_class_or_union_type): Add an overload for type_base_sptr. (function_decl::get_id): Virtual member functions that are defined on decl-only classes have a special ID to avoid confusing them with their counterparts defined on defined classes. * src/abg-reader.cc (build_function_decl) (build_function_decl_if_not_suppressed): Add a new add_to_exported_decls parameter to avoid adding the function to the set of exported decls to early. For the function to be properly added to the set of exported decls, all its attributes must be set, including virtualness. In some case, those attributes are not yet known by the end of the call to build_function_decl. The caller must then set those properties and then add the function to the set of exported decls. (build_{class,union}_decl): Build the function first, then add its missing properties and *then* add it to the set of exported decls. (handle_function_decl): Adjust. * tests/data/test-read-dwarf/test22-pr19097-libstdc++.so.6.0.17.so.abi: Adjust. Signed-off-by: Dodji Seketeli --- src/abg-ir.cc | 31 +++++++ src/abg-reader.cc | 42 +++++++--- .../test22-pr19097-libstdc++.so.6.0.17.so.abi | 82 +++++++++---------- 3 files changed, 102 insertions(+), 53 deletions(-) diff --git a/src/abg-ir.cc b/src/abg-ir.cc index 2bbb1a49..a1033c75 100644 --- a/src/abg-ir.cc +++ b/src/abg-ir.cc @@ -10564,6 +10564,21 @@ is_declaration_only_class_or_union_type(const type_base *t, return false; } + +/// Test wheter a type is a declaration-only class. +/// +/// @param t the type to considier. +/// +/// @param look_through_decl_only if true, then look through the +/// decl-only class to see if it actually has a class definition in +/// the same ABI corpus. +/// +/// @return true iff @p t is a declaration-only class. +bool +is_declaration_only_class_or_union_type(const type_base_sptr& t, + bool look_through_decl_only) +{return is_declaration_only_class_or_union_type(t.get(), look_through_decl_only);} + /// Test wheter a type is a declaration-only class. /// /// @param t the type to considier. @@ -21366,6 +21381,19 @@ function_decl::get_id() const const environment& env = get_type()->get_environment(); if (elf_symbol_sptr s = get_symbol()) { + string virtual_member_suffix; + if (is_member_function(this)) + { + method_decl* m = is_method_decl(this); + ABG_ASSERT(m); + if (get_member_function_is_virtual(m)) + { + if (is_declaration_only_class_or_union_type + (m->get_type()->get_class_type(), + /*look_through_decl_only=*/true)) + virtual_member_suffix += "/o"; + } + } if (s->has_aliases()) // The symbol has several aliases, so let's use a scheme // that allows all aliased functions to have different @@ -21374,6 +21402,9 @@ function_decl::get_id() const else // Let's use the full symbol name with its version as ID. priv_->id_ = env.intern(s->get_id_string()); + + if (!virtual_member_suffix.empty()) + priv_->id_ = env.intern(priv_->id_ + virtual_member_suffix); } else if (!get_linkage_name().empty()) priv_->id_= env.intern(get_linkage_name()); diff --git a/src/abg-reader.cc b/src/abg-reader.cc index 2b902096..993b6266 100644 --- a/src/abg-reader.cc +++ b/src/abg-reader.cc @@ -1397,11 +1397,11 @@ build_function_parameter (reader&, const xmlNodePtr); static function_decl_sptr build_function_decl(reader&, const xmlNodePtr, - class_or_union_sptr, bool); + class_or_union_sptr, bool, bool); static function_decl_sptr build_function_decl_if_not_suppressed(reader&, const xmlNodePtr, - class_or_union_sptr, bool); + class_or_union_sptr, bool, bool); static bool function_is_suppressed(const reader& rdr, @@ -3440,16 +3440,21 @@ build_function_parameter(reader& rdr, const xmlNodePtr node) /// shared_ptr that is returned is then really a /// shared_ptr. /// -/// @param add_to_current_scope if set to yes, the resulting of +/// @param add_to_current_scope if set to yes, the result of /// this function is added to its current scope. /// +/// @param add_to_exported_decls if set to yes, the resulting of this +/// function is added to the set of decls exported by the current +/// corpus being built. +/// /// @return a pointer to a newly created function_decl upon successful /// completion, a null pointer otherwise. static function_decl_sptr -build_function_decl(reader& rdr, +build_function_decl(reader& rdr, const xmlNodePtr node, class_or_union_sptr as_method_decl, - bool add_to_current_scope) + bool add_to_current_scope, + bool add_to_exported_decls) { function_decl_sptr nil; @@ -3555,7 +3560,8 @@ build_function_decl(reader& rdr, rdr.maybe_canonicalize_type(fn_type, !add_to_current_scope); - rdr.maybe_add_fn_to_exported_decls(fn_decl.get()); + if (add_to_exported_decls) + rdr.maybe_add_fn_to_exported_decls(fn_decl.get()); return fn_decl; } @@ -3578,6 +3584,10 @@ build_function_decl(reader& rdr, /// @param add_to_current_scope if set to yes, the resulting of /// this function is added to its current scope. /// +/// @param add_to_exported_decls if set to yes, the resulting of this +/// function is added to the set of decls exported by the current +/// corpus being built. +/// /// @return a pointer to a newly created function_decl upon successful /// completion. If the function was suppressed by a suppression /// specification then returns nil. @@ -3585,7 +3595,8 @@ static function_decl_sptr build_function_decl_if_not_suppressed(reader& rdr, const xmlNodePtr node, class_or_union_sptr as_method_decl, - bool add_to_current_scope) + bool add_to_current_scope, + bool add_to_exported_decls) { function_decl_sptr fn; @@ -3596,7 +3607,8 @@ build_function_decl_if_not_suppressed(reader& rdr, ; else fn = build_function_decl(rdr, node, as_method_decl, - add_to_current_scope); + add_to_current_scope, + add_to_exported_decls); return fn; } @@ -5148,7 +5160,8 @@ build_class_decl(reader& rdr, { if (function_decl_sptr f = build_function_decl_if_not_suppressed(rdr, p, decl, - /*add_to_cur_sc=*/true)) + /*add_to_cur_sc=*/true, + /*add_to_exported_decls=*/false)) { method_decl_sptr m = is_method_decl(f); ABG_ASSERT(m); @@ -5161,6 +5174,7 @@ build_class_decl(reader& rdr, set_member_function_is_dtor(m, is_dtor); set_member_function_is_const(m, is_const); rdr.map_xml_node_to_decl(p, m); + rdr.maybe_add_fn_to_exported_decls(f.get()); break; } } @@ -5505,7 +5519,8 @@ build_union_decl(reader& rdr, { if (function_decl_sptr f = build_function_decl_if_not_suppressed(rdr, p, decl, - /*add_to_cur_sc=*/true)) + /*add_to_cur_sc=*/true, + /*add_to_exported_decls=*/false)) { method_decl_sptr m = is_method_decl(f); ABG_ASSERT(m); @@ -5514,6 +5529,7 @@ build_union_decl(reader& rdr, set_member_function_is_ctor(m, is_ctor); set_member_function_is_dtor(m, is_dtor); set_member_function_is_const(m, is_const); + rdr.maybe_add_fn_to_exported_decls(f.get()); break; } } @@ -5626,7 +5642,8 @@ build_function_tdecl(reader& rdr, } else if (function_decl_sptr f = build_function_decl_if_not_suppressed(rdr, n, class_decl_sptr(), - /*add_to_current_scope=*/true)) + /*add_to_current_scope=*/true, + /*add_to_exported_decls=*/true)) fn_tmpl_decl->set_pattern(f); } @@ -6204,7 +6221,8 @@ handle_function_decl(reader& rdr, bool add_to_current_scope) { return build_function_decl_if_not_suppressed(rdr, node, class_decl_sptr(), - add_to_current_scope); + add_to_current_scope, + /*add_to_exported_decls=*/true); } /// Parse a 'class-decl' xml element. diff --git a/tests/data/test-read-dwarf/test22-pr19097-libstdc++.so.6.0.17.so.abi b/tests/data/test-read-dwarf/test22-pr19097-libstdc++.so.6.0.17.so.abi index 9ff6c6e4..3dfc812a 100644 --- a/tests/data/test-read-dwarf/test22-pr19097-libstdc++.so.6.0.17.so.abi +++ b/tests/data/test-read-dwarf/test22-pr19097-libstdc++.so.6.0.17.so.abi @@ -27187,7 +27187,7 @@ - + @@ -27210,7 +27210,7 @@ - + @@ -27223,7 +27223,7 @@ - + @@ -27235,7 +27235,7 @@ - + @@ -27245,7 +27245,7 @@ - + @@ -33661,7 +33661,7 @@ - + @@ -33684,7 +33684,7 @@ - + @@ -33697,7 +33697,7 @@ - + @@ -33709,7 +33709,7 @@ - + @@ -33719,7 +33719,7 @@ - + @@ -38839,7 +38839,7 @@ - + @@ -38847,7 +38847,7 @@ - + @@ -38856,7 +38856,7 @@ - + @@ -38865,7 +38865,7 @@ - + @@ -38874,14 +38874,14 @@ - + - + @@ -38889,14 +38889,14 @@ - + - + @@ -38904,14 +38904,14 @@ - + - + @@ -38920,7 +38920,7 @@ - + @@ -38928,7 +38928,7 @@ - + @@ -40375,14 +40375,14 @@ - + - + @@ -40390,14 +40390,14 @@ - + - + @@ -40520,7 +40520,7 @@ - + @@ -40528,7 +40528,7 @@ - + @@ -40537,7 +40537,7 @@ - + @@ -40546,7 +40546,7 @@ - + @@ -40555,14 +40555,14 @@ - + - + @@ -40570,14 +40570,14 @@ - + - + @@ -40585,14 +40585,14 @@ - + - + @@ -40601,7 +40601,7 @@ - + @@ -40609,7 +40609,7 @@ - + @@ -47214,7 +47214,7 @@ - + @@ -50629,7 +50629,7 @@ - + @@ -59336,7 +59336,7 @@ - +