From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from relay9-d.mail.gandi.net (relay9-d.mail.gandi.net [217.70.183.199]) by sourceware.org (Postfix) with ESMTPS id 34CDE385EC58 for ; Wed, 17 Mar 2021 15:45:02 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 34CDE385EC58 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 relay9-d.mail.gandi.net (Postfix) with ESMTPSA id 57B58FF806; Wed, 17 Mar 2021 15:45:00 +0000 (UTC) Received: by localhost (Postfix, from userid 1000) id AA3075800FB; Wed, 17 Mar 2021 16:44:59 +0100 (CET) From: Dodji Seketeli To: Matthias Maennich Cc: libabigail@sourceware.org, gprocida@google.com, kernel-team@android.com Subject: Re: [PATCH 18/20] dwarf-reader/writer: consider aliases when dealing with suppressions Organization: Me, myself and I References: <20200619214305.562-1-maennich@google.com> <20210127125853.886677-1-maennich@google.com> <20210127125853.886677-19-maennich@google.com> X-Operating-System: Fedora 34 X-URL: http://www.seketeli.net/~dodji Date: Wed, 17 Mar 2021 16:44:59 +0100 In-Reply-To: <20210127125853.886677-19-maennich@google.com> (Matthias Maennich's message of "Wed, 27 Jan 2021 12:58:51 +0000") Message-ID: <87v99pwzr8.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, 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 15:45:04 -0000 Matthias Maennich a =C3=A9crit: > When a symbol is suppressed and it happens to be the main symbol of a > group of aliased symbols where another symbol is not suppressed, the > dwarf reader discards the DWARF information upon reading and the writer > will not be able to connect dwarf information to the aliased elf symbol. I think I understand what you mean here, but I'd rather say it this way for the sake of avoiding confusion: When the symbol of a decl is suppressed and it happens to be the main symbol of a group of aliased symbols where another symbol is not suppressed, the dwarf reader discards the decl from the internal representation altogether upon reading and thus the writer will not be able to connect that decl to the non-suppressed aliased elf symbol. To me, once the IR is built, there is more DWARF involved. So the writer doesn't deal with any DWARF anymore. It deals with the IR. Where that IR comes from is another matter. It can come from DWARF, abixml or maybe something else in the future. That's why it can be confusing. OK, I am nit picking a little bit, but I think it's important to make this clear. > In order to address this, ensure we are not suppressing symbols > (actually functions and variables) for which an alias is not suppressed. Right, so you felt the need to talk about decls (functions and variable) as well ;-) So how about saying: In order to address this, ensure we are not suppressing decls for which an alias is not suppressed. > We therefore keep the DWARF information even if only a non-main symbol > is asked for. We therefore keep the decl in the IR when at least one its underlying aliased symbols is non-suppressed. > Likewise, when the abg-writer is having to attach an elf-symbol-id to > the DWARF collected information (for functions and variables),=20 Likewise, when the abg-writer is having to attach an elf-symbol-id to the decl, > instead of omitting the symbol altogether, rather make use of the propert= y of > aliases and connect the dwarf information to an alias instead. This way > the function dwarf information stays connected to the elf symbol that we > want to track. > > When reading from XML with a symbol whitelist that leads to suppression > of aliased symbols, abidiff would hit an assertion and crash when > looking up the aliased symbol due to it being suppressed. In the new > symtab reader we can still suppress a symbol without removing it from > the lookup. Make use of that property to fix this bug. > > A test has been added for this as well. > > * src/abg-dwarf-reader.cc(function_is_suppressed): Do not suppress > a function for which there is an alias that is not suppressed. > (variable_is_suppressed): Likewise for variables. > * src/abg-reader.cc (build_elf_symbol): Improve handling of > suppressed aliased symbols when reading from XML. > * src/abg-symtab-reader.cc (load): Likewise. > * src/abg-writer.cc(write_elf_symbol_reference): Fall back to > any aliased symbol if the main symbol is suppressed. > * tests/data/Makefile.am: Add new test files. > * tests/data/test-abidiff-exit/test-missing-alias-report.txt: New test f= ile. > * tests/data/test-abidiff-exit/test-missing-alias.abi: Likewise. > * tests/data/test-abidiff-exit/test-missing-alias.suppr: Likewise. > * tests/test-abidiff-exit.cc: Add support for whitelists and add > new testcase. > * tests/data/test-read-dwarf/test-suppressed-alias.c: New test file. > * tests/data/test-read-dwarf/test-suppressed-alias.o: Likewise. > * tests/data/test-read-dwarf/test-suppressed-alias.o.abi: Likewise. > * tests/data/test-read-dwarf/test-suppressed-alias.suppr: Likewise. > * tests/data/test-read-dwarf/test3-alias-1.so.hash.abi: Likewise. > * tests/data/test-read-dwarf/test3-alias-1.suppr: Likewise. > * tests/data/test-read-dwarf/test3-alias-2.so.hash.abi: Likewise. > * tests/data/test-read-dwarf/test3-alias-2.suppr: Likewise. > * tests/data/test-read-dwarf/test3-alias-3.so.hash.abi: Likewise. > * tests/data/test-read-dwarf/test3-alias-3.suppr: Likewise. > * tests/data/test-read-dwarf/test3-alias-4.so.hash.abi: Likewise. > * tests/data/test-read-dwarf/test3-alias-4.suppr: Likewise. > * tests/test-read-dwarf.cc: Add new test cases. > > Reviewed-by: Giuliano Procida > Signed-off-by: Matthias Maennich OK to apply master with those commit log changes when the prerequisite patches are in. Thanks! [...] Cheers, --=20 Dodji