This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH, CHKP] Restore transparent alias chains
- From: Ilya Enkovich <enkovich dot gnu at gmail dot com>
- To: Jan Hubicka <hubicka at ucw dot cz>
- Cc: Jeff Law <law at redhat dot com>, gcc-patches <gcc-patches at gcc dot gnu dot org>
- Date: Fri, 3 Apr 2015 11:18:41 +0300
- Subject: Re: [PATCH, CHKP] Restore transparent alias chains
- Authentication-results: sourceware.org; auth=none
- References: <20150320082033 dot GG64546 at msticlxl57 dot ims dot intel dot com> <551D8B66 dot 90601 at redhat dot com> <20150402184804 dot GE21276 at atrey dot karlin dot mff dot cuni dot cz>
2015-04-02 21:48 GMT+03:00 Jan Hubicka <hubicka@ucw.cz>:
>> On 03/20/2015 02:20 AM, Ilya Enkovich wrote:
>> >Hi,
>> >
>> >Identifiers read with input_identifier miss IDENTIFIER_TRANSPARENT_ALIAS bit. We always expect it for instrumentation clones, thus restore it input_cgraph_1. Bootstrapped and tested on x86_64-unknown-linux-gnu. OK for trunk?
>> >
>> >Thanks,
>> >Ilya
>> >--
>> >2015-03-20 Ilya Enkovich <ilya.enkovich@intel.com>
>> >
>> > * lto-cgraph.c (input_cgraph_1): Always link instrumented
>> > assembler name with original one.
>> This appears to be a code path that only triggers when MPX is
>> enabled and is roughly analogous to the code in
>> chkp_build_instrumented_fndecl links things up.
>>
>> OK for the trunk.
>
> I think this will lead to wrong code. At this time, we may have multple
> declarations sharing single assembler name (and thus
> IDENTIFIER_TRANSPARENT_ALIAS link). Think of case where one unit defines static
> function and other global function of the same name. We may end up renaming the
> symbol but keeping bogus transparent alias link that will trigger on other
> symbol.
That is the reason for fixing chains in privatize_symbol_name.
>
> During WPA the assembler names are never fully unique, but we also probably do
> not need to set IDENTIFIER_TRANSPARENT_ALIAS.
>
> I think IDENTIFIER_TRANSPARENT_ALIAS should be set in rename_statics and
> separately in ltrans on the place we skip lto_symtab merging. At least it is
> the place I link it with my patch for supporting transparent aliases in cgraph.
>
> I will try to find time to audit chkp code - I missed the addition of
> transparent aliases in the initial chkp patches. Symbol table code expect 1-1
> correspondence between symbol table entries and real symbols. Basically all
> places we special case aliases or weakrefs needs to be revisited for
> transparent aliases.
I don't see how 1-1 matching may be achieved now for instrumented
functions. We have two cgraph_nodes which actually represent the same
function. Thus single real symbol for two nodes.
Thanks,
Ilya
>
> Honza