On Tue, Sep 11, 2012 at 3:30 PM, Michael Matz <m...@suse.de> wrote:
> Hi,
>
> On Tue, 11 Sep 2012, Richard Guenther wrote:
>
>> >>> +++ gcc/lto/lto.c (working copy)
>> >>> @@ -1559,8 +1559,6 @@ lto_fixup_prevailing_decls (tree t)
>> >>> {
>> >>> enum tree_code code = TREE_CODE (t);
>> >>> LTO_NO_PREVAIL (TREE_TYPE (t));
>> >>> - if (CODE_CONTAINS_STRUCT (code, TS_COMMON))
>> >>> - LTO_NO_PREVAIL (TREE_CHAIN (t));
>> >>
>> >> That change is odd. Can you show us how it breaks?
>> >
>> > This will break LTO build of gcc.c-torture/execute/pr38051.c
>> >
>> > There is data structure like:
>> >
>> > union { long int l; char c[sizeof (long int)]; } u;
>> >
>> > Once the block info is reserved for this, it'll reserve this data
>> > structure. And inside this data structure, there is VAR_DECL. Thus
>> > LTO_NO_PREVAIL assertion does not satisfy here for TREE_CHAIN (t).
>>
>> I see - the issue here is that this data structure is not reached at the
>> time we call free_lang_data (via find_decls_types_r).
>
> It should be reached just fine. The problem is that TREE_CHAIN of that
> union type contains random garbage (in this case the var_decl 'u'). This
> is not supposed to happen. It's set as part of reading back a BLOCK_VARS
> chain, so the type_decl itself is in such a chain (and 'u' is part of it
> via the TREE_CHAIN pointer).
>
> I have no idea why this is no problem without the patch. Possibly because
> of the hunk in remove_unused_scope_block_p that makes more blocks stay.
>
>> But maybe I do not understand "once the block info is reserved for
>> this".
>>
>> So the patch papers over an issue elsewhere I believe. Maybe Micha can
>> add some clarification here though, how BLOCK_VARS should be visible
>> here
>
> Hmm. Without the half-hearted tries to support debug info with LTO the
> block_vars list was no problem, it simply wouldn't be streamed. Now I
> think it is a problem, and we need to fix it up with the prevailing decls
> if there are multiple ones. I.e. instead of removing the two lines,
> replace LTO_NO_PREVAIL (TREE_CHAIN (t)) with LTO_SET_PREVAIL.
>
> This is quite unfortunate as we really rather want to make sure that
> TREE_CHAIN isn't randomly set to something. But as long as block_vars are
> implemented via TREE_CHAIN, and we want to preserve block_vars we don't
> have much choice :-(
I don't think we can fixup TREE_CHAIN - the things cannot be in multiple
lists after all. Unifying/fixing up would need to happen at a BLOCK level.
But as you say - only TYPE_DECLs should be in BLOCK_VARS, but never
global ones, so there would be nothing to replace. Which means we shouldn't
even try to merge those. Hmm.
Richard.
>
> Ciao,
> Michael.
