On Mon, Aug 12, 2019 at 12:39 PM Richard Biener <richard.guent...@gmail.com> wrote: > > On Mon, Aug 12, 2019 at 11:57 AM Martin Liška <mli...@suse.cz> wrote: > > > > On 8/12/19 11:45 AM, Richard Biener wrote: > > > On Fri, Aug 9, 2019 at 3:57 PM Martin Liška <mli...@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. > > > > git blame helps us here: > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=42531 > > > > > > > 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 = {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 <mli...@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 <mli...@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 > > >> > > >> > >
