From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gnu.wildebeest.org (wildebeest.demon.nl [212.238.236.112]) by sourceware.org (Postfix) with ESMTPS id 243EA3946C34 for ; Wed, 17 Feb 2021 20:22:17 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 243EA3946C34 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=klomp.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=mark@klomp.org Received: from tarox.wildebeest.org (tarox.wildebeest.org [172.31.17.39]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by gnu.wildebeest.org (Postfix) with ESMTPSA id C96CA30014A4; Wed, 17 Feb 2021 21:22:15 +0100 (CET) Received: by tarox.wildebeest.org (Postfix, from userid 1000) id B3BC84000BAB; Wed, 17 Feb 2021 21:22:15 +0100 (CET) Message-ID: Subject: Re: [PATCH] Improve building with LTO From: Mark Wielaard To: Alexander Miller , elfutils-devel@sourceware.org Date: Wed, 17 Feb 2021 21:22:15 +0100 In-Reply-To: <20210214235718.7654b5f1.alex.miller@gmx.de> References: <20210214235718.7654b5f1.alex.miller@gmx.de> Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Mailer: Evolution 3.28.5 (3.28.5-10.el7) Mime-Version: 1.0 X-Spam-Status: No, score=-5.6 required=5.0 tests=BAYES_00, KAM_DMARC_STATUS, 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: elfutils-devel@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Elfutils-devel mailing list List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 17 Feb 2021 20:22:18 -0000 Hi Alexander, First my apologies, your last commit had "Alexander Miller via Elfutils-devel " as author. That is the stupid mailinglist doing From mangling, bad mailinglist. But also bad me for not catching that. We probably need a git hook to reject pushes with such bogus authors. Could you add a From: Alexander Miller < alex.miller@gmx.de> in the body (I believe git send-email --from=3D... will do that for you). That way git am does the right thing. On Sun, 2021-02-14 at 23:57 +0100, Alexander Miller via Elfutils-devel wrot= e: > Here's my attempt to use gcc-10's new symver attribute to avoid global > asm statements that cause trouble with LTO. That requires converting > from triple @ syntax to double @ syntax. To satisfy those picky linkers, > the unversioned name of the affected symbols has to be changed. > The NEW_VERSION macros have to be moved before the function definitions. This is really exciting! I had more or less given up on this. I couldn't make it work because of the triple @ syntax not being supported. But it looks we don't need that after all. > Also tried to improve the situation for older gcc versions. Although > gcc-5 added a no_reorder attribute that's supposed to help here, it > doesn't work reliably (but improves the situation substantially). So you rewrite the asm statements to not use @@@ just @ and @@, that way they match the symver attribute usage. Smart, that way they are as similar as possible. > I've tested the patch with different compilers (gcc-10, gcc-9, gcc-6, > clang-11), linkers (bfd, gold, lld), 32/64 bit (x86), with/without LTO. > Of course you need to cheat a bit to build elfutils with clang, and > lld can't be used with every combination. The workaround for older gcc > was enough for gcc-9 and 32bit gcc-6, but 64bit gcc-6 builds needed > -flto-partition=3Dnone, so this seems to depend on the version and option= s > used. Wow, you really went above and beyond. IMHO it would have been fine to simply say that you need GCC10 and a recent binutils ld to get LTO working. Supporting LTO with gcc-6 is really nice, but people stuck on such an old compiler should probably first look at upgrading that than trying to play with LTO. We should probably look at making an LTO build part of the buildbot. > Symbol versioning worked as expected in all cases, at least the list > of dynamic symbols of the libs looked good to me. >=20 > The test suite seems brittle, though. It fails on 32bit builds, with > gold or lld, and with lto builds using clang (unknown object format) > or gcc-6 (debug info not found). But that's not related to this patch. > For 64bit builds with gcc-{9,10} and bfd, the test suite succeeds even > with lto enabled. Do the 32bit builds with gcc10/binutils ld have a clean test suite? > Additional notes: > * The asm names for the compat versions seem unnecessary, but I've > kept them. > * Commenting out old versions in the .map file may not be needed. > It's mostly a leftover from an earlier attempt, but I didn't want to > re-run all test and I actually prefer it like this, so I left it in. I would like to make sure it isn't needed. Having to comment out old versions is a bit of a pain. In my own attempts I actually tried it the other way around. Keep the earliest version in the version script and don't add (remove) new versions (those would come from the symver). That is slightly easier for maintenance, so you don't need to remove older versions, just add the new (.symver) version in the source. > * See commit message below. >=20 > ------------------------ 8< ------------------------ > Use symver attribute for symbol versioning instead of .symver > assembler directive when available. Convert to use double @ syntax > for default version in all cases (required when using the attribute). >=20 > Add the attributes externally_visible, no_reorder if available when > using assembler directives to improve the situation for < gcc-10. > This is not 100% reliable, though; -flto-partition=3Dnone may still be > needed in some cases. How does the __COUNTER__ magic work? > Note that -Wno-error=3Dstack-usage=3D is still needed to build with LTO. Yeah that is a pity. The issue is in just a few files, but with LTO those get combined with all the others causing issues. I believe the warnings are only in the tools now. It would be really good if we could get rid of the stack-usage warnings in: readelf.c, nm.c, size.c, strip.c, elflint.c, findtextrel.c, elfcmp.c, objdump.c, ranlib.c, ar.c, unstrip.c. But that is for another day. I have to stare at the marcos a bit to make sure I really understand what is going on. But this looks really good. Thanks, Mark