From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 62615 invoked by alias); 12 Aug 2019 10:39:44 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 62338 invoked by uid 89); 12 Aug 2019 10:39:43 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-7.2 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,GIT_PATCH_2,GIT_PATCH_3,KAM_SHORT,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.1 spammy=hmm, appending X-HELO: mail-lj1-f194.google.com Received: from mail-lj1-f194.google.com (HELO mail-lj1-f194.google.com) (209.85.208.194) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 12 Aug 2019 10:39:41 +0000 Received: by mail-lj1-f194.google.com with SMTP id e27so2685653ljb.7 for ; Mon, 12 Aug 2019 03:39:41 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=8FQImT6XWbzhPubc/lOW2C1ZYkuur4DnHUf71+9T9x0=; b=lftukObuPtJy4a/hzWqrtobqNJIGwseZkTCfsPq9gJWjSQTHbiAE6inEtQaTU8/6eI KqcKkaMO8bXOdRAQU14LN9yAN8eaqkVHfjH4MgJrpq/Gyp09ZtrsS/xONErUVKr4eGsx x2jO5TZ8+OaTD727QBJpTtLoJ7ki3Q8ezLVh+UfLxXxck9Q/EsZMKm8sBojZQ6QbK2x6 VIO/VVzNnb7PcS3i3oytUVvznvD/4f7lrj30C4HZK0JYde87YE7rgR57RLqkm3mib8w0 hb7wrD4JwcxVjCFrSZ3RI6FC3fBY7be00xOAXLA5bPtleis7VY71WsGw1teo7foBJUpt XBwQ== MIME-Version: 1.0 References: <55b4acda-9673-557b-5819-50bff07fa095@suse.cz> <0e977dd4-c9ff-ed24-fbe0-fa6993a9a14e@suse.cz> In-Reply-To: <0e977dd4-c9ff-ed24-fbe0-fa6993a9a14e@suse.cz> From: Richard Biener Date: Mon, 12 Aug 2019 10:46:00 -0000 Message-ID: Subject: Re: [PATCH] Prevent LTO section collision for a symbol name starting with '*'. To: =?UTF-8?Q?Martin_Li=C5=A1ka?= Cc: GCC Patches , Jan Hubicka Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes X-SW-Source: 2019-08/txt/msg00724.txt.bz2 On Mon, Aug 12, 2019 at 11:57 AM Martin Li=C5=A1ka wrote: > > On 8/12/19 11:45 AM, Richard Biener wrote: > > On Fri, Aug 9, 2019 at 3:57 PM Martin Li=C5=A1ka wrote: > >> > >> Hi. > >> > >> The patch is about prevention of LTO section name clashing. > >> Now we have a situation where body of 2 functions is streamed > >> into the same ELF section. Then we'll end up with smashed data. > >> > >> Patch can bootstrap on x86_64-linux-gnu and survives regression tests. > >> > >> Ready to be installed? > > > > I think the comment should mention why we skip a leading '*' > > at all. > > git blame helps us here: > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=3D42531 > > > > IIRC this is some target mangling applied to DECL_ASSEMBLER_NAME? > > Yes, it's prepended here: > set_user_assembler_name > > > And section names cannot contain '*'? > > As seen in the PR, not. > > > Or do we need to actually > > indentify '*fn' and 'fn' as the same? > > No, they should be identified as different symbols. > > > For the testcase what is the clashing > > symbol? > > void __open_alias(int, ...) __asm__("open"); - aka *open > and: > +extern __inline __attribute__((__gnu_inline__)) int open() {} Hmm, so for void __open_alias(int, ...) __asm__("open"); void __open_alias(int flags, ...) {} a non-LTO compile will output __open_alias with the symbol "open". Then we have extern __inline __attribute__((__gnu_inline__)) int open() {} which is the "other" body for 'open' but it shouldn't really be output to the symbol table. Still we want to emit its body for the purpose of inlining. So IMHO the fix is not to do magic '0' appending for the user-asm-name case but instead "mangling" of extern inline bodies because those are the speciality causing the issue in the end. > > > Can't we have many so that using "0" always is broken as well? > > If I'll define 2 symbols that alias to a same asm name, I'll get: > $ cat clash.c > void __open_alias(int, ...) __asm__("open"); > void __open_alias2(int, ...) __asm__("open"); > void __open_alias(int flags, ...) {} > void __open_alias2(int flags, ...) {} > extern __inline __attribute__((__gnu_inline__)) int open() {} > struct { > void *func; > } a =3D {open}; > > int main() > { > return 0; > } > > $ gcc clash.c -flto > lto1: fatal error: missing resolution data for *open > compilation terminated. > > Which is a reasonable error message to me. Here I don't agree, earlier diagnostic of such invalid testcase would be welcome instead. If you build w/o LTO you'll get /tmp/ccjjlMhr.s: Assembler messages: /tmp/ccjjlMhr.s:40: Error: symbol `open' is already defined IMHO this is invalid (undiagnosed) C. Richard. > Martin > > > > > Richard. > > > >> Thanks, > >> Martin > >> > >> gcc/ChangeLog: > >> > >> 2019-08-09 Martin Liska > >> > >> PR lto/91393 > >> PR lto/88220 > >> * lto-streamer.c (lto_get_section_name): Replace '*' leading > >> character with '0'. > >> > >> gcc/testsuite/ChangeLog: > >> > >> 2019-08-09 Martin Liska > >> > >> PR lto/91393 > >> PR lto/88220 > >> * gcc.dg/lto/pr91393_0.c: New test. > >> --- > >> gcc/lto-streamer.c | 15 ++++++++++++--- > >> gcc/testsuite/gcc.dg/lto/pr91393_0.c | 11 +++++++++++ > >> 2 files changed, 23 insertions(+), 3 deletions(-) > >> create mode 100644 gcc/testsuite/gcc.dg/lto/pr91393_0.c > >> > >> >