From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp.gentoo.org (dev.gentoo.org [IPv6:2001:470:ea4a:1:5054:ff:fec7:86e4]) by sourceware.org (Postfix) with ESMTP id C5CED3972029 for ; Mon, 21 Sep 2020 23:14:08 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org C5CED3972029 Date: Tue, 22 Sep 2020 00:13:55 +0100 From: Sergei Trofimovich To: Alexander Monakov Cc: Martin =?UTF-8?B?TGnFoWth?= , Jan Hubicka , Nathan Sidwell , gcc-patches@gcc.gnu.org, Sergei Trofimovich Subject: Re: [PATCH] gcov: fix TOPN streaming from shared libraries Message-ID: <20200922001355.3e09fa4c@sf> In-Reply-To: References: <20200906112421.2363343-1-slyfox@gentoo.org> <424a6bdc-7a32-e1df-dab4-74232c128bb8@suse.cz> X-Mailer: Claws Mail 3.17.6 (GTK+ 2.24.32; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="MP_/Ri/cU.qJovSO5wxm+Zl0g_p" X-Spam-Status: No, score=-10.1 required=5.0 tests=BAYES_00, GIT_PATCH_0, KAM_DMARC_STATUS, RCVD_IN_DNSWL_NONE, SPF_HELO_PASS, 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: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 21 Sep 2020 23:14:10 -0000 --MP_/Ri/cU.qJovSO5wxm+Zl0g_p Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Content-Disposition: inline On Mon, 21 Sep 2020 20:38:07 +0300 (MSK) Alexander Monakov wrote: > On Mon, 21 Sep 2020, Martin Li=C5=A1ka wrote: >=20 > > On 9/6/20 1:24 PM, Sergei Trofimovich wrote: =20 > > > From: Sergei Trofimovich > > >=20 > > > Before the change gcc did not stream correctly TOPN counters > > > if counters belonged to a non-local shared object. > > >=20 > > > As a result zero-section optimization generated TOPN sections > > > in a form not recognizable by '__gcov_merge_topn'. > > >=20 > > > The problem happens because in a case of multiple shared objects > > > '__gcov_merge_topn' function is present in address space multiple > > > times (once per each object). > > >=20 > > > The fix is to never rely on function address and predicate on TOPN > > > counter types. =20 > >=20 > > Hello. > >=20 > > Thank you for the analysis! I think it's the correct fix and it's proba= bly > > similar to what we used to see for indirect_call_tuple. > >=20 > > @Alexander: Am I right? =20 >=20 > Yes, analysis presented by Sergei in Bugzilla looks correct. Pedantically= I > wouldn't say the indirect call issue was similar: it's a different gotcha > arising from mixing static and dynamic linking. There we had some symbols > preempted by the main executable (but not all symbols), here we have lack > of preemption/unification as relevant libgcov symbol is hidden. >=20 > I cannot judge if the fix is correct (don't know the code that well) but = it > looks reasonable. If you could come up with a clearer wording for the new > comment it would be nice, I struggled to understand it. Yeah, I agree the comment is very misleading. The code is already very clear about special casing of TOPN counters. How about dropping the comment? v2: =46rom 300585164f0a719a3a283c8da3a4061615f6da3a Mon Sep 17 00:00:00 2001 From: Sergei Trofimovich Date: Sun, 6 Sep 2020 12:13:54 +0100 Subject: [PATCH v2] gcov: fix TOPN streaming from shared libraries Before the change gcc did not stream correctly TOPN counters if counters belonged to a non-local shared object. As a result zero-section optimization generated TOPN sections in a form not recognizable by '__gcov_merge_topn'. The problem happens because in a case of multiple shared objects '__gcov_merge_topn' function is present in address space multiple times (once per each object). The fix is to never rely on function address and predicate on TOPN counter types. libgcc/ChangeLog: PR gcov-profile/96913 * libgcov-driver.c (write_one_data): Avoid function pointer comparison in TOP streaming decision. --- libgcc/libgcov-driver.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libgcc/libgcov-driver.c b/libgcc/libgcov-driver.c index 58914268d4e..e53e4dc392a 100644 --- a/libgcc/libgcov-driver.c +++ b/libgcc/libgcov-driver.c @@ -424,7 +424,7 @@ write_one_data (const struct gcov_info *gi_ptr, n_counts =3D ci_ptr->num; - if (gi_ptr->merge[t_ix] =3D=3D __gcov_merge_topn) + if (t_ix =3D=3D GCOV_COUNTER_V_TOPN || t_ix =3D=3D GCOV_COUNTER_V= _INDIR) write_top_counters (ci_ptr, t_ix, n_counts); else { --=20 Sergei --MP_/Ri/cU.qJovSO5wxm+Zl0g_p Content-Type: text/x-patch Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename=v2-0001-gcov-fix-TOPN-streaming-from-shared-libraries.patch >From 300585164f0a719a3a283c8da3a4061615f6da3a Mon Sep 17 00:00:00 2001 From: Sergei Trofimovich Date: Sun, 6 Sep 2020 12:13:54 +0100 Subject: [PATCH v2] gcov: fix TOPN streaming from shared libraries Before the change gcc did not stream correctly TOPN counters if counters belonged to a non-local shared object. As a result zero-section optimization generated TOPN sections in a form not recognizable by '__gcov_merge_topn'. The problem happens because in a case of multiple shared objects '__gcov_merge_topn' function is present in address space multiple times (once per each object). The fix is to never rely on function address and predicate on TOPN counter types. libgcc/ChangeLog: PR gcov-profile/96913 * libgcov-driver.c (write_one_data): Avoid function pointer comparison in TOP streaming decision. --- libgcc/libgcov-driver.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libgcc/libgcov-driver.c b/libgcc/libgcov-driver.c index 58914268d4e..e53e4dc392a 100644 --- a/libgcc/libgcov-driver.c +++ b/libgcc/libgcov-driver.c @@ -424,7 +424,7 @@ write_one_data (const struct gcov_info *gi_ptr, n_counts = ci_ptr->num; - if (gi_ptr->merge[t_ix] == __gcov_merge_topn) + if (t_ix == GCOV_COUNTER_V_TOPN || t_ix == GCOV_COUNTER_V_INDIR) write_top_counters (ci_ptr, t_ix, n_counts); else { -- 2.28.0 --MP_/Ri/cU.qJovSO5wxm+Zl0g_p--