[pph] Fix child references being included multiple times (issue4904050)

[pph] Fix child references being included multiple times (issue4904050)

[Gabriel Charette]

This patch fixes the fact that referenced child includes were read more then once before as stream->includes would contain too much.

This refactors the include structure.

pph_read_images now contains a flat vector of the pph_streams of all images read so far.

pph_out_stream->encoder.w.includes now contains a list of the streams of the includes coming directly from the file we are currently pph'ing (NOT their references)

Only the main stream (pph_out_stream) has such a list, we do not need to keep track of the include hiearchy of the non-direct includes as it is only needed on read and is already saved in the included pphs themselves.

This fixes c2deepincl.cc which was introduced yesterday to expose this same issue.

Tested with bootstrap and pph regression testing on x64.

Gab

2011-08-17  Gabriel Charette  <gchare@google.com>

	gcc/cp/ChangeLog.pph
	* pph-streamer-in.c (pph_reading_includes): New.
	(pph_in_includes): Add logic to control pph_reading_includes.
	(pph_read_file_1): Only call pph_add_include if reading an include
	from the main file (not from pph_in_includes).
	(pph_read_file): Don't handle adding to pph_read_images here.
	(pph_reader_finish): Free pph_read_images.
	* pph-streamer-out.c (pph_tree_matches): Remove now unused STREAM
	parameter. Update all users.
	(pph_add_include): Changed signature to (stream, bool).
	Update all users.
	Handle adding INCLUDE to pph_read_images here.
	* gcc/cp/pph-streamer.c (pph_read_images): Moved here from
	pph-streamer-in.c
	(pph_stream_close): Free stream->encoder.w.includes.
	(pph_cache_lookup_in_includes): Lookup in pph_read_images.
	Remove now unused STREAM parameter.
	Update all users.
	(pph_cache_get): Index in pph_read_images.
	* pph-streamer.h (pph_stream): Move field includes to
	pph_stream.encoder.w.includes. Update all users.
	* gcc/cp/pph.c (pph_finish): Fixed extra space typo.

	gcc/testsuite/ChangeLog.pph
	* g++.dg/pph/c2deepincl.cc: Remove asm xdiff.

diff --git a/gcc/cp/pph-streamer-in.c b/gcc/cp/pph-streamer-in.c
index 013f526..3b2e082 100644
--- a/gcc/cp/pph-streamer-in.c
+++ b/gcc/cp/pph-streamer-in.c
@@ -33,13 +33,6 @@ along with GCC; see the file COPYING3.  If not see
 #include "cppbuiltin.h"
 #include "toplev.h"
 
-/* List of PPH images read during parsing.  Images opened during #include
-   processing and opened from pph_in_includes cannot be closed
-   immediately after reading, because the pickle cache contained in
-   them may be referenced from other images.  We delay closing all of
-   them until the end of parsing (when pph_reader_finish is called).  */
-static VEC(pph_stream_ptr,heap) *pph_read_images = NULL;
-
 typedef char *char_p;
 DEF_VEC_P(char_p);
 DEF_VEC_ALLOC_P(char_p,heap);
@@ -53,6 +46,12 @@ DEF_VEC_ALLOC_P(char_p,heap);
   memory will remain allocated until the end of compilation.  */
 static VEC(char_p,heap) *string_tables = NULL;
 
+/* Increment when we are in the process of reading includes as we do not want
+   to add those to the parent pph stream's list of includes to be written out.
+   Decrement when done. We cannot use a simple true/false flag as read includes
+   will call pph_in_includes as well.  */
+static int pph_reading_includes = 0;
+
 /* Wrapper for memory allocation calls that should have their results
    registered in the PPH streamer cache.  DATA is the pointer returned
    by the memory allocation call in ALLOC_EXPR.  IX is the cache slot 
@@ -1289,13 +1288,17 @@ pph_in_includes (pph_stream *stream)
 {
   unsigned i, num;
 
+  pph_reading_includes++;
+
   num = pph_in_uint (stream);
   for (i = 0; i < num; i++)
     {
       const char *include_name = pph_in_string (stream);
       pph_stream *include = pph_read_file (include_name);
-      pph_add_include (stream, include);
+      pph_add_include (include, false);
     }
+
+  pph_reading_includes--;
 }
 
 
@@ -1467,8 +1470,8 @@ pph_read_file_1 (pph_stream *stream)
   /* If we are generating an image, the PPH contents we just read from
      STREAM will need to be read again the next time we want to read
      the image we are now generating.  */
-  if (pph_out_file)
-    pph_add_include (NULL, stream);
+  if (pph_out_file && !pph_reading_includes)
+    pph_add_include (stream, true);
 }
 
 
@@ -1481,10 +1484,7 @@ pph_read_file (const char *filename)
 
   stream = pph_stream_open (filename, "rb");
   if (stream)
-    {
-      pph_read_file_1 (stream);
-      VEC_safe_push (pph_stream_ptr, heap, pph_read_images, stream);
-    }
+    pph_read_file_1 (stream);
   else
     error ("Cannot open PPH file for reading: %s: %m", filename);
 
@@ -1964,4 +1964,6 @@ pph_reader_finish (void)
   /* Close any images read during parsing.  */
   FOR_EACH_VEC_ELT (pph_stream_ptr, pph_read_images, i, image)
     pph_stream_close (image);
+
+  VEC_free (pph_stream_ptr, heap, pph_read_images);
 }
diff --git a/gcc/cp/pph-streamer-out.c b/gcc/cp/pph-streamer-out.c
index 2099d4e..d5b01dd 100644
--- a/gcc/cp/pph-streamer-out.c
+++ b/gcc/cp/pph-streamer-out.c
@@ -207,11 +207,11 @@ pph_out_start_record (pph_stream *stream, void *data)
       return false;
     }
 
