From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from relay4-d.mail.gandi.net (relay4-d.mail.gandi.net [217.70.183.196]) by sourceware.org (Postfix) with ESMTPS id 7A62D385781A for ; Wed, 17 Mar 2021 13:40:57 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 7A62D385781A Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=seketeli.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=dodji@seketeli.org X-Originating-IP: 88.120.130.27 Received: from localhost (unknown [88.120.130.27]) (Authenticated sender: dodji@seketeli.org) by relay4-d.mail.gandi.net (Postfix) with ESMTPSA id 21568E0006; Wed, 17 Mar 2021 13:40:55 +0000 (UTC) Received: by localhost (Postfix, from userid 1000) id 4CF1958000E; Wed, 17 Mar 2021 14:40:40 +0100 (CET) From: Dodji Seketeli To: Matthias Maennich Cc: libabigail@sourceware.org, gprocida@google.com, kernel-team@android.com Subject: Re: [PATCH 17/20] symtab/dwarf-reader: allow hinting of main symbols for aliases Organization: Me, myself and I References: <20200619214305.562-1-maennich@google.com> <20210127125853.886677-1-maennich@google.com> <20210127125853.886677-18-maennich@google.com> X-Operating-System: Fedora 34 X-URL: http://www.seketeli.net/~dodji Date: Wed, 17 Mar 2021 14:40:40 +0100 In-Reply-To: <20210127125853.886677-18-maennich@google.com> (Matthias Maennich's message of "Wed, 27 Jan 2021 12:58:50 +0000") Message-ID: <871rcdyk2v.fsf@seketeli.org> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-4.2 required=5.0 tests=BAYES_00, JMQ_SPF_NEUTRAL, KAM_DMARC_STATUS, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: libabigail@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Mailing list of the Libabigail project List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 17 Mar 2021 13:40:59 -0000 Matthias Maennich a =C3=A9crit: > In case of aliased symbols, the "main symbol" cannot be deduced from > the symtab as this solely contains a name->addr mapping and aliases > are represented by multiple names resolving to the same address. > Therefore the main symbol can only be picked rather randomly and > unpredictable. > > Unlike DWARF, which contains a single symbol entry for only the main > symbol. Hence we can (late) detect the main symbol. Exploiting that > property allows to correct the addr->symbol lookup in the symtab to > return the correct main symbol and it also allows to correct the aliased > symbols to maintain the correct main symbol. > > This patch adds the `update_main_symbol` functionality to `elf_symbol` > to update the main symbol by name (ELF symbols need unique names) and > adds `update_main_symbol` to `symtab` that makes use of said new method. > > When we discover a main symbol during DWARF reading, we instruct the > symtab to update the mapping. > > This creates consistent representations across different builds of the > same binary with the same ABI by pinning down the main symbol to the > defined one. Knowing the main symbol also helps to keep the correct > dwarf information in the representation in the presence of symbol > suppressions. A later patch will address that. > > Some test cases in tests/data need adjustment and they have all been > verified to be valid changes. > > - main symbol changed for various elf symbols > - test-annotate/test15-pr18892.so.abi > - test-annotate/test19-pr19023-libtcmalloc_and_profiler.so.abi > - test-annotate/test3.so.abi > - test-read-dwarf/test15-pr18892.so.abi > - test-read-dwarf/test19-pr19023-libtcmalloc_and_profiler.so.abi > - test-read-dwarf/test3.so.abi > - test-read-dwarf/test3.so.hash.abi > > - due to main symbol changes, the symbol diff needs to be corrected > - test-diff-dwarf/test12-report.txt > - test-diff-pkg/tbb-4.1-9.20130314.fc22.x86_64--tbb-4.3-3.20141204.fc23= .x86_64-report-0.txt > - test-diff-pkg/tbb-4.1-9.20130314.fc22.x86_64--tbb-4.3-3.20141204.fc23= .x86_64-report-1.txt > > - the test scenario needed adjustments as the main symbol changed > - test-diff-suppr/test23-alias-filter-4.suppr > - test-diff-suppr/test23-alias-filter-report-0.txt > - test-diff-suppr/test23-alias-filter-report-2.txt > > As usual, the complete changelog follows. The patch itself looks good to me. However, the part that update the tests won't apply anymore given the cheer amount of changes that happen since the submission of this patchset. So I think you'll need to refresh them before being able to apply this one. > > * include/abg-ir.h (elf_symbol::update_main_symbol): New method. > * include/abg-symtab-reader.h (symtab::update_main_symbol): New method. > * src/abg-dwarf-reader.cc > (build_var_decl): Hint symtab about main symbol discovered in DWARF. > (build_function_decl): Likewise. > * src/abg-ir.cc (elf_symbol::get_main_symbol): Lock the weak_ptr > on access in both overloads. > (update_main_symbol): New method to allow updating the main symbol. > * src/abg-symtab-reader.cc (symtab::update_main_symbol): New method. > * data/Makefile.am: Add new test data files. > * tests/data/test-annotate/test15-pr18892.so.abi: Updated test file. > * tests/data/test-annotate/test19-pr19023-libtcmalloc_and_profiler.so.ab= i: Likewise. > * tests/data/test-annotate/test2.so.abi: Likewise. > * tests/data/test-annotate/test3.so.abi: Likewise. > * tests/data/test-diff-dwarf/test12-report.txt: Likewise. > * tests/data/test-diff-dwarf/test42-PR21296-clanggcc-report0.txt: Likewi= se. > * tests/data/test-diff-filter/test31-pr18535-libstdc++-report-0.txt: Lik= ewise. > * tests/data/test-diff-filter/test31-pr18535-libstdc++-report-0.txt: Lik= ewise. > * tests/data/test-diff-pkg/tbb-4.1-9.20130314.fc22.x86_64--tbb-4.3-3.201= 41204.fc23.x86_64-report-0.txt: Likewise. > * tests/data/test-diff-pkg/tbb-4.1-9.20130314.fc22.x86_64--tbb-4.3-3.201= 41204.fc23.x86_64-report-1.txt: Likewise. > * tests/data/test-diff-suppr/test23-alias-filter-4.suppr: Likewise. > * tests/data/test-diff-suppr/test23-alias-filter-report-0.txt: Likewise. > * tests/data/test-diff-suppr/test23-alias-filter-report-2.txt: Likewise. > * tests/data/test-read-dwarf/PR22015-libboost_iostreams.so.abi: Likewise. > * tests/data/test-read-dwarf/PR22122-libftdc.so.abi: Likewise. > * tests/data/test-read-dwarf/test10-pr18818-gcc.so.abi: Likewise. > * tests/data/test-read-dwarf/test11-pr18828.so.abi: Likewise. > * tests/data/test-read-dwarf/test12-pr18844.so.abi: Likewise. > * tests/data/test-read-dwarf/test15-pr18892.so.abi: Likewise. > * tests/data/test-read-dwarf/test16-pr18904.so.abi: Likewise. > * tests/data/test-read-dwarf/test19-pr19023-libtcmalloc_and_profiler.so.= abi: Likewise. > * tests/data/test-read-dwarf/test2.so.abi: Likewise. > * tests/data/test-read-dwarf/test2.so.hash.abi: Likewise. > * tests/data/test-read-dwarf/test22-pr19097-libstdc++.so.6.0.17.so.abi: = Likewise. > * tests/data/test-read-dwarf/test3.so.abi: Likewise. > * tests/data/test-read-dwarf/test3.so.hash.abi: Likewise. > * tests/data/test-symtab/basic/aliases.c: New test source file. > * tests/data/test-symtab/basic/aliases.so: Likewise. > * tests/test-symtab.cc (Symtab::AliasedFunctionSymbols): New test case. > (Symtab::AliasedVariableSymbols): Likewise. > > Reviewed-by: Giuliano Procida > Signed-off-by: Matthias Maennich OK to apply to master after refreshing the test updates. Thanks! [...] Cheers, --=20 Dodji