From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 121210 invoked by alias); 12 Aug 2019 10:46:16 -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 120920 invoked by uid 89); 12 Aug 2019 10:46:16 -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 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:46:14 +0000 Received: by mail-lj1-f194.google.com with SMTP id x4so5718204ljj.6 for ; Mon, 12 Aug 2019 03:46:13 -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=WvE5JJi5LfroYQrDiHg8xBUTyQmfz4sp+Rl1+VdFgrs=; b=W97QlsAUjNtyzmp2ZmH2qbfsB3AfxeODtjQ16+z2+lgdtLtS1F+/OlmAJTVNnHBgYj 30iPlsjnHrK3SHdkOCuGfBoo/mFsAOxd/grqJg7ZjcasMyA6uXsLPs2kVT8lXXrxxE/m s5Gq5i1fjdbH8YT9PyLSjnb0OYW8EXp7PCoFNwp+73blKXviJzNXtd1g78mtLSSxJVZC KfSAVYfISgpJtcam4vndcdnx/U6lUscYwn8Gvyho15b5EghBd5Tf1IrRpk7y4jQMsioq P7cVHkGfcylKTHEZJjgljudOEvEVcgaZx2ov+u9ULPEs+fMo9IpnHD0jOd62JOcJb5Yk 1A+w== MIME-Version: 1.0 References: <55b4acda-9673-557b-5819-50bff07fa095@suse.cz> <0e977dd4-c9ff-ed24-fbe0-fa6993a9a14e@suse.cz> In-Reply-To: From: Richard Biener Date: Mon, 12 Aug 2019 10:52: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/msg00726.txt.bz2 On Mon, Aug 12, 2019 at 12:39 PM Richard Biener wrote: > > 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 wro= te: > > >> > > >> 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 test= s. > > >> > > >> 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. Btw, with -flto we also only get a single function section for both (not sure if the bytecode ends up sensible). > 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 > > >> > > >> > >