From patchwork Fri Nov 27 17:07:20 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dodji Seketeli X-Patchwork-Id: 41219 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 70B743857026; Fri, 27 Nov 2020 17:07:44 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 70B743857026 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1606496864; bh=UCOEydlqisOb7u33wV6PqxVPRhNUEUpowBXWRUV8QWk=; h=To:Subject:References:Date:In-Reply-To:List-Id:List-Unsubscribe: List-Archive:List-Help:List-Subscribe:From:Reply-To:Cc:From; b=CuqLnkXqgQQojHuWSFNcpNNMbCkkS3qBZ81qAaGnMZTCPPAMEUl7bz7QHmhiSBVEZ g/MHMqXkoDwzJUlG2CuQ1QSuO0dSJ50H40NGdzQt12VUUIfttJxQHhC8LwSvmc9vxI Az+TUt5E52CP7UT5vDmMlIbUSD7LhX+U+x0bAMfo= 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 [63.128.21.124]) by sourceware.org (Postfix) with ESMTP id 8AA443857026 for ; Fri, 27 Nov 2020 17:07:37 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 8AA443857026 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-562-PT182sNdMXKk9_kpjiMDIA-1; Fri, 27 Nov 2020 12:07:35 -0500 X-MC-Unique: PT182sNdMXKk9_kpjiMDIA-1 Received: by mail-wr1-f71.google.com with SMTP id z13so3706413wrm.19 for ; Fri, 27 Nov 2020 09:07:35 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:organization:references:date :in-reply-to:message-id:user-agent:mime-version; bh=UCOEydlqisOb7u33wV6PqxVPRhNUEUpowBXWRUV8QWk=; b=q/JoGxTWjr9K/Yct/vjVYOSPuZzq4PJ1xX/LwHQVIT4P1wk92RgnI4KvQws8BCeEy5 xDGq1tO5buA8iM1bhvSDT5uM+fx1t/e6PdC3+yI7Ze/FImE7TLQf9R8lh1m89nXP+RNn ZBr5HenkXDiL9OHOtumbsefWLX4qk6bNL2vmxEMCCIDm2XalIHTwHpNSfeyhTrNIZdz+ VPBgWhDys5sfDUGyEfFy5j0uDUr/6hVCt8i351D88+lklHVyeICUGJW1vMhtwtMSL2XA Lnts7CjV6791xtjPGvAbQ578Te4T3VaQTUKe52h1lO7sEcLBoV0+G27qgxmOxnNgQ5Cu V6oQ== X-Gm-Message-State: AOAM530SYLLNKwiogPNmcwvpHPD+2d6XRw089hIlmVsUgRZrMKC++GYp KO/RVLsh99k2y1Bm+wBHDdEi+509hEk3DYZ37R0i2FiPDm3opLeufB+RwemDmkb5qHNikCiPrZL fu2qB5J5VXqojPHrN73ZJ X-Received: by 2002:a1c:9ecf:: with SMTP id h198mr10194927wme.104.1606496848829; Fri, 27 Nov 2020 09:07:28 -0800 (PST) X-Google-Smtp-Source: ABdhPJx/5w9CSmwGsCqI1T8aRvOhe0xV+W+8C9K7uCXaBJqBP+YzkXSfmxocSXa4IvIvtxaC1iJpRA== X-Received: by 2002:a1c:9ecf:: with SMTP id h198mr10194902wme.104.1606496848416; Fri, 27 Nov 2020 09:07:28 -0800 (PST) Received: from localhost (91-166-131-65.subs.proxad.net. [91.166.131.65]) by smtp.gmail.com with ESMTPSA id f18sm15457761wru.42.2020.11.27.09.07.20 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 27 Nov 2020 09:07:22 -0800 (PST) Received: by localhost (Postfix, from userid 1000) id 38A305802B4; Fri, 27 Nov 2020 18:07:20 +0100 (CET) To: Dodji Seketeli Subject: [PATCH 4/6] dwarf-reader: Avoid having several functions with the same symbol Organization: Red Hat / France References: <87h7pa7n8c.fsf@redhat.com> X-Operating-System: Fedora 34 X-URL: http://www.redhat.com Date: Fri, 27 Nov 2020 18:07:20 +0100 In-Reply-To: <87h7pa7n8c.fsf@redhat.com> (Dodji Seketeli's message of "Fri, 27 Nov 2020 17:56:03 +0100") Message-ID: <87zh326853.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=-10.4 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_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: libabigail@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Mailing list of the Libabigail project List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-Patchwork-Original-From: Dodji Seketeli via Libabigail From: Dodji Seketeli Reply-To: Dodji Seketeli Cc: libabigail@sourceware.org Errors-To: libabigail-bounces@sourceware.org Sender: "Libabigail" Hello, In the DWARF debug info, a C++ class can be represented by pieces throughout a given binary. In this picture, a given virtual member function can be represented several times; each time in one different piece of the C++ class. In a given piece of the class, a virtual member function can be represented with its ELF symbol set. In another one, the same virtual member function can be represented without that ELF symbol set. And there can also be pieces of the class that don't have a given virtual function. To handle this, the DWARF reader constructs one class from all its pieces scattered around. It adds each virtual member function to the class as it comes across them while scanning the DWARF. Then there is a pass at the end of the process that sets ELF symbols to the (virtual) member functions that need it. The problem with that pass is that it sometimes sets the same ELF symbol to more than one virtual member function. Those virtual member functions all have the same mangled name that correspond to the ELF symbol; but only one of them is meant to be associated with the ELF symbol. In essence, that one is the one that is exported by the ELF binary. This patch teaches the pass that sets the ELF symbols of function to avoid setting the same ELF symbol to more than one function. The patch also avoids build_function_decl to set symbol to a function if that symbol was already set to an existing function. This patch thus fixes a class of issues what arise when comparing a binary against its own ABIXML representation. Those several functions having the same ELF symbol would cause spurious changes in that context. * src/abg-dwarf-reader.cc (read_context::symbol_already_belongs_to_a_function): Define new member function. (read_context::fixup_functions_with_no_symbols): Use the new symbol_already_belongs_to_a_function function to avoid setting a symbol that already belongs to a function. * tests/data/test-read-dwarf/PR22122-libftdc.so.abi: Adjust. * tests/data/test-read-dwarf/test10-pr18818-gcc.so.abi: Likewise. * tests/data/test-read-dwarf/test16-pr18904.so.abi: Likewise. * tests/data/test-read-dwarf/test22-pr19097-libstdc++.so.6.0.17.so.abi: Likewise. Signed-off-by: Dodji Seketeli Applied to master. --- src/abg-dwarf-reader.cc | 53 ++++++- .../test-read-dwarf/PR22122-libftdc.so.abi | 2 +- .../test-read-dwarf/test10-pr18818-gcc.so.abi | 10 +- .../test-read-dwarf/test16-pr18904.so.abi | 48 +++--- .../test22-pr19097-libstdc++.so.6.0.17.so.abi | 140 +++++++++--------- 5 files changed, 152 insertions(+), 101 deletions(-) diff --git a/src/abg-dwarf-reader.cc b/src/abg-dwarf-reader.cc index 6e9f3cad..c2eb17d4 100644 --- a/src/abg-dwarf-reader.cc +++ b/src/abg-dwarf-reader.cc @@ -4630,6 +4630,40 @@ public: } } + /// Test if a symbol belongs to a function of the current ABI + /// corpus. + /// + /// This is a sub-routine of fixup_functions_with_no_symbols. + /// + /// @param fn the function symbol to consider. + /// + /// @returnt true if @p fn belongs to a function of the current ABI + /// corpus. + bool + symbol_already_belongs_to_a_function(elf_symbol_sptr& fn) + { + corpus_sptr corp = current_corpus(); + if (!corp) + return false; + + string id = fn->get_id_string(); + + const vector *fns = corp->lookup_functions(id); + if (!fns) + return false; + + for (vector::const_iterator i = fns->begin(); + i != fns->end(); + ++i) + { + function_decl* f = *i; + ABG_ASSERT(f); + if (f->get_symbol()) + return true; + } + return false; + } + /// Some functions described by DWARF may have their linkage name /// set, but no link to their actual underlying elf symbol. When /// these are virtual member functions, comparing the enclosing type @@ -4663,6 +4697,23 @@ public: if (elf_symbol_sptr sym = corp->lookup_function_symbol(i->second->get_linkage_name())) { + // So i->second is a virtual member function that was + // previously scheduled to be set a function symbol. + // + // But if it appears that it now has a symbol already set, + // then do not set a symbol to it again. + // + // Or if it appears that another virtual member function + // from the current ABI Corpus, with the same linkage + // (mangled) name has already been set a symbol, then do not + // set a symbol to this function either. Otherwise, there + // will be two virtual member functions with the same symbol + // in the class and that leads to spurious hard-to-debug + // change reports later down the road. + if (i->second->get_symbol() + || symbol_already_belongs_to_a_function(sym)) + continue; + ABG_ASSERT(is_member_function(i->second)); ABG_ASSERT(get_member_function_is_virtual(i->second)); i->second->set_symbol(sym); @@ -15707,7 +15758,7 @@ build_function_decl(read_context& ctxt, fn_sym = ctxt.function_symbol_is_exported(fn_addr); } - if (fn_sym) + if (fn_sym && !ctxt.symbol_already_belongs_to_a_function(fn_sym)) { result->set_symbol(fn_sym); string linkage_name = result->get_linkage_name(); diff --git a/tests/data/test-read-dwarf/PR22122-libftdc.so.abi b/tests/data/test-read-dwarf/PR22122-libftdc.so.abi index d697e447..dffdfd3e 100644 --- a/tests/data/test-read-dwarf/PR22122-libftdc.so.abi +++ b/tests/data/test-read-dwarf/PR22122-libftdc.so.abi @@ -1348,7 +1348,7 @@ - + diff --git a/tests/data/test-read-dwarf/test10-pr18818-gcc.so.abi b/tests/data/test-read-dwarf/test10-pr18818-gcc.so.abi index adc9dcdd..acab3bd5 100644 --- a/tests/data/test-read-dwarf/test10-pr18818-gcc.so.abi +++ b/tests/data/test-read-dwarf/test10-pr18818-gcc.so.abi @@ -6254,7 +6254,7 @@ - + @@ -8802,28 +8802,28 @@ - + - + - + - + diff --git a/tests/data/test-read-dwarf/test16-pr18904.so.abi b/tests/data/test-read-dwarf/test16-pr18904.so.abi index ab5c7e1f..449528e7 100644 --- a/tests/data/test-read-dwarf/test16-pr18904.so.abi +++ b/tests/data/test-read-dwarf/test16-pr18904.so.abi @@ -3566,7 +3566,7 @@ - + @@ -3681,7 +3681,7 @@ - + @@ -5618,7 +5618,7 @@ - + @@ -6696,7 +6696,7 @@ - + @@ -7781,7 +7781,7 @@ - + @@ -11063,7 +11063,7 @@ - + @@ -14631,7 +14631,7 @@ - + @@ -15002,7 +15002,7 @@ - + @@ -18455,7 +18455,7 @@ - + @@ -18974,7 +18974,7 @@ - + @@ -19183,7 +19183,7 @@ - + @@ -19231,7 +19231,7 @@ - + @@ -22663,7 +22663,7 @@ - + @@ -22676,7 +22676,7 @@ - + @@ -22689,7 +22689,7 @@ - + @@ -22702,7 +22702,7 @@ - + @@ -22883,7 +22883,7 @@ - + @@ -24103,7 +24103,7 @@ - + @@ -24117,7 +24117,7 @@ - + @@ -26209,7 +26209,7 @@ - + @@ -37366,7 +37366,7 @@ - + @@ -37379,7 +37379,7 @@ - + @@ -37392,7 +37392,7 @@ - + @@ -37405,7 +37405,7 @@ - + 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 1d5daafd..e6b52d26 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 @@ -26211,7 +26211,7 @@ - + @@ -26247,7 +26247,7 @@ - + @@ -26523,6 +26523,13 @@ + + + + + + + @@ -26530,11 +26537,12 @@ - - + + - - + + + @@ -26545,12 +26553,11 @@ - - + + - - - + + @@ -26560,11 +26567,12 @@ - - + + - - + + + @@ -26575,14 +26583,6 @@ - - - - - - - - @@ -26714,7 +26714,7 @@ - + @@ -26730,7 +26730,7 @@ - + @@ -26748,7 +26748,7 @@ - + @@ -26766,7 +26766,7 @@ - + @@ -26784,7 +26784,7 @@ - + @@ -26798,7 +26798,7 @@ - + @@ -26814,7 +26814,7 @@ - + @@ -26828,7 +26828,7 @@ - + @@ -26844,7 +26844,7 @@ - + @@ -26858,7 +26858,7 @@ - + @@ -26876,7 +26876,7 @@ - + @@ -26892,7 +26892,7 @@ - + @@ -45277,7 +45277,7 @@ - + @@ -45313,7 +45313,7 @@ - + @@ -47757,7 +47757,7 @@ - + @@ -47773,7 +47773,7 @@ - + @@ -47791,7 +47791,7 @@ - + @@ -47809,7 +47809,7 @@ - + @@ -47827,7 +47827,7 @@ - + @@ -47841,7 +47841,7 @@ - + @@ -47857,7 +47857,7 @@ - + @@ -47871,7 +47871,7 @@ - + @@ -47887,7 +47887,7 @@ - + @@ -47901,7 +47901,7 @@ - + @@ -47919,7 +47919,7 @@ - + @@ -47935,7 +47935,7 @@ - + @@ -49990,6 +49990,13 @@ + + + + + + + @@ -49997,11 +50004,12 @@ - - + + - - + + + @@ -50012,12 +50020,11 @@ - - + + - - - + + @@ -50027,11 +50034,12 @@ - - + + - - + + + @@ -50042,14 +50050,6 @@ - - - - - - - -