Re: Fix uninitialized memory access in WHOPR partitioning

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

On Fri, Jun 4, 2010 at 7:36 AM, Jan Hubicka <hubicka@ucw.cz> wrote:
> Hi,
> today I got two teststuite failures with WHOPR. ?Those all have just datastructures
> and not functions in their WHOPR units. ?This leads to uninitialied memory
> access into lto_1_to_1_map since file_data variable is not initialized in varpool
> processing loop (so till now we get all varpool nodes in the same partition
> as last cgraph node got).
>
> This patch adds symemtric lto_file_data filed to varpool node as we do have for
> cgraph nodes. ?I am not thrilled by the existence of lto_file_data pointer at
> all here, isn't there some alternative to look up back file name from declaration?

I just posted exactly the same patch minor ....

> Honza
>
> ? ? ? ?* cgraph.h (struct varpool_node): Add lto_file_data.
> ? ? ? ?* lto-cgraph.c (input_varpool_node): Set file_data.
> ? ? ? ?* lto/lto.c (lto_1_to_1_map): Use it.
> Index: cgraph.h
> ===================================================================
> --- cgraph.h ? ?(revision 160239)
> +++ cgraph.h ? ?(working copy)
> @@ -467,6 +467,9 @@ struct GTY((chain_next ("%h.next"), chai
> ? struct varpool_node *same_comdat_group;
> ? struct ipa_ref_list ref_list;
> ? PTR GTY ((skip)) aux;
> + ?/* File stream where this node is being written to. ?*/
> + ?struct lto_file_decl_data * lto_file_data;
> +
> ? /* Ordering of all cgraph nodes. ?*/
> ? int order;
>
> Index: lto-cgraph.c
> ===================================================================
> --- lto-cgraph.c ? ? ? ?(revision 160239)
> +++ lto-cgraph.c ? ? ? ?(working copy)
> @@ -1091,6 +1091,7 @@ input_varpool_node (struct lto_file_decl
> ? node = varpool_node (var_decl);
>
> ? bp = lto_input_bitpack (ib);
> + ?node->lto_file_data = file_data;
> ? node->externally_visible = bp_unpack_value (bp, 1);
> ? node->force_output = bp_unpack_value (bp, 1);
> ? node->finalized = bp_unpack_value (bp, 1);
> Index: lto/lto.c
> ===================================================================
> --- lto/lto.c ? (revision 160239)
> +++ lto/lto.c ? (working copy)
> @@ -599,6 +599,9 @@ lto_1_to_1_map (void)
> ? ? {
> ? ? ? if (vnode->alias || !vnode->needed)
> ? ? ? ?continue;
> + ? ? ?file_data = vnode->local.lto_file_data;
> + ? ? ?gcc_assert (file_data);
> + ? ? ?file_data = node->local.lto_file_data;

this - which looks bogus if there is no node.

So I guess my patch is ok then ...?

Richard.

> ? ? ? slot = pointer_map_contains (pmap, file_data);
> ? ? ? if (slot)
> ? ? ? ?partition = (ltrans_partition) *slot;
>