-  /* DATA is not in STREAM's cache.  See if it is in any of STREAM's
+  /* DATA is not in STREAM's cache.  See if it is in any of the
      included images.  If it is, write an external reference to it
      and inform the caller that it should not write a physical
      representation for DATA.  */
-  if (pph_cache_lookup_in_includes (stream, data, &include_ix, &ix))
+  if (pph_cache_lookup_in_includes (data, &include_ix, &ix))
     {
       pph_out_record_marker (stream, PPH_RECORD_XREF);
       pph_out_uint (stream, include_ix);
@@ -351,7 +351,7 @@ pph_out_ld_min (pph_stream *stream, struct lang_decl_min *ldm)
 /* Return true if T matches FILTER for STREAM.  */
 
 static inline bool
-pph_tree_matches (pph_stream *stream, tree t, unsigned filter)
+pph_tree_matches (tree t, unsigned filter)
 {
   if ((filter & PPHF_NO_BUILTINS)
       && DECL_P (t)
@@ -359,7 +359,7 @@ pph_tree_matches (pph_stream *stream, tree t, unsigned filter)
     return false;
 
   if ((filter & PPHF_NO_XREFS)
-      && pph_cache_lookup_in_includes (stream, t, NULL, NULL))
+      && pph_cache_lookup_in_includes (t, NULL, NULL))
     return false;
 
   return true;
@@ -399,7 +399,7 @@ pph_out_tree_vec_filtered (pph_stream *stream, VEC(tree,gc) *v, unsigned filter)
 
   /* Collect all the nodes that match the filter.  */
   FOR_EACH_VEC_ELT (tree, v, i, t)
-    if (pph_tree_matches (stream, t, filter))
+    if (pph_tree_matches (t, filter))
       VEC_safe_push (tree, heap, to_write, t);
 
   /* Write them.  */
@@ -533,7 +533,7 @@ pph_out_chain_filtered (pph_stream *stream, tree first, unsigned filter)
 
   /* Collect all the nodes that match the filter.  */
   for (t = first; t; t = TREE_CHAIN (t))
-    if (pph_tree_matches (stream, t, filter))
+    if (pph_tree_matches (t, filter))
       VEC_safe_push (tree, heap, to_write, t);
 
   /* Write them.  */
@@ -1150,8 +1150,9 @@ pph_out_includes (pph_stream *stream)
   unsigned i;
   pph_stream *include;
 
-  pph_out_uint (stream, VEC_length (pph_stream_ptr, stream->includes));
-  FOR_EACH_VEC_ELT (pph_stream_ptr, stream->includes, i, include)
+  pph_out_uint (stream, VEC_length (pph_stream_ptr,
+	                            stream->encoder.w.includes));
+  FOR_EACH_VEC_ELT (pph_stream_ptr, stream->encoder.w.includes, i, include)
     pph_out_string (stream, include->name);
 }
 
@@ -1744,16 +1745,18 @@ pph_add_decl_to_symtab (tree decl)
 }
 
 
-/* Add INCLUDE to the list of files included by STREAM.  If STREAM is
-   NULL, INCLUDE is added to the list of includes for pph_out_stream
-   (the image that we are currently generating).  */
+/* Add INCLUDE to the list of files included by  the main pph_out_stream
+   if IS_MAIN_STREAM_INCLUDE, as well as to the global list of all read
+   includes.  */
 
 void
-pph_add_include (pph_stream *stream, pph_stream *include)
+pph_add_include (pph_stream *include, bool is_main_stream_include)
 {
-  if (stream == NULL)
-    stream = pph_out_stream;
-  VEC_safe_push (pph_stream_ptr, heap, stream->includes, include);
+  if (is_main_stream_include)
+    VEC_safe_push (pph_stream_ptr, heap,
+	           pph_out_stream->encoder.w.includes, include);
+
+  VEC_safe_push (pph_stream_ptr, heap, pph_read_images, include);
 }
 
 
diff --git a/gcc/cp/pph-streamer.c b/gcc/cp/pph-streamer.c
index 1ac5bf4..117801f 100644
--- a/gcc/cp/pph-streamer.c
+++ b/gcc/cp/pph-streamer.c
@@ -33,6 +33,13 @@ along with GCC; see the file COPYING3.  If not see
 #include "cppbuiltin.h"
 #include "streamer-hooks.h"
 
+/* List of PPH images read during parsing.  Images opened during #include
+   processing and opened from pph_in_includes cannot be closed
+   immediately after reading, because the pickle cache contained in
+   them may be referenced from other images.  We delay closing all of
+   them until the end of parsing (when pph_reader_finish is called).  */
+VEC(pph_stream_ptr, heap) *pph_read_images = NULL;
+
 /* Pre-load common tree nodes into the pickle cache in STREAM.  These
    nodes are always built by the front end, so there is no need to
    pickle them.
@@ -154,6 +161,7 @@ pph_stream_close (pph_stream *stream)
       destroy_output_block (stream->encoder.w.ob);
       free (stream->encoder.w.decl_state_stream);
       lto_delete_out_decl_state (stream->encoder.w.out_state);
+      VEC_free (pph_stream_ptr, heap, stream->encoder.w.includes);
     }
   else
     {
@@ -423,8 +431,7 @@ pph_cache_lookup (pph_stream *stream, void *data, unsigned *ix_p)
 }
 
 
-/* Return true if DATA is in the pickle cache of one of STREAM's
-   included images.
+/* Return true if DATA is in the pickle cache of one of the included images.
 
    If DATA is found:
       - the index for INCLUDE_P into IMAGE->INCLUDES is returned in
@@ -439,15 +446,15 @@ pph_cache_lookup (pph_stream *stream, void *data, unsigned *ix_p)
       - the function returns false.  */
 
 bool
