Re: [PATCH] Prevent LTO section collision for a symbol name starting with '*'.

[Richard Biener <richard dot guenther at gmail dot com>]

On Thu, Aug 15, 2019 at 4:33 PM Jan Hubicka <hubicka@ucw.cz> wrote:
>
> > On Fri, Aug 9, 2019 at 3:57 PM Martin Liška <mliska@suse.cz> 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.  IIRC this is some target mangling applied to DECL_ASSEMBLER_NAME?
> DECL_ASSEMBLER_NAME works in a way that if it starts with "*"
> it is copied verbatim to the linker ouptut.  If it is w/o "*"
> then user_label_prefix is applied first, see
> symbol_table::assembler_names_equal_p
>
> So if we skip "*" one can definitly construct testcases of different
> function names ending up in same section especially when
> user_label_prefix is non-empty, like on Windows I think it is "_".
>
> > And section names cannot contain '*'?  Or do we need to actually
> > indentify '*fn' and 'fn' as the same?  For the testcase what is the clashing
> > symbol?  Can't we have many so that using "0" always is broken as well?
>
> We may have duplicate symbols during the compile time->WPA streaming
> since we do not do lto-symtab at compile time and user can use asm names
> that way.  This is not limited to extern inlines, so it would be nice to
> make this work reliably. I also plan support for keeping multiple
> function bodies defined for one symbol in cases it is necessary (glibc
> checking, when optimization flags are mismatches) for WPA->ltrans
> streaming.
>
> I was always considering option to simply use node->order ids to stream
> sections.  Because of way node->order is merged we area always able to
> recover the ID.

Heh, that sounds like a nice idea indeed.

>
> It is however kind of nice to see actual names in the objdump
> of the LTO .o files.  I would not mind that much to see this go and it
> would also save bit of space since symbol names can be long.
>
> Honza
> >
> > Richard.
> >
> > > Thanks,
> > > Martin
> > >
> > > gcc/ChangeLog:
> > >
> > > 2019-08-09  Martin Liska  <mliska@suse.cz>
> > >
> > >         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  <mliska@suse.cz>
> > >
> > >         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
> > >
> > >