diff mbox

[pph] Free buffers used during tree encoding/decoding

Message ID 20110728175040.GA15491@google.com
State New
Headers show

Commit Message

Diego Novillo July 28, 2011, 5:50 p.m. UTC
Noticed this while debugging the new tree encoding cache.  No
functional changes.  This frees the memory used by the buffers
used during tree streaming.  It also moves the reader and writer
data into a union to better distinguish them.

Tested on x86_64.


Diego.


	* pph-streamer.h (pph_stream): Move fields OB, OUT_STATE,
	DECL_STATE_STREAM, IB, DATA_IN, PPH_SECTIONS, FILE_DATA and
	FILE_SIZE into a union of structures.
	Update all users.
	* pph-streamer.c (pph_stream_close): Free memory used by tree
	encoding routines.

Comments

Lawrence Crowl July 28, 2011, 11:30 p.m. UTC | #1
I'm getting massive failures after incorporating this change:

   bytecode stream: trying to read 1735 bytes after the end of the
   input buffer

where the number of bytes changes.  Suggestions?


On 7/28/11, Diego Novillo <dnovillo@google.com> wrote:
> Noticed this while debugging the new tree encoding cache.  No
> functional changes.  This frees the memory used by the buffers
> used during tree streaming.  It also moves the reader and writer
> data into a union to better distinguish them.
>
> Tested on x86_64.
>
>
> Diego.
>
>
> 	* pph-streamer.h (pph_stream): Move fields OB, OUT_STATE,
> 	DECL_STATE_STREAM, IB, DATA_IN, PPH_SECTIONS, FILE_DATA and
> 	FILE_SIZE into a union of structures.
> 	Update all users.
> 	* pph-streamer.c (pph_stream_close): Free memory used by tree
> 	encoding routines.
>
> Index: cp/pph-streamer-in.c
> ===================================================================
> --- cp/pph-streamer-in.c	(revision 176879)
> +++ cp/pph-streamer-in.c	(working copy)
> @@ -109,8 +109,8 @@ pph_get_section_data (struct lto_file_de
>  {
>    /* FIXME pph - Stop abusing lto_file_decl_data fields.  */
>    const pph_stream *stream = (const pph_stream *) file_data->file_name;
> -  *len = stream->file_size - sizeof (pph_file_header);
> -  return (const char *) stream->file_data + sizeof (pph_file_header);
> +  *len = stream->encoder.r.file_size - sizeof (pph_file_header);
> +  return (const char *) stream->encoder.r.file_data + sizeof
> (pph_file_header);
>  }
>
>
> @@ -119,14 +119,14 @@ pph_get_section_data (struct lto_file_de
>
>  static void
>  pph_free_section_data (struct lto_file_decl_data *file_data,
> -		   enum lto_section_type section_type ATTRIBUTE_UNUSED,
> -		   const char *name ATTRIBUTE_UNUSED,
> -		   const char *offset ATTRIBUTE_UNUSED,
> -		   size_t len ATTRIBUTE_UNUSED)
> +		       enum lto_section_type section_type ATTRIBUTE_UNUSED,
> +		       const char *name ATTRIBUTE_UNUSED,
> +		       const char *offset ATTRIBUTE_UNUSED,
> +		       size_t len ATTRIBUTE_UNUSED)
>  {
>    /* FIXME pph - Stop abusing lto_file_decl_data fields.  */
>    const pph_stream *stream = (const pph_stream *) file_data->file_name;
> -  free (stream->file_data);
> +  free (stream->encoder.r.file_data);
>  }
>
>
> @@ -145,46 +145,48 @@ pph_init_read (pph_stream *stream)
>
>    lto_reader_init ();
>
> -  /* Read STREAM->NAME into the memory buffer STREAM->FILE_DATA.
> -     FIXME pph, we are reading the whole file at once.  This seems
> -     wasteful.  */
> +  /* Read STREAM->NAME into the memory buffer stream->encoder.r.file_data.
> */
>    retcode = fstat (fileno (stream->file), &st);
>    gcc_assert (retcode == 0);
> -  stream->file_size = (size_t) st.st_size;
> -  stream->file_data = XCNEWVEC (char, stream->file_size);
> -  bytes_read = fread (stream->file_data, 1, stream->file_size,
> stream->file);
> -  gcc_assert (bytes_read == stream->file_size);
> +  stream->encoder.r.file_size = (size_t) st.st_size;
> +  stream->encoder.r.file_data = XCNEWVEC (char,
> stream->encoder.r.file_size);
> +  bytes_read = fread (stream->encoder.r.file_data, 1,
> +		      stream->encoder.r.file_size, stream->file);
> +  gcc_assert (bytes_read == stream->encoder.r.file_size);
>
>    /* Set LTO callbacks to read the PPH file.  */
> -  stream->pph_sections = XCNEWVEC (struct lto_file_decl_data *,
> -				   PPH_NUM_SECTIONS);
> +  stream->encoder.r.pph_sections = XCNEWVEC (struct lto_file_decl_data *,
> +					     PPH_NUM_SECTIONS);
>    for (i = 0; i < PPH_NUM_SECTIONS; i++)
>      {
> -      stream->pph_sections[i] = XCNEW (struct lto_file_decl_data);
> +      stream->encoder.r.pph_sections[i] = XCNEW (struct
> lto_file_decl_data);
>        /* FIXME pph - Stop abusing fields in lto_file_decl_data.  */
> -      stream->pph_sections[i]->file_name = (const char *) stream;
> +      stream->encoder.r.pph_sections[i]->file_name = (const char *) stream;
>      }
>
> -  lto_set_in_hooks (stream->pph_sections, pph_get_section_data,
> +  lto_set_in_hooks (stream->encoder.r.pph_sections, pph_get_section_data,
>  		    pph_free_section_data);
>
> -  header = (pph_file_header *) stream->file_data;
> +  header = (pph_file_header *) stream->encoder.r.file_data;
>    strtab = (const char *) header + sizeof (pph_file_header);
>    strtab_size = header->strtab_size;
>    body = strtab + strtab_size;
> -  gcc_assert (stream->file_size >= strtab_size + sizeof (pph_file_header));
> -  body_size = stream->file_size - strtab_size - sizeof (pph_file_header);
> +  gcc_assert (stream->encoder.r.file_size
> +	      >= strtab_size + sizeof (pph_file_header));
> +  body_size = stream->encoder.r.file_size
> +	      - strtab_size - sizeof (pph_file_header);
>
>    /* Create an input block structure pointing right after the string
>       table.  */
> -  stream->ib = XCNEW (struct lto_input_block);
> -  LTO_INIT_INPUT_BLOCK_PTR (stream->ib, body, 0, body_size);
> -  stream->data_in = lto_data_in_create (stream->pph_sections[0], strtab,
> -                                        strtab_size, NULL);
> +  stream->encoder.r.ib = XCNEW (struct lto_input_block);
> +  LTO_INIT_INPUT_BLOCK_PTR (stream->encoder.r.ib, body, 0, body_size);
> +  stream->encoder.r.data_in
> +      = lto_data_in_create (stream->encoder.r.pph_sections[0], strtab,
> +			    strtab_size, NULL);
>
> -  /* Associate STREAM with STREAM->DATA_IN so we can recover it from
> +  /* Associate STREAM with STREAM->ENCODER.R.DATA_IN so we can recover it
> from
>       the streamer hooks.  */
> -  stream->data_in->sdata = (void *) stream;
> +  stream->encoder.r.data_in->sdata = (void *) stream;
>  }
>
>
> @@ -827,7 +829,8 @@ pph_in_struct_function (pph_stream *stre
>    allocate_struct_function (decl, false);
>    fn = DECL_STRUCT_FUNCTION (decl);
>
> -  input_struct_function_base (fn, stream->data_in, stream->ib);
> +  input_struct_function_base (fn, stream->encoder.r.data_in,
> +			      stream->encoder.r.ib);
>
>    /* struct eh_status *eh;					-- zero init */
>    /* struct control_flow_graph *cfg;				-- zero init */
> Index: cp/pph-streamer.c
> ===================================================================
> --- cp/pph-streamer.c	(revision 176879)
> +++ cp/pph-streamer.c	(working copy)
> @@ -158,6 +158,25 @@ pph_stream_close (pph_stream *stream)
>    stream->file = NULL;
>    VEC_free (void_p, heap, stream->cache.v);
>    pointer_map_destroy (stream->cache.m);
> +
> +  if (stream->write_p)
> +    {
> +      destroy_output_block (stream->encoder.w.ob);
> +      free (stream->encoder.w.decl_state_stream);
> +      lto_delete_out_decl_state (stream->encoder.w.out_state);
> +    }
> +  else
> +    {
> +      unsigned i;
> +
> +      free (stream->encoder.r.ib);
> +      lto_data_in_delete (stream->encoder.r.data_in);
> +      for (i = 0; i < PPH_NUM_SECTIONS; i++)
> +	free (stream->encoder.r.pph_sections[i]);
> +      free (stream->encoder.r.pph_sections);
> +      free (stream->encoder.r.file_data);
> +    }
> +
>    free (stream);
>  }
>
> Index: cp/pph-streamer-out.c
> ===================================================================
> --- cp/pph-streamer-out.c	(revision 176879)
> +++ cp/pph-streamer-out.c	(working copy)
> @@ -99,14 +99,14 @@ void
>  pph_init_write (pph_stream *stream)
>  {
>    lto_streamer_init ();
> -  stream->out_state = lto_new_out_decl_state ();
> -  lto_push_out_decl_state (stream->out_state);
> -  stream->decl_state_stream = XCNEW (struct lto_output_stream);
> -  stream->ob = create_output_block (LTO_section_decls);
> +  stream->encoder.w.out_state = lto_new_out_decl_state ();
> +  lto_push_out_decl_state (stream->encoder.w.out_state);
> +  stream->encoder.w.decl_state_stream = XCNEW (struct lto_output_stream);
> +  stream->encoder.w.ob = create_output_block (LTO_section_decls);
>
> -  /* Associate STREAM with STREAM->OB so we can recover it from the
> +  /* Associate STREAM with stream->encoder.w.ob so we can recover it from
> the
>       streamer hooks.  */
> -  stream->ob->sdata = (void *) stream;
> +  stream->encoder.w.ob->sdata = (void *) stream;
>  }
>
>
> @@ -158,7 +158,7 @@ pph_out_header (pph_stream *stream)
>    header.major_version = (char) major;
>    header.minor_version = (char) minor;
>    header.patchlevel = (char) patchlevel;
> -  header.strtab_size = stream->ob->string_stream->total_size;
> +  header.strtab_size = stream->encoder.w.ob->string_stream->total_size;
>
>    memset (&header_stream, 0, sizeof (header_stream));
>    lto_output_data_stream (&header_stream, &header, sizeof (header));
> @@ -172,18 +172,20 @@ static void
>  pph_out_body (pph_stream *stream)
>  {
>    /* Write the string table.  */
> -  lto_write_stream (stream->ob->string_stream);
> +  lto_write_stream (stream->encoder.w.ob->string_stream);
>
>    /* Write out the physical representation for every AST in all the
> -     streams in STREAM->OUT_STATE.  */
> -  lto_output_decl_state_streams (stream->ob, stream->out_state);
> +     streams in STREAM->ENCODER.W.OUT_STATE.  */
> +  lto_output_decl_state_streams (stream->encoder.w.ob,
> +				 stream->encoder.w.out_state);
>
>    /* Now write the vector of all AST references.  */
> -  lto_output_decl_state_refs (stream->ob, stream->decl_state_stream,
> -			      stream->out_state);
> +  lto_output_decl_state_refs (stream->encoder.w.ob,
> +			      stream->encoder.w.decl_state_stream,
> +			      stream->encoder.w.out_state);
>
>    /* Finally, physically write all the streams.  */
> -  lto_write_stream (stream->ob->main_stream);
> +  lto_write_stream (stream->encoder.w.ob->main_stream);
>  }
>
>
> @@ -455,7 +457,7 @@ pph_out_ld_base (pph_stream *stream, str
>  {
>    struct bitpack_d bp;
>
> -  bp = bitpack_create (stream->ob->main_stream);
> +  bp = bitpack_create (stream->encoder.w.ob->main_stream);
>    bp_pack_value (&bp, ldb->selector, 16);
>    bp_pack_value (&bp, ldb->language, 4);
>    bp_pack_value (&bp, ldb->use_template, 2);
> @@ -537,7 +539,7 @@ pph_out_cxx_binding_1 (pph_stream *strea
>    pph_out_tree_or_ref (stream, cb->value);
>    pph_out_tree_or_ref (stream, cb->type);
>    pph_out_binding_level (stream, cb->scope);
> -  bp = bitpack_create (stream->ob->main_stream);
> +  bp = bitpack_create (stream->encoder.w.ob->main_stream);
>    bp_pack_value (&bp, cb->value_is_inherited, 1);
>    bp_pack_value (&bp, cb->is_local, 1);
>    pph_out_bitpack (stream, &bp);
> @@ -684,7 +686,7 @@ pph_out_binding_level (pph_stream *strea
>    pph_out_chain (stream, bl->statement_list);
>    pph_out_uint (stream, bl->binding_depth);
>
> -  bp = bitpack_create (stream->ob->main_stream);
> +  bp = bitpack_create (stream->encoder.w.ob->main_stream);
>    bp_pack_value (&bp, bl->kind, 4);
>    bp_pack_value (&bp, bl->keep, 1);
>    bp_pack_value (&bp, bl->more_cleanups_ok, 1);
> @@ -735,7 +737,7 @@ pph_out_language_function (pph_stream *s
>    pph_out_tree_or_ref (stream, lf->x_in_charge_parm);
>    pph_out_tree_or_ref (stream, lf->x_vtt_parm);
>    pph_out_tree_or_ref (stream, lf->x_return_value);
> -  bp = bitpack_create (stream->ob->main_stream);
> +  bp = bitpack_create (stream->encoder.w.ob->main_stream);
>    bp_pack_value (&bp, lf->x_returns_value, 1);
>    bp_pack_value (&bp, lf->x_returns_null, 1);
>    bp_pack_value (&bp, lf->x_returns_abnormally, 1);
> @@ -763,7 +765,7 @@ pph_out_ld_fn (pph_stream *stream, struc
>    /* Write all the fields in lang_decl_min.  */
>    pph_out_ld_min (stream, &ldf->min);
>
> -  bp = bitpack_create (stream->ob->main_stream);
> +  bp = bitpack_create (stream->encoder.w.ob->main_stream);
>    bp_pack_value (&bp, ldf->operator_code, 16);
>    bp_pack_value (&bp, ldf->global_ctor_p, 1);
>    bp_pack_value (&bp, ldf->global_dtor_p, 1);
> @@ -809,7 +811,7 @@ struct pph_tree_info {
>  static int
>  pph_out_used_types_slot (void **slot, void *aux)
>  {
> -  struct pph_tree_info *pti = (struct pph_tree_info *)aux;
> +  struct pph_tree_info *pti = (struct pph_tree_info *) aux;
>    pph_out_tree_or_ref (pti->stream, (tree) *slot);
>    return 1;
>  }
> @@ -826,7 +828,7 @@ pph_out_struct_function (pph_stream *str
>      return;
>
>    pph_out_tree (stream, fn->decl);
> -  output_struct_function_base (stream->ob, fn);
> +  output_struct_function_base (stream->encoder.w.ob, fn);
>
>    /* struct eh_status *eh;					-- ignored */
>    gcc_assert (fn->cfg == NULL);
> @@ -842,14 +844,14 @@ pph_out_struct_function (pph_stream *str
>    /* struct machine_function *machine;				-- ignored */
>    pph_out_language_function (stream, fn->language);
>
> -  /*FIXME pph: We would like to detect improper sharing here.  */
> +  /* FIXME pph: We would like to detect improper sharing here.  */
>    if (fn->used_types_hash)
>      {
> -      /*FIXME pph: This write may be unstable.  */
> +      /* FIXME pph: This write may be unstable.  */
>        pph_out_uint (stream, htab_elements (fn->used_types_hash));
>        pti.stream = stream;
> -      htab_traverse_noresize (fn->used_types_hash,
> -			      pph_out_used_types_slot, &pti);
> +      htab_traverse_noresize (fn->used_types_hash, pph_out_used_types_slot,
> +			      &pti);
>      }
>    else
>      pph_out_uint (stream, 0);
> @@ -943,12 +945,11 @@ pph_out_lang_specific (pph_stream *strea
>  /* Write all the fields in lang_type_header instance LTH to STREAM.  */
>
>  static void
> -pph_out_lang_type_header (pph_stream *stream,
> -				   struct lang_type_header *lth)
> +pph_out_lang_type_header (pph_stream *stream, struct lang_type_header *lth)
>  {
>    struct bitpack_d bp;
>
> -  bp = bitpack_create (stream->ob->main_stream);
> +  bp = bitpack_create (stream->encoder.w.ob->main_stream);
>    bp_pack_value (&bp, lth->is_lang_type_class, 1);
>    bp_pack_value (&bp, lth->has_type_conversion, 1);
>    bp_pack_value (&bp, lth->has_copy_ctor, 1);
> @@ -1002,7 +1003,7 @@ pph_out_lang_type_class (pph_stream *str
>
>    pph_out_uchar (stream, ltc->align);
>
> -  bp = bitpack_create (stream->ob->main_stream);
> +  bp = bitpack_create (stream->encoder.w.ob->main_stream);
>    bp_pack_value (&bp, ltc->has_mutable, 1);
>    bp_pack_value (&bp, ltc->com_interface, 1);
>    bp_pack_value (&bp, ltc->non_pod_class, 1);
> Index: cp/pph-streamer.h
> ===================================================================
> --- cp/pph-streamer.h	(revision 176879)
> +++ cp/pph-streamer.h	(working copy)
> @@ -93,6 +93,8 @@ typedef struct pph_pickle_cache {
>  } pph_pickle_cache;
>
>
> +/* Data structures used to encode and decode trees.  */
> +
>  /* A PPH stream contains all the data and attributes needed to
>     write symbols, declarations and other parsing products to disk.  */
>  typedef struct pph_stream {
> @@ -102,27 +104,24 @@ typedef struct pph_stream {
>    /* FILE object associated with it.  */
>    FILE *file;
>
> -  /* LTO output block to hold pickled ASTs and references.  This is
> -      NULL when the file is opened for reading.  */
> -  struct output_block *ob;
> -  struct lto_out_decl_state *out_state;
> -  struct lto_output_stream *decl_state_stream;
> -
> -  /* LTO input block to read ASTs and references from.  This is NULL
> -      when the file is opened for writing.  */
> -  struct lto_input_block *ib;
> -
> -  /* String tables and other descriptors used by the LTO reading
> -     routines.  NULL when the file is opened for writing.  */
> -  struct data_in *data_in;
> -
> -  /* Array of sections in the PPH file.  */
> -  struct lto_file_decl_data **pph_sections;
> +  /* Data structures used to encode/decode trees.  */
> +  union {
> +    /* Encoding tables and buffers used to write trees to a file.  */
> +    struct {
> +	struct output_block *ob;
> +	struct lto_out_decl_state *out_state;
> +	struct lto_output_stream *decl_state_stream;
> +    } w;
>
> -  /* Buffer holding the file contents.  FIXME pph, we are bringing
> -     the whole file in memory at once.  This seems wasteful.  */
> -  char *file_data;
> -  size_t file_size;
> +    /* Decoding tables and buffers used to read trees from a file.  */
> +    struct {
> +	struct lto_input_block *ib;
> +	struct data_in *data_in;
> +	struct lto_file_decl_data **pph_sections;
> +	char *file_data;
> +	size_t file_size;
> +    } r;
> +  } encoder;
>
>    /* Cache of pickled data structures.  */
>    pph_pickle_cache cache;
> @@ -186,7 +185,7 @@ pph_out_tree (pph_stream *stream, tree t
>  {
>    if (flag_pph_tracer >= 1)
>      pph_trace_tree (stream, t);
> -  lto_output_tree (stream->ob, t, false);
> +  lto_output_tree (stream->encoder.w.ob, t, false);
>  }
>
>  /* Output array A of cardinality C of ASTs to STREAM.  */
> @@ -200,7 +199,7 @@ pph_out_tree_array (pph_stream *stream,
>      {
>        if (flag_pph_tracer >= 1)
>          pph_trace_tree (stream, a[i]);
> -      lto_output_tree (stream->ob, a[i]);
> +      lto_output_tree (stream->encoder.w.ob, a[i]);
>      }
>  }
>  #endif
> @@ -212,7 +211,7 @@ pph_out_tree_or_ref_1 (pph_stream *strea
>  {
>    if (flag_pph_tracer >= tlevel)
>      pph_trace_tree (stream, t);
> -  lto_output_tree (stream->ob, t, false);
> +  lto_output_tree (stream->encoder.w.ob, t, false);
>  }
>
>  /* Output AST T to STREAM.  Trigger tracing at -fpph-tracer=2.  */
> @@ -228,7 +227,7 @@ pph_out_uint (pph_stream *stream, unsign
>  {
>    if (flag_pph_tracer >= 4)
>      pph_trace_uint (stream, value);
> -  lto_output_sleb128_stream (stream->ob->main_stream, value);
> +  lto_output_sleb128_stream (stream->encoder.w.ob->main_stream, value);
>  }
>
>  /* Write an unsigned char VALUE to STREAM.  */
> @@ -237,7 +236,7 @@ pph_out_uchar (pph_stream *stream, unsig
>  {
>    if (flag_pph_tracer >= 4)
>      pph_trace_uint (stream, value);
> -  lto_output_1_stream (stream->ob->main_stream, value);
> +  lto_output_1_stream (stream->encoder.w.ob->main_stream, value);
>  }
>
>  /* Write N bytes from P to STREAM.  */
> @@ -246,7 +245,7 @@ pph_out_bytes (pph_stream *stream, const
>  {
>    if (flag_pph_tracer >= 4)
>      pph_trace_bytes (stream, p, n);
> -  lto_output_data_stream (stream->ob->main_stream, p, n);
> +  lto_output_data_stream (stream->encoder.w.ob->main_stream, p, n);
>  }
>
>  /* Write string STR to STREAM.  */
> @@ -255,18 +254,20 @@ pph_out_string (pph_stream *stream, cons
>  {
>    if (flag_pph_tracer >= 4)
>      pph_trace_string (stream, str);
> -  lto_output_string (stream->ob, stream->ob->main_stream, str, false);
> +  lto_output_string (stream->encoder.w.ob,
> stream->encoder.w.ob->main_stream,
> +		     str, false);
>  }
>
>  /* Write string STR of length LEN to STREAM.  */
>  static inline void
>  pph_out_string_with_length (pph_stream *stream, const char *str,
> -			       unsigned int len)
> +			    unsigned int len)
>  {
>    if (flag_pph_tracer >= 4)
>      pph_trace_string_with_length (stream, str, len);
> -  lto_output_string_with_length (stream->ob, stream->ob->main_stream,
> -				  str, len + 1, false);
> +  lto_output_string_with_length (stream->encoder.w.ob,
> +				 stream->encoder.w.ob->main_stream,
> +				 str, len + 1, false);
>  }
>
>  /* Output VEC V of ASTs to STREAM.  */
> @@ -283,7 +284,7 @@ pph_out_tree_VEC (pph_stream *stream, VE
>      {
>        if (flag_pph_tracer >= 1)
>          pph_trace_tree (stream, t);
> -      lto_output_tree (stream->ob, t);
> +      lto_output_tree (stream->encoder.w.ob, t);
>      }
>  }
>  #endif
> @@ -294,7 +295,7 @@ pph_out_location (pph_stream *stream, lo
>  {
>    if (flag_pph_tracer >= 4)
>      pph_trace_location (stream, loc);
> -  lto_output_location (stream->ob, loc);
> +  lto_output_location (stream->encoder.w.ob, loc);
>  }
>
>  /* Write a chain of ASTs to STREAM starting with FIRST.  */
> @@ -303,14 +304,14 @@ pph_out_chain (pph_stream *stream, tree
>  {
>    if (flag_pph_tracer >= 2)
>      pph_trace_chain (stream, first);
> -  lto_output_chain (stream->ob, first, false);
> +  lto_output_chain (stream->encoder.w.ob, first, false);
>  }
>
>  /* Write a bitpack BP to STREAM.  */
>  static inline void
>  pph_out_bitpack (pph_stream *stream, struct bitpack_d *bp)
>  {
> -  gcc_assert (stream->ob->main_stream == bp->stream);
> +  gcc_assert (stream->encoder.w.ob->main_stream == bp->stream);
>    if (flag_pph_tracer >= 4)
>      pph_trace_bitpack (stream, bp);
>    lto_output_bitpack (bp);
> @@ -320,7 +321,7 @@ pph_out_bitpack (pph_stream *stream, str
>  static inline unsigned int
>  pph_in_uint (pph_stream *stream)
>  {
> -  HOST_WIDE_INT unsigned n = lto_input_uleb128 (stream->ib);
> +  HOST_WIDE_INT unsigned n = lto_input_uleb128 (stream->encoder.r.ib);
>    gcc_assert (n == (unsigned) n);
>    if (flag_pph_tracer >= 4)
>      pph_trace_uint (stream, n);
> @@ -331,7 +332,7 @@ pph_in_uint (pph_stream *stream)
>  static inline unsigned char
>  pph_in_uchar (pph_stream *stream)
>  {
> -  unsigned char n = lto_input_1_unsigned (stream->ib);
> +  unsigned char n = lto_input_1_unsigned (stream->encoder.r.ib);
>    if (flag_pph_tracer >= 4)
>      pph_trace_uint (stream, n);
>    return n;
> @@ -342,7 +343,7 @@ pph_in_uchar (pph_stream *stream)
>  static inline void
>  pph_in_bytes (pph_stream *stream, void *p, size_t n)
>  {
> -  lto_input_data_block (stream->ib, p, n);
> +  lto_input_data_block (stream->encoder.r.ib, p, n);
>    if (flag_pph_tracer >= 4)
>      pph_trace_bytes (stream, p, n);
>  }
> @@ -352,7 +353,8 @@ pph_in_bytes (pph_stream *stream, void *
>  static inline const char *
>  pph_in_string (pph_stream *stream)
>  {
> -  const char *s = lto_input_string (stream->data_in, stream->ib);
> +  const char *s = lto_input_string (stream->encoder.r.data_in,
> +				    stream->encoder.r.ib);
>    if (flag_pph_tracer >= 4)
>      pph_trace_string (stream, s);
>    return s;
> @@ -362,7 +364,8 @@ pph_in_string (pph_stream *stream)
>  static inline location_t
>  pph_in_location (pph_stream *stream)
>  {
> -  location_t loc = lto_input_location (stream->ib, stream->data_in);
> +  location_t loc = lto_input_location (stream->encoder.r.ib,
> +				       stream->encoder.r.data_in);
>    if (flag_pph_tracer >= 4)
>      pph_trace_location (stream, loc);
>    return loc;
> @@ -372,7 +375,7 @@ pph_in_location (pph_stream *stream)
>  static inline tree
>  pph_in_tree (pph_stream *stream)
>  {
> -  tree t = lto_input_tree (stream->ib, stream->data_in);
> +  tree t = lto_input_tree (stream->encoder.r.ib,
> stream->encoder.r.data_in);
>    if (flag_pph_tracer >= 4)
>      pph_trace_tree (stream, t);
>    return t;
> @@ -387,7 +390,7 @@ pph_in_tree_array (pph_stream *stream, t
>    size_t i;
>    for (i = 0; i < c; ++i)
>      {
> -      tree t = lto_input_tree (stream->ib, stream->data_in);
> +      tree t = lto_input_tree (stream->encoder.r.ib,
> stream->encoder.r.data_in);
>        if (flag_pph_tracer >= 4)
>          pph_trace_tree (stream, t, false); /* FIXME pph: always false? */
>        a[i] = t;
> @@ -405,7 +408,7 @@ pph_in_tree_VEC (pph_stream *stream, VEC
>    unsigned int c = pph_in_uint (stream);
>    for (i = 0; i < c; ++i)
>      {
> -      tree t = lto_input_tree (stream->ib, stream->data_in);
> +      tree t = lto_input_tree (stream->encoder.r.ib,
> stream->encoder.r.data_in);
>        if (flag_pph_tracer >= 4)
>          pph_trace_tree (stream, t, false); /* FIXME pph: always false? */
>        VEC_safe_push (tree, gc, v, t);
> @@ -417,7 +420,7 @@ pph_in_tree_VEC (pph_stream *stream, VEC
>  static inline tree
>  pph_in_chain (pph_stream *stream)
>  {
> -  tree t = lto_input_chain (stream->ib, stream->data_in);
> +  tree t = lto_input_chain (stream->encoder.r.ib,
> stream->encoder.r.data_in);
>    if (flag_pph_tracer >= 2)
>      pph_trace_chain (stream, t);
>    return t;
> @@ -427,7 +430,7 @@ pph_in_chain (pph_stream *stream)
>  static inline struct bitpack_d
>  pph_in_bitpack (pph_stream *stream)
>  {
> -  struct bitpack_d bp = lto_input_bitpack (stream->ib);
> +  struct bitpack_d bp = lto_input_bitpack (stream->encoder.r.ib);
>    if (flag_pph_tracer >= 4)
>      pph_trace_bitpack (stream, &bp);
>    return bp;
>
Diego Novillo July 29, 2011, 5:01 a.m. UTC | #2
On Thu, Jul 28, 2011 at 16:30, Lawrence Crowl <crowl@google.com> wrote:
> I'm getting massive failures after incorporating this change:
>
>   bytecode stream: trying to read 1735 bytes after the end of the
>   input buffer
>
> where the number of bytes changes.  Suggestions?

Odd.  I'm getting the usual results with:

$ git svn info
Path: .
URL: svn+ssh://gcc.gnu.org/svn/gcc/branches/pph/gcc
Repository Root: svn+ssh://gcc.gnu.org/svn/gcc
Repository UUID: 138bc75d-0d04-0410-961f-82ee72b054a4
Revision: 176671
Node Kind: directory
Schedule: normal
Last Changed Author: gchare
Last Changed Rev: 176671
Last Changed Date: 2011-07-22 21:04:48 -0400 (Fri, 22 Jul 2011)

Perhaps a file did not get rebuilt after you updated your tree?  That
may point to a Makefile dependency bug.  Or maybe you have some local
patch?


Diego.
Gab Charette July 29, 2011, 5:03 p.m. UTC | #3
I just stashed all my changes and pulled in the latest svn HEAD this
morning to check if I was seeing these failures:

Repository Root: svn+ssh://gcc.gnu.org/svn/gcc
Repository UUID: 138bc75d-0d04-0410-961f-82ee72b054a4
Revision: 176906
Node Kind: directory
Schedule: normal
Last Changed Author: crowl
Last Changed Rev: 176906
Last Changed Date: 2011-07-28 16:18:55 -0700 (Thu, 28 Jul 2011)

I did a successful build + pph check of both debug and opt builds
(incremental build only, I didn't actually need to start from scratch;
however I was stashing changes to some headers in libcpp, so
potentially that rebuilt somethings that weren't rebuilt in a smaller
incremental build if there is a missing dependency..?)

Gab

On Thu, Jul 28, 2011 at 10:01 PM, Diego Novillo <dnovillo@google.com> wrote:
> On Thu, Jul 28, 2011 at 16:30, Lawrence Crowl <crowl@google.com> wrote:
>> I'm getting massive failures after incorporating this change:
>>
>>   bytecode stream: trying to read 1735 bytes after the end of the
>>   input buffer
>>
>> where the number of bytes changes.  Suggestions?
>
> Odd.  I'm getting the usual results with:
>
> $ git svn info
> Path: .
> URL: svn+ssh://gcc.gnu.org/svn/gcc/branches/pph/gcc
> Repository Root: svn+ssh://gcc.gnu.org/svn/gcc
> Repository UUID: 138bc75d-0d04-0410-961f-82ee72b054a4
> Revision: 176671
> Node Kind: directory
> Schedule: normal
> Last Changed Author: gchare
> Last Changed Rev: 176671
> Last Changed Date: 2011-07-22 21:04:48 -0400 (Fri, 22 Jul 2011)
>
> Perhaps a file did not get rebuilt after you updated your tree?  That
> may point to a Makefile dependency bug.  Or maybe you have some local
> patch?
>
>
> Diego.
>
Lawrence Crowl July 29, 2011, 11:16 p.m. UTC | #4
I removed the build directories and rebuilt.  Everything worked.
There are gremlins in the machine.

On 7/29/11, Gabriel Charette <gchare@google.com> wrote:
> I just stashed all my changes and pulled in the latest svn HEAD this
> morning to check if I was seeing these failures:
>
> Repository Root: svn+ssh://gcc.gnu.org/svn/gcc
> Repository UUID: 138bc75d-0d04-0410-961f-82ee72b054a4
> Revision: 176906
> Node Kind: directory
> Schedule: normal
> Last Changed Author: crowl
> Last Changed Rev: 176906
> Last Changed Date: 2011-07-28 16:18:55 -0700 (Thu, 28 Jul 2011)
>
> I did a successful build + pph check of both debug and opt builds
> (incremental build only, I didn't actually need to start from scratch;
> however I was stashing changes to some headers in libcpp, so
> potentially that rebuilt somethings that weren't rebuilt in a smaller
> incremental build if there is a missing dependency..?)
>
> Gab
>
> On Thu, Jul 28, 2011 at 10:01 PM, Diego Novillo <dnovillo@google.com> wrote:
>> On Thu, Jul 28, 2011 at 16:30, Lawrence Crowl <crowl@google.com> wrote:
>>> I'm getting massive failures after incorporating this change:
>>>
>>>   bytecode stream: trying to read 1735 bytes after the end of the
>>>   input buffer
>>>
>>> where the number of bytes changes.  Suggestions?
>>
>> Odd.  I'm getting the usual results with:
>>
>> $ git svn info
>> Path: .
>> URL: svn+ssh://gcc.gnu.org/svn/gcc/branches/pph/gcc
>> Repository Root: svn+ssh://gcc.gnu.org/svn/gcc
>> Repository UUID: 138bc75d-0d04-0410-961f-82ee72b054a4
>> Revision: 176671
>> Node Kind: directory
>> Schedule: normal
>> Last Changed Author: gchare
>> Last Changed Rev: 176671
>> Last Changed Date: 2011-07-22 21:04:48 -0400 (Fri, 22 Jul 2011)
>>
>> Perhaps a file did not get rebuilt after you updated your tree?  That
>> may point to a Makefile dependency bug.  Or maybe you have some local
>> patch?
>>
>>
>> Diego.
>>
>
Gab Charette Aug. 3, 2011, 6:15 p.m. UTC | #5
My linemap implementation just caught a very important failure due to
this patch (because of it's massive use of pph_in_string for
filenames).

We CANNOT free stream->encoder.r.file_data this early as it contains
all of the strings streamed in (which are refered to directly from all
over the place (pointers passed back by lto_input_string).

We will probably need the strings in here until the end of the
compilation, so we could either just not free it (i.e. memory leak,
but allow important when program is over...)

Or we could strcpy the strings pph'ed in, but that involves
duplication of identical strings, unless we use a cache-like trick for
this.

Or we could put it in a side table that we free at the end of the
compilation (or whenever the strings are no longer needed, but I was
getting some seg faults deep down in the compilation because of this
memory issue).

On a positive note: with this fixed (I just commented out the free
line (pph-streamer.c:179) for now to try it), my linemap
implementation is now complete and makes c1eabi1.cc pass (and
p4eabi1's asm diff has changed, I'll look into that now).

Gab

On Fri, Jul 29, 2011 at 4:16 PM, Lawrence Crowl <crowl@google.com> wrote:
> I removed the build directories and rebuilt.  Everything worked.
> There are gremlins in the machine.
>
> On 7/29/11, Gabriel Charette <gchare@google.com> wrote:
>> I just stashed all my changes and pulled in the latest svn HEAD this
>> morning to check if I was seeing these failures:
>>
>> Repository Root: svn+ssh://gcc.gnu.org/svn/gcc
>> Repository UUID: 138bc75d-0d04-0410-961f-82ee72b054a4
>> Revision: 176906
>> Node Kind: directory
>> Schedule: normal
>> Last Changed Author: crowl
>> Last Changed Rev: 176906
>> Last Changed Date: 2011-07-28 16:18:55 -0700 (Thu, 28 Jul 2011)
>>
>> I did a successful build + pph check of both debug and opt builds
>> (incremental build only, I didn't actually need to start from scratch;
>> however I was stashing changes to some headers in libcpp, so
>> potentially that rebuilt somethings that weren't rebuilt in a smaller
>> incremental build if there is a missing dependency..?)
>>
>> Gab
>>
>> On Thu, Jul 28, 2011 at 10:01 PM, Diego Novillo <dnovillo@google.com> wrote:
>>> On Thu, Jul 28, 2011 at 16:30, Lawrence Crowl <crowl@google.com> wrote:
>>>> I'm getting massive failures after incorporating this change:
>>>>
>>>>   bytecode stream: trying to read 1735 bytes after the end of the
>>>>   input buffer
>>>>
>>>> where the number of bytes changes.  Suggestions?
>>>
>>> Odd.  I'm getting the usual results with:
>>>
>>> $ git svn info
>>> Path: .
>>> URL: svn+ssh://gcc.gnu.org/svn/gcc/branches/pph/gcc
>>> Repository Root: svn+ssh://gcc.gnu.org/svn/gcc
>>> Repository UUID: 138bc75d-0d04-0410-961f-82ee72b054a4
>>> Revision: 176671
>>> Node Kind: directory
>>> Schedule: normal
>>> Last Changed Author: gchare
>>> Last Changed Rev: 176671
>>> Last Changed Date: 2011-07-22 21:04:48 -0400 (Fri, 22 Jul 2011)
>>>
>>> Perhaps a file did not get rebuilt after you updated your tree?  That
>>> may point to a Makefile dependency bug.  Or maybe you have some local
>>> patch?
>>>
>>>
>>> Diego.
>>>
>>
>
>
> --
> Lawrence Crowl
>
Diego Novillo Aug. 4, 2011, 5:25 p.m. UTC | #6
On Wed, Aug 3, 2011 at 14:15, Gabriel Charette <gchare@google.com> wrote:

> We will probably need the strings in here until the end of the
> compilation, so we could either just not free it (i.e. memory leak,
> but allow important when program is over...)
>
> Or we could strcpy the strings pph'ed in, but that involves
> duplication of identical strings, unless we use a cache-like trick for
> this.

OK, I think I know what you are running into.  I just committed
http://codereview.appspot.com/4843044 which should fix the problem you
were running into.  Instead of creating the string table on top of the
raw file buffer, we allocate them separately and never free them up.

Does that fix your problem?


Diego.
diff mbox

Patch

Index: cp/pph-streamer-in.c
===================================================================
--- cp/pph-streamer-in.c	(revision 176879)
+++ cp/pph-streamer-in.c	(working copy)
@@ -109,8 +109,8 @@  pph_get_section_data (struct lto_file_de
 {
   /* FIXME pph - Stop abusing lto_file_decl_data fields.  */
   const pph_stream *stream = (const pph_stream *) file_data->file_name;
-  *len = stream->file_size - sizeof (pph_file_header);
-  return (const char *) stream->file_data + sizeof (pph_file_header);
+  *len = stream->encoder.r.file_size - sizeof (pph_file_header);
+  return (const char *) stream->encoder.r.file_data + sizeof (pph_file_header);
 }
 
 
@@ -119,14 +119,14 @@  pph_get_section_data (struct lto_file_de
 
 static void
 pph_free_section_data (struct lto_file_decl_data *file_data,
-		   enum lto_section_type section_type ATTRIBUTE_UNUSED,
-		   const char *name ATTRIBUTE_UNUSED,
-		   const char *offset ATTRIBUTE_UNUSED,
-		   size_t len ATTRIBUTE_UNUSED)
+		       enum lto_section_type section_type ATTRIBUTE_UNUSED,
+		       const char *name ATTRIBUTE_UNUSED,
+		       const char *offset ATTRIBUTE_UNUSED,
+		       size_t len ATTRIBUTE_UNUSED)
 {
   /* FIXME pph - Stop abusing lto_file_decl_data fields.  */
   const pph_stream *stream = (const pph_stream *) file_data->file_name;
-  free (stream->file_data);
+  free (stream->encoder.r.file_data);
 }
 
 
@@ -145,46 +145,48 @@  pph_init_read (pph_stream *stream)
 
   lto_reader_init ();
 
-  /* Read STREAM->NAME into the memory buffer STREAM->FILE_DATA.
-     FIXME pph, we are reading the whole file at once.  This seems
-     wasteful.  */
+  /* Read STREAM->NAME into the memory buffer stream->encoder.r.file_data.  */
   retcode = fstat (fileno (stream->file), &st);
   gcc_assert (retcode == 0);
-  stream->file_size = (size_t) st.st_size;
-  stream->file_data = XCNEWVEC (char, stream->file_size);
-  bytes_read = fread (stream->file_data, 1, stream->file_size, stream->file);
-  gcc_assert (bytes_read == stream->file_size);
+  stream->encoder.r.file_size = (size_t) st.st_size;
+  stream->encoder.r.file_data = XCNEWVEC (char, stream->encoder.r.file_size);
+  bytes_read = fread (stream->encoder.r.file_data, 1,
+		      stream->encoder.r.file_size, stream->file);
+  gcc_assert (bytes_read == stream->encoder.r.file_size);
 
   /* Set LTO callbacks to read the PPH file.  */
-  stream->pph_sections = XCNEWVEC (struct lto_file_decl_data *,
-				   PPH_NUM_SECTIONS);
+  stream->encoder.r.pph_sections = XCNEWVEC (struct lto_file_decl_data *,
+					     PPH_NUM_SECTIONS);
   for (i = 0; i < PPH_NUM_SECTIONS; i++)
     {
-      stream->pph_sections[i] = XCNEW (struct lto_file_decl_data);
+      stream->encoder.r.pph_sections[i] = XCNEW (struct lto_file_decl_data);
       /* FIXME pph - Stop abusing fields in lto_file_decl_data.  */
-      stream->pph_sections[i]->file_name = (const char *) stream;
+      stream->encoder.r.pph_sections[i]->file_name = (const char *) stream;
     }
 
-  lto_set_in_hooks (stream->pph_sections, pph_get_section_data,
+  lto_set_in_hooks (stream->encoder.r.pph_sections, pph_get_section_data,
 		    pph_free_section_data);
 
-  header = (pph_file_header *) stream->file_data;
+  header = (pph_file_header *) stream->encoder.r.file_data;
   strtab = (const char *) header + sizeof (pph_file_header);
   strtab_size = header->strtab_size;
   body = strtab + strtab_size;
-  gcc_assert (stream->file_size >= strtab_size + sizeof (pph_file_header));
-  body_size = stream->file_size - strtab_size - sizeof (pph_file_header);
+  gcc_assert (stream->encoder.r.file_size
+	      >= strtab_size + sizeof (pph_file_header));
+  body_size = stream->encoder.r.file_size
+	      - strtab_size - sizeof (pph_file_header);
 
   /* Create an input block structure pointing right after the string
      table.  */
-  stream->ib = XCNEW (struct lto_input_block);
-  LTO_INIT_INPUT_BLOCK_PTR (stream->ib, body, 0, body_size);
-  stream->data_in = lto_data_in_create (stream->pph_sections[0], strtab,
-                                        strtab_size, NULL);
+  stream->encoder.r.ib = XCNEW (struct lto_input_block);
+  LTO_INIT_INPUT_BLOCK_PTR (stream->encoder.r.ib, body, 0, body_size);
+  stream->encoder.r.data_in
+      = lto_data_in_create (stream->encoder.r.pph_sections[0], strtab,
+			    strtab_size, NULL);
 
-  /* Associate STREAM with STREAM->DATA_IN so we can recover it from
+  /* Associate STREAM with STREAM->ENCODER.R.DATA_IN so we can recover it from
      the streamer hooks.  */
-  stream->data_in->sdata = (void *) stream;
+  stream->encoder.r.data_in->sdata = (void *) stream;
 }
 
 
@@ -827,7 +829,8 @@  pph_in_struct_function (pph_stream *stre
   allocate_struct_function (decl, false);
   fn = DECL_STRUCT_FUNCTION (decl);
 
-  input_struct_function_base (fn, stream->data_in, stream->ib);
+  input_struct_function_base (fn, stream->encoder.r.data_in,
+			      stream->encoder.r.ib);
 
   /* struct eh_status *eh;					-- zero init */
   /* struct control_flow_graph *cfg;				-- zero init */
Index: cp/pph-streamer.c
===================================================================
--- cp/pph-streamer.c	(revision 176879)
+++ cp/pph-streamer.c	(working copy)
@@ -158,6 +158,25 @@  pph_stream_close (pph_stream *stream)
   stream->file = NULL;
   VEC_free (void_p, heap, stream->cache.v);
   pointer_map_destroy (stream->cache.m);
+
+  if (stream->write_p)
+    {
+      destroy_output_block (stream->encoder.w.ob);
+      free (stream->encoder.w.decl_state_stream);
+      lto_delete_out_decl_state (stream->encoder.w.out_state);
+    }
+  else
+    {
+      unsigned i;
+
+      free (stream->encoder.r.ib);
+      lto_data_in_delete (stream->encoder.r.data_in);
+      for (i = 0; i < PPH_NUM_SECTIONS; i++)
+	free (stream->encoder.r.pph_sections[i]);
+      free (stream->encoder.r.pph_sections);
+      free (stream->encoder.r.file_data);
+    }
+
   free (stream);
 }
 
Index: cp/pph-streamer-out.c
===================================================================
--- cp/pph-streamer-out.c	(revision 176879)
+++ cp/pph-streamer-out.c	(working copy)
@@ -99,14 +99,14 @@  void
 pph_init_write (pph_stream *stream)
 {
   lto_streamer_init ();
-  stream->out_state = lto_new_out_decl_state ();
-  lto_push_out_decl_state (stream->out_state);
-  stream->decl_state_stream = XCNEW (struct lto_output_stream);
-  stream->ob = create_output_block (LTO_section_decls);
+  stream->encoder.w.out_state = lto_new_out_decl_state ();
+  lto_push_out_decl_state (stream->encoder.w.out_state);
+  stream->encoder.w.decl_state_stream = XCNEW (struct lto_output_stream);
+  stream->encoder.w.ob = create_output_block (LTO_section_decls);
 
-  /* Associate STREAM with STREAM->OB so we can recover it from the
+  /* Associate STREAM with stream->encoder.w.ob so we can recover it from the
      streamer hooks.  */
-  stream->ob->sdata = (void *) stream;
+  stream->encoder.w.ob->sdata = (void *) stream;
 }
 
 
@@ -158,7 +158,7 @@  pph_out_header (pph_stream *stream)
   header.major_version = (char) major;
   header.minor_version = (char) minor;
   header.patchlevel = (char) patchlevel;
-  header.strtab_size = stream->ob->string_stream->total_size;
+  header.strtab_size = stream->encoder.w.ob->string_stream->total_size;
 
   memset (&header_stream, 0, sizeof (header_stream));
   lto_output_data_stream (&header_stream, &header, sizeof (header));
@@ -172,18 +172,20 @@  static void
 pph_out_body (pph_stream *stream)
 {
   /* Write the string table.  */
-  lto_write_stream (stream->ob->string_stream);
+  lto_write_stream (stream->encoder.w.ob->string_stream);
 
   /* Write out the physical representation for every AST in all the
-     streams in STREAM->OUT_STATE.  */
-  lto_output_decl_state_streams (stream->ob, stream->out_state);
+     streams in STREAM->ENCODER.W.OUT_STATE.  */
+  lto_output_decl_state_streams (stream->encoder.w.ob,
+				 stream->encoder.w.out_state);
 
   /* Now write the vector of all AST references.  */
-  lto_output_decl_state_refs (stream->ob, stream->decl_state_stream,
-			      stream->out_state);
+  lto_output_decl_state_refs (stream->encoder.w.ob,
+			      stream->encoder.w.decl_state_stream,
+			      stream->encoder.w.out_state);
 
   /* Finally, physically write all the streams.  */
-  lto_write_stream (stream->ob->main_stream);
+  lto_write_stream (stream->encoder.w.ob->main_stream);
 }
 
 
@@ -455,7 +457,7 @@  pph_out_ld_base (pph_stream *stream, str
 {
   struct bitpack_d bp;
 
-  bp = bitpack_create (stream->ob->main_stream);
+  bp = bitpack_create (stream->encoder.w.ob->main_stream);
   bp_pack_value (&bp, ldb->selector, 16);
   bp_pack_value (&bp, ldb->language, 4);
   bp_pack_value (&bp, ldb->use_template, 2);
@@ -537,7 +539,7 @@  pph_out_cxx_binding_1 (pph_stream *strea
   pph_out_tree_or_ref (stream, cb->value);
   pph_out_tree_or_ref (stream, cb->type);
   pph_out_binding_level (stream, cb->scope);
-  bp = bitpack_create (stream->ob->main_stream);
+  bp = bitpack_create (stream->encoder.w.ob->main_stream);
   bp_pack_value (&bp, cb->value_is_inherited, 1);
   bp_pack_value (&bp, cb->is_local, 1);
   pph_out_bitpack (stream, &bp);
@@ -684,7 +686,7 @@  pph_out_binding_level (pph_stream *strea
   pph_out_chain (stream, bl->statement_list);
   pph_out_uint (stream, bl->binding_depth);
 
-  bp = bitpack_create (stream->ob->main_stream);
+  bp = bitpack_create (stream->encoder.w.ob->main_stream);
   bp_pack_value (&bp, bl->kind, 4);
   bp_pack_value (&bp, bl->keep, 1);
   bp_pack_value (&bp, bl->more_cleanups_ok, 1);
@@ -735,7 +737,7 @@  pph_out_language_function (pph_stream *s
   pph_out_tree_or_ref (stream, lf->x_in_charge_parm);
   pph_out_tree_or_ref (stream, lf->x_vtt_parm);
   pph_out_tree_or_ref (stream, lf->x_return_value);
-  bp = bitpack_create (stream->ob->main_stream);
+  bp = bitpack_create (stream->encoder.w.ob->main_stream);
   bp_pack_value (&bp, lf->x_returns_value, 1);
   bp_pack_value (&bp, lf->x_returns_null, 1);
   bp_pack_value (&bp, lf->x_returns_abnormally, 1);
@@ -763,7 +765,7 @@  pph_out_ld_fn (pph_stream *stream, struc
   /* Write all the fields in lang_decl_min.  */
   pph_out_ld_min (stream, &ldf->min);
 
-  bp = bitpack_create (stream->ob->main_stream);
+  bp = bitpack_create (stream->encoder.w.ob->main_stream);
   bp_pack_value (&bp, ldf->operator_code, 16);
   bp_pack_value (&bp, ldf->global_ctor_p, 1);
   bp_pack_value (&bp, ldf->global_dtor_p, 1);
@@ -809,7 +811,7 @@  struct pph_tree_info {
 static int
 pph_out_used_types_slot (void **slot, void *aux)
 {
-  struct pph_tree_info *pti = (struct pph_tree_info *)aux;
+  struct pph_tree_info *pti = (struct pph_tree_info *) aux;
   pph_out_tree_or_ref (pti->stream, (tree) *slot);
   return 1;
 }
@@ -826,7 +828,7 @@  pph_out_struct_function (pph_stream *str
     return;
 
   pph_out_tree (stream, fn->decl);
-  output_struct_function_base (stream->ob, fn);
+  output_struct_function_base (stream->encoder.w.ob, fn);
 
   /* struct eh_status *eh;					-- ignored */
   gcc_assert (fn->cfg == NULL);
@@ -842,14 +844,14 @@  pph_out_struct_function (pph_stream *str
   /* struct machine_function *machine;				-- ignored */
   pph_out_language_function (stream, fn->language);
 
-  /*FIXME pph: We would like to detect improper sharing here.  */
+  /* FIXME pph: We would like to detect improper sharing here.  */
   if (fn->used_types_hash)
     {
-      /*FIXME pph: This write may be unstable.  */
+      /* FIXME pph: This write may be unstable.  */
       pph_out_uint (stream, htab_elements (fn->used_types_hash));
       pti.stream = stream;
-      htab_traverse_noresize (fn->used_types_hash,
-			      pph_out_used_types_slot, &pti);
+      htab_traverse_noresize (fn->used_types_hash, pph_out_used_types_slot,
+			      &pti);
     }
   else
     pph_out_uint (stream, 0);
@@ -943,12 +945,11 @@  pph_out_lang_specific (pph_stream *strea
 /* Write all the fields in lang_type_header instance LTH to STREAM.  */
 
 static void
-pph_out_lang_type_header (pph_stream *stream,
-				   struct lang_type_header *lth)
+pph_out_lang_type_header (pph_stream *stream, struct lang_type_header *lth)
 {
   struct bitpack_d bp;
 
-  bp = bitpack_create (stream->ob->main_stream);
+  bp = bitpack_create (stream->encoder.w.ob->main_stream);
   bp_pack_value (&bp, lth->is_lang_type_class, 1);
   bp_pack_value (&bp, lth->has_type_conversion, 1);
   bp_pack_value (&bp, lth->has_copy_ctor, 1);
@@ -1002,7 +1003,7 @@  pph_out_lang_type_class (pph_stream *str
 
   pph_out_uchar (stream, ltc->align);
 
-  bp = bitpack_create (stream->ob->main_stream);
+  bp = bitpack_create (stream->encoder.w.ob->main_stream);
   bp_pack_value (&bp, ltc->has_mutable, 1);
   bp_pack_value (&bp, ltc->com_interface, 1);
   bp_pack_value (&bp, ltc->non_pod_class, 1);
Index: cp/pph-streamer.h
===================================================================
--- cp/pph-streamer.h	(revision 176879)
+++ cp/pph-streamer.h	(working copy)
@@ -93,6 +93,8 @@  typedef struct pph_pickle_cache {
 } pph_pickle_cache;
 
 
+/* Data structures used to encode and decode trees.  */
+
 /* A PPH stream contains all the data and attributes needed to
    write symbols, declarations and other parsing products to disk.  */
 typedef struct pph_stream {
@@ -102,27 +104,24 @@  typedef struct pph_stream {
   /* FILE object associated with it.  */
   FILE *file;
 
-  /* LTO output block to hold pickled ASTs and references.  This is
-      NULL when the file is opened for reading.  */
-  struct output_block *ob;
-  struct lto_out_decl_state *out_state;
-  struct lto_output_stream *decl_state_stream;
-
-  /* LTO input block to read ASTs and references from.  This is NULL
-      when the file is opened for writing.  */
-  struct lto_input_block *ib;
-
-  /* String tables and other descriptors used by the LTO reading
-     routines.  NULL when the file is opened for writing.  */
-  struct data_in *data_in;
-
-  /* Array of sections in the PPH file.  */
-  struct lto_file_decl_data **pph_sections;
+  /* Data structures used to encode/decode trees.  */
+  union {
+    /* Encoding tables and buffers used to write trees to a file.  */
+    struct {
+	struct output_block *ob;
+	struct lto_out_decl_state *out_state;
+	struct lto_output_stream *decl_state_stream;
+    } w;
 
-  /* Buffer holding the file contents.  FIXME pph, we are bringing
-     the whole file in memory at once.  This seems wasteful.  */
-  char *file_data;
-  size_t file_size;
+    /* Decoding tables and buffers used to read trees from a file.  */
+    struct {
+	struct lto_input_block *ib;
+	struct data_in *data_in;
+	struct lto_file_decl_data **pph_sections;
+	char *file_data;
+	size_t file_size;
+    } r;
+  } encoder;
 
   /* Cache of pickled data structures.  */
   pph_pickle_cache cache;
@@ -186,7 +185,7 @@  pph_out_tree (pph_stream *stream, tree t
 {
   if (flag_pph_tracer >= 1)
     pph_trace_tree (stream, t);
-  lto_output_tree (stream->ob, t, false);
+  lto_output_tree (stream->encoder.w.ob, t, false);
 }
 
 /* Output array A of cardinality C of ASTs to STREAM.  */
@@ -200,7 +199,7 @@  pph_out_tree_array (pph_stream *stream, 
     {
       if (flag_pph_tracer >= 1)
         pph_trace_tree (stream, a[i]);
-      lto_output_tree (stream->ob, a[i]);
+      lto_output_tree (stream->encoder.w.ob, a[i]);
     }
 }
 #endif
@@ -212,7 +211,7 @@  pph_out_tree_or_ref_1 (pph_stream *strea
 {
   if (flag_pph_tracer >= tlevel)
     pph_trace_tree (stream, t);
-  lto_output_tree (stream->ob, t, false);
+  lto_output_tree (stream->encoder.w.ob, t, false);
 }
 
 /* Output AST T to STREAM.  Trigger tracing at -fpph-tracer=2.  */
@@ -228,7 +227,7 @@  pph_out_uint (pph_stream *stream, unsign
 {
   if (flag_pph_tracer >= 4)
     pph_trace_uint (stream, value);
-  lto_output_sleb128_stream (stream->ob->main_stream, value);
+  lto_output_sleb128_stream (stream->encoder.w.ob->main_stream, value);
 }
 
 /* Write an unsigned char VALUE to STREAM.  */
@@ -237,7 +236,7 @@  pph_out_uchar (pph_stream *stream, unsig
 {
   if (flag_pph_tracer >= 4)
     pph_trace_uint (stream, value);
-  lto_output_1_stream (stream->ob->main_stream, value);
+  lto_output_1_stream (stream->encoder.w.ob->main_stream, value);
 }
 
 /* Write N bytes from P to STREAM.  */
@@ -246,7 +245,7 @@  pph_out_bytes (pph_stream *stream, const
 {
   if (flag_pph_tracer >= 4)
     pph_trace_bytes (stream, p, n);
-  lto_output_data_stream (stream->ob->main_stream, p, n);
+  lto_output_data_stream (stream->encoder.w.ob->main_stream, p, n);
 }
 
 /* Write string STR to STREAM.  */
@@ -255,18 +254,20 @@  pph_out_string (pph_stream *stream, cons
 {
   if (flag_pph_tracer >= 4)
     pph_trace_string (stream, str);
-  lto_output_string (stream->ob, stream->ob->main_stream, str, false);
+  lto_output_string (stream->encoder.w.ob, stream->encoder.w.ob->main_stream,
+		     str, false);
 }
 
 /* Write string STR of length LEN to STREAM.  */
 static inline void
 pph_out_string_with_length (pph_stream *stream, const char *str,
-			       unsigned int len)
+			    unsigned int len)
 {
   if (flag_pph_tracer >= 4)
     pph_trace_string_with_length (stream, str, len);
-  lto_output_string_with_length (stream->ob, stream->ob->main_stream,
-				  str, len + 1, false);
+  lto_output_string_with_length (stream->encoder.w.ob,
+				 stream->encoder.w.ob->main_stream,
+				 str, len + 1, false);
 }
 
 /* Output VEC V of ASTs to STREAM.  */
@@ -283,7 +284,7 @@  pph_out_tree_VEC (pph_stream *stream, VE
     {
       if (flag_pph_tracer >= 1)
         pph_trace_tree (stream, t);
-      lto_output_tree (stream->ob, t);
+      lto_output_tree (stream->encoder.w.ob, t);
     }
 }
 #endif
@@ -294,7 +295,7 @@  pph_out_location (pph_stream *stream, lo
 {
   if (flag_pph_tracer >= 4)
     pph_trace_location (stream, loc);
-  lto_output_location (stream->ob, loc);
+  lto_output_location (stream->encoder.w.ob, loc);
 }
 
 /* Write a chain of ASTs to STREAM starting with FIRST.  */
@@ -303,14 +304,14 @@  pph_out_chain (pph_stream *stream, tree 
 {
   if (flag_pph_tracer >= 2)
     pph_trace_chain (stream, first);
-  lto_output_chain (stream->ob, first, false);
+  lto_output_chain (stream->encoder.w.ob, first, false);
 }
 
 /* Write a bitpack BP to STREAM.  */
 static inline void
 pph_out_bitpack (pph_stream *stream, struct bitpack_d *bp)
 {
-  gcc_assert (stream->ob->main_stream == bp->stream);
+  gcc_assert (stream->encoder.w.ob->main_stream == bp->stream);
   if (flag_pph_tracer >= 4)
     pph_trace_bitpack (stream, bp);
   lto_output_bitpack (bp);
@@ -320,7 +321,7 @@  pph_out_bitpack (pph_stream *stream, str
 static inline unsigned int
 pph_in_uint (pph_stream *stream)
 {
-  HOST_WIDE_INT unsigned n = lto_input_uleb128 (stream->ib);
+  HOST_WIDE_INT unsigned n = lto_input_uleb128 (stream->encoder.r.ib);
   gcc_assert (n == (unsigned) n);
   if (flag_pph_tracer >= 4)
     pph_trace_uint (stream, n);
@@ -331,7 +332,7 @@  pph_in_uint (pph_stream *stream)
 static inline unsigned char
 pph_in_uchar (pph_stream *stream)
 {
-  unsigned char n = lto_input_1_unsigned (stream->ib);
+  unsigned char n = lto_input_1_unsigned (stream->encoder.r.ib);
   if (flag_pph_tracer >= 4)
     pph_trace_uint (stream, n);
   return n;
@@ -342,7 +343,7 @@  pph_in_uchar (pph_stream *stream)
 static inline void
 pph_in_bytes (pph_stream *stream, void *p, size_t n)
 {
-  lto_input_data_block (stream->ib, p, n);
+  lto_input_data_block (stream->encoder.r.ib, p, n);
   if (flag_pph_tracer >= 4)
     pph_trace_bytes (stream, p, n);
 }
@@ -352,7 +353,8 @@  pph_in_bytes (pph_stream *stream, void *
 static inline const char *
 pph_in_string (pph_stream *stream)
 {
-  const char *s = lto_input_string (stream->data_in, stream->ib);
+  const char *s = lto_input_string (stream->encoder.r.data_in,
+				    stream->encoder.r.ib);
   if (flag_pph_tracer >= 4)
     pph_trace_string (stream, s);
   return s;
@@ -362,7 +364,8 @@  pph_in_string (pph_stream *stream)
 static inline location_t
 pph_in_location (pph_stream *stream)
 {
-  location_t loc = lto_input_location (stream->ib, stream->data_in);
+  location_t loc = lto_input_location (stream->encoder.r.ib,
+				       stream->encoder.r.data_in);
   if (flag_pph_tracer >= 4)
     pph_trace_location (stream, loc);
   return loc;
@@ -372,7 +375,7 @@  pph_in_location (pph_stream *stream)
 static inline tree
 pph_in_tree (pph_stream *stream)
 {
-  tree t = lto_input_tree (stream->ib, stream->data_in);
+  tree t = lto_input_tree (stream->encoder.r.ib, stream->encoder.r.data_in);
   if (flag_pph_tracer >= 4)
     pph_trace_tree (stream, t);
   return t;
@@ -387,7 +390,7 @@  pph_in_tree_array (pph_stream *stream, t
   size_t i;
   for (i = 0; i < c; ++i)
     {
-      tree t = lto_input_tree (stream->ib, stream->data_in);
+      tree t = lto_input_tree (stream->encoder.r.ib, stream->encoder.r.data_in);
       if (flag_pph_tracer >= 4)
         pph_trace_tree (stream, t, false); /* FIXME pph: always false? */
       a[i] = t;
@@ -405,7 +408,7 @@  pph_in_tree_VEC (pph_stream *stream, VEC
   unsigned int c = pph_in_uint (stream);
   for (i = 0; i < c; ++i)
     {
-      tree t = lto_input_tree (stream->ib, stream->data_in);
+      tree t = lto_input_tree (stream->encoder.r.ib, stream->encoder.r.data_in);
       if (flag_pph_tracer >= 4)
         pph_trace_tree (stream, t, false); /* FIXME pph: always false? */
       VEC_safe_push (tree, gc, v, t);
@@ -417,7 +420,7 @@  pph_in_tree_VEC (pph_stream *stream, VEC
 static inline tree
 pph_in_chain (pph_stream *stream)
 {
-  tree t = lto_input_chain (stream->ib, stream->data_in);
+  tree t = lto_input_chain (stream->encoder.r.ib, stream->encoder.r.data_in);
   if (flag_pph_tracer >= 2)
     pph_trace_chain (stream, t);
   return t;
@@ -427,7 +430,7 @@  pph_in_chain (pph_stream *stream)
 static inline struct bitpack_d
 pph_in_bitpack (pph_stream *stream)
 {
-  struct bitpack_d bp = lto_input_bitpack (stream->ib);
+  struct bitpack_d bp = lto_input_bitpack (stream->encoder.r.ib);
   if (flag_pph_tracer >= 4)
     pph_trace_bitpack (stream, &bp);
   return bp;