-pph_cache_lookup_in_includes (pph_stream *stream, void *data,
-			      unsigned *include_ix_p, unsigned *ix_p)
+pph_cache_lookup_in_includes (void *data, unsigned *include_ix_p,
+                              unsigned *ix_p)
 {
   unsigned include_ix, ix;
   pph_stream *include;
   bool found_it;
 
   found_it = false;
-  FOR_EACH_VEC_ELT (pph_stream_ptr, stream->includes, include_ix, include)
+  FOR_EACH_VEC_ELT (pph_stream_ptr, pph_read_images, include_ix, include)
     if (pph_cache_lookup (include, data, &ix))
       {
 	found_it = true;
@@ -512,7 +519,7 @@ pph_cache_get (pph_stream *stream, unsigned include_ix, unsigned ix)
   if (include_ix == (unsigned) -1)
     image = stream;
   else
-    image = VEC_index (pph_stream_ptr, stream->includes, include_ix);
+    image = VEC_index (pph_stream_ptr, pph_read_images, include_ix);
 
   data = VEC_index (void_p, image->cache.v, ix);
   gcc_assert (data);
diff --git a/gcc/cp/pph-streamer.h b/gcc/cp/pph-streamer.h
index 8c3298a..60946b5 100644
--- a/gcc/cp/pph-streamer.h
+++ b/gcc/cp/pph-streamer.h
@@ -154,6 +154,11 @@ typedef struct pph_stream {
 	struct output_block *ob;
 	struct lto_out_decl_state *out_state;
 	struct lto_output_stream *decl_state_stream;
+
+	/* List of PPH files included by the PPH file that we are currently
+	  generating.  Note that this list only contains PPH files, not
+	  regular text headers.  Those are embedded in this stream.  */
+	VEC(pph_stream_ptr,heap) *includes;
     } w;
 
     /* Decoding tables and buffers used to read trees from a file.  */
@@ -178,11 +183,6 @@ typedef struct pph_stream {
     will be able to instantiate these symbols in the same order that
     they were instantiated originally.  */
   pph_symtab symtab;
-
-  /* List of PPH files included by the PPH file that we are currently
-     generating.  Note that this list only contains PPH files, not
-     regular text headers.  Those are embedded in this stream.  */
-  VEC(pph_stream_ptr,heap) *includes;
 } pph_stream;
 
 /* Filter values to avoid emitting certain objects to a PPH file.  */
@@ -203,17 +203,18 @@ void pph_trace_chain (pph_stream *, tree);
 void pph_trace_bitpack (pph_stream *, struct bitpack_d *);
 void pph_cache_insert_at (pph_stream *, void *, unsigned);
 bool pph_cache_lookup (pph_stream *, void *, unsigned *);
-bool pph_cache_lookup_in_includes (pph_stream *, void *, unsigned *,
-				   unsigned *);
+bool pph_cache_lookup_in_includes (void *, unsigned *, unsigned *);
 bool pph_cache_add (pph_stream *, void *, unsigned *);
 void *pph_cache_get (pph_stream *, unsigned, unsigned);
 
+extern VEC(pph_stream_ptr, heap) *pph_read_images;
+
 /* In pph-streamer-out.c.  */
 void pph_flush_buffers (pph_stream *);
 void pph_init_write (pph_stream *);
 void pph_write_tree (struct output_block *, tree, bool);
 void pph_add_decl_to_symtab (tree);
-void pph_add_include (pph_stream *, pph_stream *);
+void pph_add_include (pph_stream *, bool);
 void pph_writer_init (void);
 void pph_writer_finish (void);
 void pph_write_location (struct output_block *, location_t);
diff --git a/gcc/cp/pph.c b/gcc/cp/pph.c
index 91dc622..4779010 100644
--- a/gcc/cp/pph.c
+++ b/gcc/cp/pph.c
@@ -172,7 +172,7 @@ void
 pph_finish (void)
 {
   /* Finalize the writer.  */
-    pph_writer_finish ();
+  pph_writer_finish ();
 
   /* Finalize the reader.  */
   pph_reader_finish ();
diff --git a/gcc/testsuite/g++.dg/pph/c2deepincl.cc b/gcc/testsuite/g++.dg/pph/c2deepincl.cc
index da56b1e..bbf9402 100644
--- a/gcc/testsuite/g++.dg/pph/c2deepincl.cc
+++ b/gcc/testsuite/g++.dg/pph/c2deepincl.cc
@@ -1,5 +1,3 @@
-// pph asm xdiff 00611
-
 #include "c2deepincl3.h"
 
 int test() {

--
This patch is available for review at http://codereview.appspot.com/4904050

[pph] Fix child references being included multiple times (issue4904050)

[Gabriel Charette]

Committing version 1.

Added comment for pph_read_images and moved as requested.

See final patch attached.

Gab

2011-08-18  Gabriel Charette  <gchare@google.com>

	gcc/cp/ChangeLog.pph
	* pph-streamer-in.c (pph_reading_includes): New.
	(pph_in_includes): Add logic to control pph_reading_includes.
	(pph_read_file_1): Only call pph_add_include if reading an include
	from the main file (not from pph_in_includes).
	(pph_read_file): Don't handle adding to pph_read_images here.
	(pph_reader_finish): Free pph_read_images.
	* pph-streamer-out.c (pph_tree_matches): Remove now unused STREAM
	parameter. Update all users.
	(pph_add_include): Changed signature to (stream, bool).
	Update all users.
	Handle adding INCLUDE to pph_read_images here.
	* gcc/cp/pph-streamer.c (pph_read_images): Moved here from
	pph-streamer-in.c
	(pph_stream_close): Free stream->encoder.w.includes.
	(pph_cache_lookup_in_includes): Lookup in pph_read_images.
	Remove now unused STREAM parameter.
	Update all users.
	(pph_cache_get): Index in pph_read_images.
	* pph-streamer.h (pph_stream): Move field includes to
	pph_stream.encoder.w.includes. Update all users.
	* gcc/cp/pph.c (pph_init): Initialize pph_read_images to NULL.
	(pph_finish): Fixed extra space typo.

	gcc/testsuite/ChangeLog.pph
	* g++.dg/pph/c2deepincl.cc: Remove asm xdiff.

diff --git a/gcc/cp/pph-streamer-in.c b/gcc/cp/pph-streamer-in.c
index 40b65c7..4666ace 100644
--- a/gcc/cp/pph-streamer-in.c
+++ b/gcc/cp/pph-streamer-in.c
@@ -33,13 +33,6 @@ along with GCC; see the file COPYING3.  If not see
 #include "cppbuiltin.h"
 #include "toplev.h"
 
-/* List of PPH images read during parsing.  Images opened during #include
-   processing and opened from pph_in_includes cannot be closed
-   immediately after reading, because the pickle cache contained in
-   them may be referenced from other images.  We delay closing all of
-   them until the end of parsing (when pph_reader_finish is called).  */
-static VEC(pph_stream_ptr,heap) *pph_read_images = NULL;
-
 typedef char *char_p;
 DEF_VEC_P(char_p);
 DEF_VEC_ALLOC_P(char_p,heap);
@@ -53,6 +46,12 @@ DEF_VEC_ALLOC_P(char_p,heap);
   memory will remain allocated until the end of compilation.  */
 static VEC(char_p,heap) *string_tables = NULL;
 
+/* Increment when we are in the process of reading includes as we do not want
+   to add those to the parent pph stream's list of includes to be written out.
+   Decrement when done. We cannot use a simple true/false flag as read includes
+   will call pph_in_includes as well.  */
+static int pph_reading_includes = 0;
+
 /* Wrapper for memory allocation calls that should have their results
    registered in the PPH streamer cache.  DATA is the pointer returned
    by the memory allocation call in ALLOC_EXPR.  IX is the cache slot 
@@ -1289,13 +1288,17 @@ pph_in_includes (pph_stream *stream)
 {
   unsigned i, num;
 
+  pph_reading_includes++;
+
   num = pph_in_uint (stream);
   for (i = 0; i < num; i++)
     {
       const char *include_name = pph_in_string (stream);
       pph_stream *include = pph_read_file (include_name);
-      pph_add_include (stream, include);
+      pph_add_include (include, false);
     }
+
+  pph_reading_includes--;
 }
 
 
@@ -1467,8 +1470,8 @@ pph_read_file_1 (pph_stream *stream)
   /* If we are generating an image, the PPH contents we just read from
      STREAM will need to be read again the next time we want to read
      the image we are now generating.  */
-  if (pph_out_file)
-    pph_add_include (NULL, stream);
+  if (pph_out_file && !pph_reading_includes)
+    pph_add_include (stream, true);
 }
 
 
@@ -1481,10 +1484,7 @@ pph_read_file (const char *filename)
 
   stream = pph_stream_open (filename, "rb");
   if (stream)
-    {
-      pph_read_file_1 (stream);
-      VEC_safe_push (pph_stream_ptr, heap, pph_read_images, stream);
-    }
+    pph_read_file_1 (stream);
   else
     error ("Cannot open PPH file for reading: %s: %m", filename);
 
@@ -1964,4 +1964,6 @@ pph_reader_finish (void)
   /* Close any images read during parsing.  */
   FOR_EACH_VEC_ELT (pph_stream_ptr, pph_read_images, i, image)
     pph_stream_close (image);
+
+  VEC_free (pph_stream_ptr, heap, pph_read_images);
 }
diff --git a/gcc/cp/pph-streamer-out.c b/gcc/cp/pph-streamer-out.c
index d64f6c4..1b84cc3 100644
--- a/gcc/cp/pph-streamer-out.c
+++ b/gcc/cp/pph-streamer-out.c
@@ -207,11 +207,11 @@ pph_out_start_record (pph_stream *stream, void *data)
       return false;
     }
 
-  /* DATA is not in STREAM's cache.  See if it is in any of STREAM's
+  /* DATA is not in STREAM's cache.  See if it is in any of the
      included images.  If it is, write an external reference to it
      and inform the caller that it should not write a physical
      representation for DATA.  */
-  if (pph_cache_lookup_in_includes (stream, data, &include_ix, &ix))
+  if (pph_cache_lookup_in_includes (data, &include_ix, &ix))
     {
       pph_out_record_marker (stream, PPH_RECORD_XREF);
       pph_out_uint (stream, include_ix);
@@ -351,7 +351,7 @@ pph_out_ld_min (pph_stream *stream, struct lang_decl_min *ldm)
 /* Return true if T matches FILTER for STREAM.  */
 
 static inline bool
-pph_tree_matches (pph_stream *stream, tree t, unsigned filter)
+pph_tree_matches (tree t, unsigned filter)
 {
   if ((filter & PPHF_NO_BUILTINS)
       && DECL_P (t)
@@ -359,7 +359,7 @@ pph_tree_matches (pph_stream *stream, tree t, unsigned filter)
     return false;
 
   if ((filter & PPHF_NO_XREFS)
-      && pph_cache_lookup_in_includes (stream, t, NULL, NULL))
+      && pph_cache_lookup_in_includes (t, NULL, NULL))
     return false;
 
   return true;
@@ -399,7 +399,7 @@ pph_out_tree_vec_filtered (pph_stream *stream, VEC(tree,gc) *v, unsigned filter)
 
   /* Collect all the nodes that match the filter.  */
   FOR_EACH_VEC_ELT (tree, v, i, t)
-    if (pph_tree_matches (stream, t, filter))
+    if (pph_tree_matches (t, filter))
       VEC_safe_push (tree, heap, to_write, t);
 
   /* Write them.  */
@@ -533,7 +533,7 @@ pph_out_chain_filtered (pph_stream *stream, tree first, unsigned filter)
 
   /* Collect all the nodes that match the filter.  */
   for (t = first; t; t = TREE_CHAIN (t))
-    if (pph_tree_matches (stream, t, filter))
+    if (pph_tree_matches (t, filter))
       VEC_safe_push (tree, heap, to_write, t);
 
   /* Write them.  */
@@ -1150,8 +1150,9 @@ pph_out_includes (pph_stream *stream)
   unsigned i;
   pph_stream *include;
 
-  pph_out_uint (stream, VEC_length (pph_stream_ptr, stream->includes));
-  FOR_EACH_VEC_ELT (pph_stream_ptr, stream->includes, i, include)
+  pph_out_uint (stream, VEC_length (pph_stream_ptr,
+	                            stream->encoder.w.includes));
+  FOR_EACH_VEC_ELT (pph_stream_ptr, stream->encoder.w.includes, i, include)
     pph_out_string (stream, include->name);
 }
 
@@ -1744,16 +1745,18 @@ pph_add_decl_to_symtab (tree decl)
 }
 
 
-/* Add INCLUDE to the list of files included by STREAM.  If STREAM is
-   NULL, INCLUDE is added to the list of includes for pph_out_stream
-   (the image that we are currently generating).  */
+/* Add INCLUDE to the list of files included by  the main pph_out_stream
+   if IS_MAIN_STREAM_INCLUDE, as well as to the global list of all read
+   includes.  */
 
 void
-pph_add_include (pph_stream *stream, pph_stream *include)
+pph_add_include (pph_stream *include, bool is_main_stream_include)
 {
-  if (stream == NULL)
-    stream = pph_out_stream;
-  VEC_safe_push (pph_stream_ptr, heap, stream->includes, include);
+  if (is_main_stream_include)
+    VEC_safe_push (pph_stream_ptr, heap,
+	           pph_out_stream->encoder.w.includes, include);
+
+  VEC_safe_push (pph_stream_ptr, heap, pph_read_images, include);
 }
 
 
diff --git a/gcc/cp/pph-streamer.c b/gcc/cp/pph-streamer.c
index 1ac5bf4..27b2f78 100644
--- a/gcc/cp/pph-streamer.c
+++ b/gcc/cp/pph-streamer.c
@@ -33,6 +33,13 @@ along with GCC; see the file COPYING3.  If not see
 #include "cppbuiltin.h"
 #include "streamer-hooks.h"
 
+/* List of PPH images read during parsing.  Images opened during #include
+   processing and opened from pph_in_includes cannot be closed
+   immediately after reading, because the pickle cache contained in
+   them may be referenced from other images.  We delay closing all of
+   them until the end of parsing (when pph_reader_finish is called).  */
+VEC(pph_stream_ptr, heap) *pph_read_images;
+
 /* Pre-load common tree nodes into the pickle cache in STREAM.  These
    nodes are always built by the front end, so there is no need to
    pickle them.
@@ -154,6 +161,7 @@ pph_stream_close (pph_stream *stream)
       destroy_output_block (stream->encoder.w.ob);
       free (stream->encoder.w.decl_state_stream);
       lto_delete_out_decl_state (stream->encoder.w.out_state);
+      VEC_free (pph_stream_ptr, heap, stream->encoder.w.includes);
     }
   else
     {
@@ -423,8 +431,7 @@ pph_cache_lookup (pph_stream *stream, void *data, unsigned *ix_p)
 }
 
 
-/* Return true if DATA is in the pickle cache of one of STREAM's
-   included images.
+/* Return true if DATA is in the pickle cache of one of the included images.
 
    If DATA is found:
       - the index for INCLUDE_P into IMAGE->INCLUDES is returned in
@@ -439,15 +446,15 @@ pph_cache_lookup (pph_stream *stream, void *data, unsigned *ix_p)
       - the function returns false.  */
 
 bool
-pph_cache_lookup_in_includes (pph_stream *stream, void *data,
-			      unsigned *include_ix_p, unsigned *ix_p)
+pph_cache_lookup_in_includes (void *data, unsigned *include_ix_p,
+                              unsigned *ix_p)
 {
   unsigned include_ix, ix;
   pph_stream *include;
   bool found_it;
 
   found_it = false;
-  FOR_EACH_VEC_ELT (pph_stream_ptr, stream->includes, include_ix, include)
+  FOR_EACH_VEC_ELT (pph_stream_ptr, pph_read_images, include_ix, include)
     if (pph_cache_lookup (include, data, &ix))
       {
 	found_it = true;
@@ -512,7 +519,7 @@ pph_cache_get (pph_stream *stream, unsigned include_ix, unsigned ix)
   if (include_ix == (unsigned) -1)
     image = stream;
   else
-    image = VEC_index (pph_stream_ptr, stream->includes, include_ix);
+    image = VEC_index (pph_stream_ptr, pph_read_images, include_ix);
 
   data = VEC_index (void_p, image->cache.v, ix);
   gcc_assert (data);
diff --git a/gcc/cp/pph-streamer.h b/gcc/cp/pph-streamer.h
index 8c3298a..7842f1f 100644
--- a/gcc/cp/pph-streamer.h
+++ b/gcc/cp/pph-streamer.h
@@ -136,6 +136,13 @@ typedef struct pph_stream *pph_stream_ptr;
 DEF_VEC_P(pph_stream_ptr);
 DEF_VEC_ALLOC_P(pph_stream_ptr,heap);
 
+/* List of PPH images read during parsing.  Images opened during #include
+   processing and opened from pph_in_includes cannot be closed
+   immediately after reading, because the pickle cache contained in
+   them may be referenced from other images.  We delay closing all of
+   them until the end of parsing (when pph_reader_finish is called).  */
+extern VEC(pph_stream_ptr, heap) *pph_read_images;
+
 /* Data structures used to encode and decode trees.  */
 
 /* A PPH stream contains all the data and attributes needed to
@@ -154,6 +161,11 @@ typedef struct pph_stream {
 	struct output_block *ob;
 	struct lto_out_decl_state *out_state;
 	struct lto_output_stream *decl_state_stream;
+
+	/* List of PPH files included by the PPH file that we are currently
+	  generating.  Note that this list only contains PPH files, not
+	  regular text headers.  Those are embedded in this stream.  */
+	VEC(pph_stream_ptr,heap) *includes;
     } w;
 
     /* Decoding tables and buffers used to read trees from a file.  */
@@ -178,11 +190,6 @@ typedef struct pph_stream {
     will be able to instantiate these symbols in the same order that
     they were instantiated originally.  */
   pph_symtab symtab;
-
-  /* List of PPH files included by the PPH file that we are currently
-     generating.  Note that this list only contains PPH files, not
-     regular text headers.  Those are embedded in this stream.  */
-  VEC(pph_stream_ptr,heap) *includes;
 } pph_stream;
 
 /* Filter values to avoid emitting certain objects to a PPH file.  */
@@ -203,8 +210,7 @@ void pph_trace_chain (pph_stream *, tree);
 void pph_trace_bitpack (pph_stream *, struct bitpack_d *);
 void pph_cache_insert_at (pph_stream *, void *, unsigned);
 bool pph_cache_lookup (pph_stream *, void *, unsigned *);
-bool pph_cache_lookup_in_includes (pph_stream *, void *, unsigned *,
-				   unsigned *);
+bool pph_cache_lookup_in_includes (void *, unsigned *, unsigned *);
 bool pph_cache_add (pph_stream *, void *, unsigned *);
 void *pph_cache_get (pph_stream *, unsigned, unsigned);
 
@@ -213,7 +219,7 @@ void pph_flush_buffers (pph_stream *);
 void pph_init_write (pph_stream *);
 void pph_write_tree (struct output_block *, tree, bool);
 void pph_add_decl_to_symtab (tree);
-void pph_add_include (pph_stream *, pph_stream *);
+void pph_add_include (pph_stream *, bool);
 void pph_writer_init (void);
 void pph_writer_finish (void);
 void pph_write_location (struct output_block *, location_t);
diff --git a/gcc/cp/pph.c b/gcc/cp/pph.c
index 91dc622..86aacbc 100644
--- a/gcc/cp/pph.c
+++ b/gcc/cp/pph.c
@@ -163,6 +163,8 @@ pph_init (void)
   /* If we are generating a PPH file, initialize the writer.  */
   if (pph_out_file != NULL)
     pph_writer_init ();
+
+  pph_read_images = NULL;
 }
 
 
@@ -172,7 +174,7 @@ void
 pph_finish (void)
 {
   /* Finalize the writer.  */
-    pph_writer_finish ();
+  pph_writer_finish ();
 
   /* Finalize the reader.  */
   pph_reader_finish ();
diff --git a/gcc/testsuite/g++.dg/pph/c2deepincl.cc b/gcc/testsuite/g++.dg/pph/c2deepincl.cc
index da56b1e..bbf9402 100644
--- a/gcc/testsuite/g++.dg/pph/c2deepincl.cc
+++ b/gcc/testsuite/g++.dg/pph/c2deepincl.cc
@@ -1,5 +1,3 @@
-// pph asm xdiff 00611
-
 #include "c2deepincl3.h"
 
 int test() {

--
This patch is available for review at http://codereview.appspot.com/4904050

[pph] Fix child references being included multiple times (issue4904050)

[Gabriel Charette]

I changed my mind and kept stream->includes the way it was originally meant to be, i.e. every stream will have its list of includes not only the main pph's stream.

More specifically I will be using this for every stream when regenerating the linemap (and reading the includes in parallel) in my upcoming patch!

Furthermore, we could get rid of the global pph_read_image altogether now as it's only a flat vector representation of the include tree structure saved through the pph_streams'->includes. The only problem I see is in pph_cache_get where we need a fix index for a stream (we could definitely map an index to the post-order traversal of the include tree, but it won't be as fast as a simple index in a flat vector), I didn't do it for now..

Cheers,
Gab

2011-08-17  Gabriel Charette  <gchare@google.com>

	gcc/cp/ChangeLog.pph
	* pph-streamer-in.c (pph_reading_includes): New.
	(pph_in_includes): Add logic to control pph_reading_includes.
	(pph_read_file_1): Only call pph_add_include if reading an include
	from the main file (not from pph_in_includes).
	(pph_read_file): Don't handle adding to pph_read_images here.
	(pph_reader_finish): Free pph_read_images.
	* pph-streamer-out.c (pph_tree_matches): Remove now unused STREAM
	parameter. Update all users.
	(pph_add_include): Handle adding INCLUDE to pph_read_images here.
	* pph-streamer.c (pph_read_images): Moved here from pph-streamer-in.c
	(pph_stream_close): Free stream->includes.
	(pph_cache_lookup_in_includes): Lookup in pph_read_images.
	Remove now unused STREAM parameter. Update all users.
	(pph_cache_get): Index in pph_read_images.
	* pph-streamer.h (pph_stream): Changed comment for field "includes".
	* pph.c (pph_finish): Fixed extra space typo.

	gcc/testsuite/ChangeLog.pph
	* g++.dg/pph/c2deepincl.cc: Remove asm xdiff.

diff --git a/gcc/cp/pph-streamer-in.c b/gcc/cp/pph-streamer-in.c
index 013f526..58f084e 100644
--- a/gcc/cp/pph-streamer-in.c
+++ b/gcc/cp/pph-streamer-in.c
@@ -33,13 +33,6 @@ along with GCC; see the file COPYING3.  If not see
 #include "cppbuiltin.h"
 #include "toplev.h"
 
-/* List of PPH images read during parsing.  Images opened during #include
-   processing and opened from pph_in_includes cannot be closed
-   immediately after reading, because the pickle cache contained in
-   them may be referenced from other images.  We delay closing all of
-   them until the end of parsing (when pph_reader_finish is called).  */
-static VEC(pph_stream_ptr,heap) *pph_read_images = NULL;
-
 typedef char *char_p;
 DEF_VEC_P(char_p);
 DEF_VEC_ALLOC_P(char_p,heap);
@@ -53,6 +46,12 @@ DEF_VEC_ALLOC_P(char_p,heap);
   memory will remain allocated until the end of compilation.  */
 static VEC(char_p,heap) *string_tables = NULL;
 
+/* Increment when we are in the process of reading includes as we do not want
+   to add those to the parent pph stream's list of includes to be written out.
+   Decrement when done. We cannot use a simple true/false flag as read includes
+   will call pph_in_includes as well.  */
+static int pph_reading_includes = 0;
+
 /* Wrapper for memory allocation calls that should have their results
    registered in the PPH streamer cache.  DATA is the pointer returned
    by the memory allocation call in ALLOC_EXPR.  IX is the cache slot 
@@ -1289,6 +1288,8 @@ pph_in_includes (pph_stream *stream)
 {
   unsigned i, num;
 
+  pph_reading_includes++;
+
   num = pph_in_uint (stream);
   for (i = 0; i < num; i++)
     {
@@ -1296,6 +1297,8 @@ pph_in_includes (pph_stream *stream)
       pph_stream *include = pph_read_file (include_name);
       pph_add_include (stream, include);
     }
+
+  pph_reading_includes--;
 }
 
 
@@ -1467,7 +1470,7 @@ pph_read_file_1 (pph_stream *stream)
   /* If we are generating an image, the PPH contents we just read from
      STREAM will need to be read again the next time we want to read
      the image we are now generating.  */
-  if (pph_out_file)
+  if (pph_out_file && !pph_reading_includes)
     pph_add_include (NULL, stream);
 }
 
@@ -1481,10 +1484,7 @@ pph_read_file (const char *filename)
 
   stream = pph_stream_open (filename, "rb");
   if (stream)
-    {
-      pph_read_file_1 (stream);
-      VEC_safe_push (pph_stream_ptr, heap, pph_read_images, stream);
-    }
+    pph_read_file_1 (stream);
   else
     error ("Cannot open PPH file for reading: %s: %m", filename);
 
@@ -1964,4 +1964,6 @@ pph_reader_finish (void)
   /* Close any images read during parsing.  */
   FOR_EACH_VEC_ELT (pph_stream_ptr, pph_read_images, i, image)
     pph_stream_close (image);
+
+  VEC_free (pph_stream_ptr, heap, pph_read_images);
 }
diff --git a/gcc/cp/pph-streamer-out.c b/gcc/cp/pph-streamer-out.c
index 2099d4e..44fe018 100644
--- a/gcc/cp/pph-streamer-out.c
+++ b/gcc/cp/pph-streamer-out.c
@@ -207,11 +207,11 @@ pph_out_start_record (pph_stream *stream, void *data)
       return false;
     }
 
-  /* DATA is not in STREAM's cache.  See if it is in any of STREAM's
+  /* DATA is not in STREAM's cache.  See if it is in any of the
      included images.  If it is, write an external reference to it
      and inform the caller that it should not write a physical
      representation for DATA.  */
-  if (pph_cache_lookup_in_includes (stream, data, &include_ix, &ix))
+  if (pph_cache_lookup_in_includes (data, &include_ix, &ix))
     {
       pph_out_record_marker (stream, PPH_RECORD_XREF);
       pph_out_uint (stream, include_ix);
@@ -351,7 +351,7 @@ pph_out_ld_min (pph_stream *stream, struct lang_decl_min *ldm)
 /* Return true if T matches FILTER for STREAM.  */
 
 static inline bool
-pph_tree_matches (pph_stream *stream, tree t, unsigned filter)
+pph_tree_matches (tree t, unsigned filter)
 {
   if ((filter & PPHF_NO_BUILTINS)
       && DECL_P (t)
@@ -359,7 +359,7 @@ pph_tree_matches (pph_stream *stream, tree t, unsigned filter)
     return false;
 
   if ((filter & PPHF_NO_XREFS)
-      && pph_cache_lookup_in_includes (stream, t, NULL, NULL))
+      && pph_cache_lookup_in_includes (t, NULL, NULL))
     return false;
 
   return true;
@@ -399,7 +399,7 @@ pph_out_tree_vec_filtered (pph_stream *stream, VEC(tree,gc) *v, unsigned filter)
 
   /* Collect all the nodes that match the filter.  */
   FOR_EACH_VEC_ELT (tree, v, i, t)
-    if (pph_tree_matches (stream, t, filter))
+    if (pph_tree_matches (t, filter))
       VEC_safe_push (tree, heap, to_write, t);
 
   /* Write them.  */
@@ -533,7 +533,7 @@ pph_out_chain_filtered (pph_stream *stream, tree first, unsigned filter)
 
   /* Collect all the nodes that match the filter.  */
   for (t = first; t; t = TREE_CHAIN (t))
-    if (pph_tree_matches (stream, t, filter))
+    if (pph_tree_matches (t, filter))
       VEC_safe_push (tree, heap, to_write, t);
 
   /* Write them.  */
@@ -1746,7 +1746,8 @@ pph_add_decl_to_symtab (tree decl)
 
 /* Add INCLUDE to the list of files included by STREAM.  If STREAM is
    NULL, INCLUDE is added to the list of includes for pph_out_stream
-   (the image that we are currently generating).  */
+   (the image that we are currently generating).  Also add the INCLUDE
+   to the global list pph_read_images.  */
 
 void
 pph_add_include (pph_stream *stream, pph_stream *include)
@@ -1754,6 +1755,8 @@ pph_add_include (pph_stream *stream, pph_stream *include)
   if (stream == NULL)
     stream = pph_out_stream;
   VEC_safe_push (pph_stream_ptr, heap, stream->includes, include);
+
+  VEC_safe_push (pph_stream_ptr, heap, pph_read_images, include);
 }
 
 
diff --git a/gcc/cp/pph-streamer.c b/gcc/cp/pph-streamer.c
index 1ac5bf4..b6ccf0a 100644
--- a/gcc/cp/pph-streamer.c
+++ b/gcc/cp/pph-streamer.c
@@ -33,6 +33,13 @@ along with GCC; see the file COPYING3.  If not see
 #include "cppbuiltin.h"
 #include "streamer-hooks.h"
 
+/* List of PPH images read during parsing.  Images opened during #include
+   processing and opened from pph_in_includes cannot be closed
+   immediately after reading, because the pickle cache contained in
+   them may be referenced from other images.  We delay closing all of
+   them until the end of parsing (when pph_reader_finish is called).  */
+VEC(pph_stream_ptr, heap) *pph_read_images = NULL;
+
 /* Pre-load common tree nodes into the pickle cache in STREAM.  These
    nodes are always built by the front end, so there is no need to
    pickle them.
@@ -154,6 +161,7 @@ pph_stream_close (pph_stream *stream)
       destroy_output_block (stream->encoder.w.ob);
       free (stream->encoder.w.decl_state_stream);
       lto_delete_out_decl_state (stream->encoder.w.out_state);
+      VEC_free (pph_stream_ptr, heap, stream->includes);
     }
   else
     {
@@ -423,8 +431,7 @@ pph_cache_lookup (pph_stream *stream, void *data, unsigned *ix_p)
 }
 
 
-/* Return true if DATA is in the pickle cache of one of STREAM's
-   included images.
+/* Return true if DATA is in the pickle cache of one of the included images.
 
    If DATA is found:
       - the index for INCLUDE_P into IMAGE->INCLUDES is returned in
@@ -439,15 +446,15 @@ pph_cache_lookup (pph_stream *stream, void *data, unsigned *ix_p)
       - the function returns false.  */
 
 bool
-pph_cache_lookup_in_includes (pph_stream *stream, void *data,
-			      unsigned *include_ix_p, unsigned *ix_p)
+pph_cache_lookup_in_includes (void *data, unsigned *include_ix_p,
+                              unsigned *ix_p)
 {
   unsigned include_ix, ix;
   pph_stream *include;
   bool found_it;
 
   found_it = false;
-  FOR_EACH_VEC_ELT (pph_stream_ptr, stream->includes, include_ix, include)
+  FOR_EACH_VEC_ELT (pph_stream_ptr, pph_read_images, include_ix, include)
     if (pph_cache_lookup (include, data, &ix))
       {
 	found_it = true;
@@ -512,7 +519,7 @@ pph_cache_get (pph_stream *stream, unsigned include_ix, unsigned ix)
   if (include_ix == (unsigned) -1)
     image = stream;
   else
-    image = VEC_index (pph_stream_ptr, stream->includes, include_ix);
+    image = VEC_index (pph_stream_ptr, pph_read_images, include_ix);
 
   data = VEC_index (void_p, image->cache.v, ix);
   gcc_assert (data);
diff --git a/gcc/cp/pph-streamer.h b/gcc/cp/pph-streamer.h
index 8c3298a..9e790fc 100644
--- a/gcc/cp/pph-streamer.h
+++ b/gcc/cp/pph-streamer.h
@@ -179,8 +179,8 @@ typedef struct pph_stream {
     they were instantiated originally.  */
   pph_symtab symtab;
 
-  /* List of PPH files included by the PPH file that we are currently
-     generating.  Note that this list only contains PPH files, not
+  /* List of PPH files directly included by the PPH file that we are currently
+     generating/reading.  Note that this list only contains PPH files, not
      regular text headers.  Those are embedded in this stream.  */
   VEC(pph_stream_ptr,heap) *includes;
 } pph_stream;
@@ -203,11 +203,12 @@ void pph_trace_chain (pph_stream *, tree);
 void pph_trace_bitpack (pph_stream *, struct bitpack_d *);
 void pph_cache_insert_at (pph_stream *, void *, unsigned);
 bool pph_cache_lookup (pph_stream *, void *, unsigned *);
-bool pph_cache_lookup_in_includes (pph_stream *, void *, unsigned *,
-				   unsigned *);
+bool pph_cache_lookup_in_includes (void *, unsigned *, unsigned *);
 bool pph_cache_add (pph_stream *, void *, unsigned *);
 void *pph_cache_get (pph_stream *, unsigned, unsigned);
 
+extern VEC(pph_stream_ptr, heap) *pph_read_images;
+
 /* In pph-streamer-out.c.  */
 void pph_flush_buffers (pph_stream *);
 void pph_init_write (pph_stream *);
diff --git a/gcc/cp/pph.c b/gcc/cp/pph.c
index 91dc622..4779010 100644
--- a/gcc/cp/pph.c
+++ b/gcc/cp/pph.c
@@ -172,7 +172,7 @@ void
 pph_finish (void)
 {
   /* Finalize the writer.  */
-    pph_writer_finish ();
+  pph_writer_finish ();
 
   /* Finalize the reader.  */
   pph_reader_finish ();
diff --git a/gcc/testsuite/g++.dg/pph/c2deepincl.cc b/gcc/testsuite/g++.dg/pph/c2deepincl.cc
index da56b1e..bbf9402 100644
--- a/gcc/testsuite/g++.dg/pph/c2deepincl.cc
+++ b/gcc/testsuite/g++.dg/pph/c2deepincl.cc
@@ -1,5 +1,3 @@
-// pph asm xdiff 00611
-
 #include "c2deepincl3.h"
 
 int test() {

--
This patch is available for review at http://codereview.appspot.com/4904050