From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mout.gmx.net (mout.gmx.net [212.227.15.18]) by sourceware.org (Postfix) with ESMTPS id CA98B3858023 for ; Thu, 18 Feb 2021 14:27:15 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org CA98B3858023 X-UI-Sender-Class: 01bb95c1-4bf8-414a-932a-4f6e2808ef9c Received: from skylla.schaltzentrale ([46.91.80.159]) by mail.gmx.net (mrgmx005 [212.227.17.190]) with ESMTPSA (Nemesis) id 1MfHAH-1lnytQ19Mh-00gqsV; Thu, 18 Feb 2021 15:27:11 +0100 Date: Thu, 18 Feb 2021 15:27:02 +0100 From: Alexander Miller To: Mark Wielaard , elfutils-devel@sourceware.org Subject: Re: [PATCH] Improve building with LTO Message-ID: <20210218152702.18d515ed.alex.miller@gmx.de> In-Reply-To: References: <20210214235718.7654b5f1.alex.miller@gmx.de> X-Mailer: Claws Mail 3.17.7 (GTK+ 2.24.32; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: quoted-printable X-Provags-ID: V03:K1:eBE29TW1wj7oEZbWYMp0sobIQGQgqnccBh+oOwhZNO5SesJcy+x c7gQS/qwLxvx0JUAGLqzm18Uby7ZU11Go8uUmYn4DErHexQ65sdyFpwE3erdaL8nW07Vjzu ZdAva8IKezVY/+MWbgtT0/X2qCTU4AIAIzM2a/G2Awa/OOzV5jiYAg/TzCIrE4A2Lgjy6Z9 zz/hGqihRF1GFrpHNI/Kw== X-UI-Out-Filterresults: notjunk:1;V03:K0:JmPLoiJX6Io=:3XYWiCUdF+4LWRGtszgYvE 6xxEVC3B9cg0Nci+CxlC8DI6curDzyq1Oaaq7XH3ISrMYG1HcWve1GxzR6kMDrKmK84yEt/H1 CqrI1C1lUl5GaXg3UzQOgs3Phyw2qjJYecZiWuX9pcyT8wccl/jquehpbt+YcqBeQcbLu38WA qJ4eo9DuUw5dkotYTtWqvHTswGke+xjzhyenu3hnWoOpqYuze7XqplCWaAQOsWnQBK1cTMpEu I0Tmq1akWoQM7e/Pe7oGoAtwer+6P1wTuqP2bhR0hsfc0SriQmP7xwha+RTehFOhdVl0xhvtM +QbqT3Sy9p8b34NyX05N2SEpXQOwkc1ylpWSBRr8iOoPJLeBLCl6CBjZ2DePsI8o1pgx0RHPK urVlbAC/SsXoE7Y/YBGLzYxg8MNTmQcchGNxh6fyJZ6Rr712aEhdUXCb6EkhPeuNBYHavn9cs rg09pRyteeFyY2QqGEXfURH9a/a7NvdnU39w8+QPGqOYCRPJTCAcEG6Kkzu6jRQ/RCO2hkPY0 IV9n7si2WZHbM1Dsvq2b3Kl/RRaUWfrM3fjoQ7Vamsl61KlTrFLn7yw3g7yB6kueFnUQLmPmo nofkD3Y1/uiF49iEo9u4v4WZmw43/zvqBNn5vc3GABPctpwMUx9aTSzXgAqsVt4sdxp+lxiDj q5TZLXNb2qwPpkxJRA5Y+Ejo1U2LplZzLdQDo1YvxAgiskSWnVF7UBkjSE7g61/0MPSZrRA13 sXgiDukGo3lKAzCgi8RREqRbJUaDB18C9aa4DrWB6GdRoBCCpVno0keqNBCzT09XsUTZHR355 L1GNDjLti/h6j15tWzttq9wYusU0Faknyn10b9Ad66jXoZ1/geGiCR+Gzy+fyfB2LgCfAYhXW JJRqTM8pF1RPtUuKOzqg== X-Spam-Status: No, score=-5.5 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, FREEMAIL_FROM, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H2, 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: Thu, 18 Feb 2021 14:27:18 -0000 [Re-sending mail because the list has been omitted by mistake.] On Wed, 17 Feb 2021 21:22:15 +0100 Mark Wielaard wrote: > 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. Yes, I wanted symbol versioning to work the same way in both cases. > > 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 opt= ions > > 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. Of course gcc-6 is really old (I just happened to have it still available)= . But getting LTO to work with gcc-9 would be nice. That worked in my tests, but I'm not sure if it's reliable. And it's only a small addition (and I didn't add anything more for gcc-6). But most importantly I didn't want to paint myself into a corner and chang= e symbol versioning in a way that will only ever work with one particular toolchain. > > 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? Only the 64bit builds with bfd passed. For all 32bit builds (64bit system, but built with -m32) the test case "run-backtrace-native-biarch.sh" fails: | | 0x556a53cef000 0x556a53cf4000 .../tests/backtrace-child-biarch | 0x7fadd8118000 0x7fadd828d000 /lib64/libc-2.32.so | 0x7fadd8298000 0x7fadd82b4000 /lib64/libpthread-2.32.so | 0x7fadd82f0000 0x7fadd831d000 /lib64/ld-2.32.so | 0x7ffdf13fc000 0x7ffdf13fd000 [vdso: 6783] | TID 6783: | .../tests/backtrace: dwfl_thread_getframes: no error | backtrace: linux-pid-attach.c:326: pid_set_initial_registers: Assertion = `pid_arg->tid_attached =3D=3D 0' failed. | ./test-subr.sh: line 84: 6782 Aborted LD_LIBRARY_PATH= =3D"${built_library_path}${LD_LIBRARY_PATH:+:}$LD_LIBRARY_PATH" $VALGRIND_= CMD "$@" | backtrace-child-biarch: no main | FAIL run-backtrace-native-biarch.sh (exit status: 1) But that isn't related to my patch. It was the same before. > > * 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. I've run a few tests and it works without that change. I'll skip that in the next version of the patch. (Gold has trouble when a an object defines an unversioned symbol and the version script contains the old version; that isn't the case in my solution because the "new" functions get renamed.) > How does the __COUNTER__ magic work? It doesn't. Sorry. I'm adding two levels of indirection to make it work. I didn't notice since it's not needed at the moment. In the future, somebody might want to add multiple old versions for a symbol, and then they need different names. > I have to stare at the marcos a bit to make sure I really understand > what is going on. But this looks really good. Don't hesitate to ask if something isn't clear. I'll post an updated patch later. Cheers, Alex