On 8/15/19 4:33 PM, Jan Hubicka 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. 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.
Looks good to me. The only small issue is that in lto-cgraph.c: 1220 const char *section; 1221 order = streamer_read_hwi (ib) + order_base; 1222 clone_ref = streamer_read_hwi (ib); ... 1245 node->order = order; 1246 if (order >= symtab->order) 1247 symtab->order = order + 1; So in WPA, we shift order by order_base. For streaming purpose I'll need something like symtab_node::lgen_order that will be streamer_read_hwi (ib). Are you fine with that? > > 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. Or we can mix name.order as a section name? Martin > > Honza >> >> 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 >>> >>>
