[pph] Consult PPH registry when processing LC_ENTER events (issue5539074)

Diego Novillo dnovillo@google.com
Tue Jan 17 15:17:00 GMT 2012


This patch changes pph_out_line_table_and_includes to use the registry
when deciding whether an LC_ENTER linemap is entering a PPH image.

We used to keep track of entered images by using a "next" index on the
table of read PPH files.  The problem with this is that this table
contains the transitive closure of all the read images and the loop in
pph_out_line_table_and_includes skips over all the entries of PPH
images.

So, if a PPH file includes another, we would miss the second one and
ICE at the end of the loop.

With the new registry, we simply examine all the LC_ENTER linemaps
after PPH_NUM_IGNORED_LINE_TABLE_ENTRIES.  If the file being entered
is in the registry, then we are entering a PPH image and we have to
skip all the linemaps until we leave that file (because those will be
processed when we read that image).

This fixes the test failures in the first patch of this series.

	* pph-core.c (pph_stream_register): Move later in the file.
	(pph_stream_unregister): Likewise.
	(pph_stream_registry_ix_for): Fix comparison to !=.
	(pph_stream_registry_add_name): Make extern.
	Allocate a new slot in the table.
	(pph_stream_set_header_name): New.
	(pph_stream_registry_lookup): Fix computation of SLOT_IX.
	(pph_stream_open): Call pph_stream_register earlier.
	* pph-in.c (pph_in_line_table_and_includes): Call
	pph_stream_set_header_name.
	* pph-out.c (pph_init_write): Likewise.
	(pph_get_next_include): Remove.  Update all users.
	(pph_out_line_table_and_includes): Call
	pph_stream_registry_lookup to determine if the file being
	entered is a PPH image.
	* pph-streamer.h (pph_stream_registry_lookup): Declare.
	(pph_stream_set_header_name): Declare.

git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/branches/pph@183231 138bc75d-0d04-0410-961f-82ee72b054a4
---
 gcc/cp/ChangeLog.pph  |   20 ++++++++
 gcc/cp/pph-core.c     |  120 ++++++++++++++++++++++++++++---------------------
 gcc/cp/pph-in.c       |    2 +-
 gcc/cp/pph-out.c      |   34 +++-----------
 gcc/cp/pph-streamer.h |    2 +
 5 files changed, 98 insertions(+), 80 deletions(-)

diff --git a/gcc/cp/ChangeLog.pph b/gcc/cp/ChangeLog.pph
index 7e5b90f..b545e05 100644
--- a/gcc/cp/ChangeLog.pph
+++ b/gcc/cp/ChangeLog.pph
@@ -1,5 +1,25 @@
 2012-01-16   Diego Novillo  <dnovillo@google.com>
 
+	* pph-core.c (pph_stream_register): Move later in the file.
+	(pph_stream_unregister): Likewise.
+	(pph_stream_registry_ix_for): Fix comparison to !=.
+	(pph_stream_registry_add_name): Make extern.
+	Allocate a new slot in the table.
+	(pph_stream_set_header_name): New.
+	(pph_stream_registry_lookup): Fix computation of SLOT_IX.
+	(pph_stream_open): Call pph_stream_register earlier.
+	* pph-in.c (pph_in_line_table_and_includes): Call
+	pph_stream_set_header_name.
+	* pph-out.c (pph_init_write): Likewise.
+	(pph_get_next_include): Remove.  Update all users.
+	(pph_out_line_table_and_includes): Call
+	pph_stream_registry_lookup to determine if the file being
+	entered is a PPH image.
+	* pph-streamer.h (pph_stream_registry_lookup): Declare.
+	(pph_stream_set_header_name): Declare.
+
+2012-01-16   Diego Novillo  <dnovillo@google.com>
+
 	* pph-core.c (struct pph_name_stream_map): Declare.
 	(struct pph_stream_registry_d): Declare.
 	(pph_stream_registry): New.
diff --git a/gcc/cp/pph-core.c b/gcc/cp/pph-core.c
index 57dc53d..5d22c5e 100644
--- a/gcc/cp/pph-core.c
+++ b/gcc/cp/pph-core.c
@@ -855,48 +855,6 @@ pph_loaded (void)
 /*********************************************************** stream handling */
 
 
-/* Register STREAM in the table of open streams.  */
-
-static void
-pph_stream_register (pph_stream *stream)
-{
-  void **slot;
-  unsigned vlen;
-
-  slot = pointer_map_insert (pph_stream_registry.image_ix, stream);
-
-  /* Disallow multpile registration of the same STREAM.  This is overly
-     strict, but it prevents some unnecessary overhead.  */
-  gcc_assert (*slot == NULL);
-
-  VEC_safe_push (pph_stream_ptr, heap, pph_stream_registry.v, stream);
-  vlen = VEC_length (pph_stream_ptr, pph_stream_registry.v);
-  *slot = (void *)(intptr_t) (vlen - 1);
-}
-
-
-/* Unregister STREAM from the table of open streams.  */
-
-static void
-pph_stream_unregister (pph_stream *stream)
-{
-  void **slot;
-  unsigned ix;
-
-  slot = pointer_map_contains (pph_stream_registry.image_ix, stream);
-  gcc_assert (slot);
-  ix = (unsigned)(intptr_t) *slot;
-
-  /* Mark it unregistered in the image index.  */
-  *slot = (void *)(intptr_t) -1;
-
-  /* Remove it from the image list.  Note that we do not need to
-     remove the index from the name index.  Any further lookups will
-     simply return NULL.  */
-  VEC_replace (pph_stream_ptr, pph_stream_registry.v, ix, NULL);
-}
-
-
 /* Return the index into the registry for STREAM.  If STREAM has not been
    registered yet, return -1.  */
 
@@ -918,13 +876,13 @@ pph_stream_registry_ix_for (pph_stream *stream)
 static bool
 pph_stream_registered_p (pph_stream *stream)
 {
-  return pph_stream_registry_ix_for (stream) == -1u;
+  return pph_stream_registry_ix_for (stream) != -1u;
 }
 
 
 /* Associate string NAME with the registry entry for STREAM.  */
 
-void
+static void
 pph_stream_registry_add_name (pph_stream *stream, const char *name)
 {
   void **slot;
@@ -938,10 +896,22 @@ pph_stream_registry_add_name (pph_stream *stream, const char *name)
   e.ix = pph_stream_registry_ix_for (stream);
   slot = htab_find_slot (pph_stream_registry.name_ix, &e, INSERT);
   gcc_assert (*slot == NULL);
+  *slot = (void *) XNEW (pph_name_stream_map);
   memcpy ((pph_name_stream_map *) *slot, &e, sizeof (e));
 }
 
 
+/* Set NAME to be STREAM's full pathname for the corresponding header
+   file.  */
+
+void
+pph_stream_set_header_name (pph_stream *stream, const char *name)
+{
+  stream->header_name = name;
+  pph_stream_registry_add_name (stream, name);
+}
+
+
 /* Return the PPH stream associated with NAME.  Return NULL if no such
    mapping exist.  */
 
@@ -950,7 +920,7 @@ pph_stream_registry_lookup (const char *name)
 {
   void **slot;
   intptr_t slot_ix;
-  pph_name_stream_map e;
+  struct pph_name_stream_map e;
 
   e.name = name;
   e.ix = -1u;
@@ -958,10 +928,53 @@ pph_stream_registry_lookup (const char *name)
   if (slot == NULL)
     return NULL;
 
-  slot_ix = (intptr_t) *slot;
-  gcc_assert (slot_ix == (intptr_t)(unsigned) slot_ix);
-  e.ix = (unsigned) slot_ix;
-  return VEC_index (pph_stream_ptr, pph_stream_registry.v, e.ix);
+  slot_ix = ((struct pph_name_stream_map *) *slot)->ix;
+  return VEC_index (pph_stream_ptr, pph_stream_registry.v, slot_ix);
+}
+
+
+/* Register STREAM in the table of open streams.  */
+
+static void
+pph_stream_register (pph_stream *stream)
+{
+  void **slot;
+  unsigned vlen;
+
+  slot = pointer_map_insert (pph_stream_registry.image_ix, stream);
+
+  /* Disallow multpile registration of the same STREAM.  This is overly
+     strict, but it prevents some unnecessary overhead.  */
+  gcc_assert (*slot == NULL);
+
+  VEC_safe_push (pph_stream_ptr, heap, pph_stream_registry.v, stream);
+  vlen = VEC_length (pph_stream_ptr, pph_stream_registry.v);
+  *slot = (void *)(intptr_t) (vlen - 1);
+
+  /* Add a mapping between STREAM's PPH file name and STREAM.  */
+  pph_stream_registry_add_name (stream, stream->name);
+}
+
+
+/* Unregister STREAM from the table of open streams.  */
+
+static void
+pph_stream_unregister (pph_stream *stream)
+{
+  void **slot;
+  unsigned ix;
+
+  slot = pointer_map_contains (pph_stream_registry.image_ix, stream);
+  gcc_assert (slot);
+  ix = (unsigned)(intptr_t) *slot;
+
+  /* Mark it unregistered in the image index.  */
+  *slot = (void *)(intptr_t) -1;
+
+  /* Remove it from the image list.  Note that we do not need to
+     remove the index from the name index.  Any further lookups will
+     simply return NULL.  */
+  VEC_replace (pph_stream_ptr, pph_stream_registry.v, ix, NULL);
 }
 
 
@@ -993,14 +1006,17 @@ pph_stream_open (const char *name, const char *mode)
   stream->write_p = (strchr (mode, 'w') != NULL);
   pph_cache_init (&stream->cache);
   stream->preloaded_cache = pph_preloaded_cache;
+
+  /* Register STREAM in the table of all open streams.  Do it before
+     initializing the reader or writer since they may need to look it
+     up in the registry.  */
+  pph_stream_register (stream);
+
   if (stream->write_p)
     pph_init_write (stream);
   else
     pph_init_read (stream);
 
-  /* Register STREAM in the table of all open streams.  */
-  pph_stream_register (stream);
-
   return stream;
 }
 
diff --git a/gcc/cp/pph-in.c b/gcc/cp/pph-in.c
index a67ffa7..6a8fbf8 100644
--- a/gcc/cp/pph-in.c
+++ b/gcc/cp/pph-in.c
@@ -357,7 +357,7 @@ pph_in_line_table_and_includes (pph_stream *stream)
 
   /* Read the path name of the original text header file that was
      used to generate STREAM.  */
-  stream->header_name = pph_in_string (stream);
+  pph_stream_set_header_name (stream, pph_in_string (stream));
 
   /* All line map entries that have -1 as the includer, will now be
      relocated to the current last line map entry in the line table.  */
diff --git a/gcc/cp/pph-out.c b/gcc/cp/pph-out.c
index 4aa1d65..08c9862 100644
--- a/gcc/cp/pph-out.c
+++ b/gcc/cp/pph-out.c
@@ -68,7 +68,7 @@ pph_init_write (pph_stream *stream)
 
   /* Since we are about to generate STREAM, its header name is the
      name of the header file that we are currently parsing.  */
-  stream->header_name = xstrdup (input_filename);
+  pph_stream_set_header_name (stream, xstrdup (input_filename));
 }
 
 
@@ -298,20 +298,6 @@ pph_out_include (pph_stream *stream, pph_stream *include,
 }
 
 
-/* Return the *NEXT_INCLUDE_IX'th pph_stream in STREAM's list of includes.
-   Returns NULL if we have read all includes.  Increments *NEXT_INCLUDE_IX
-   when sucessful.  */
-
-static inline pph_stream *
-pph_get_next_include (pph_stream *stream, unsigned int *next_incl_ix)
-{
-  if (*next_incl_ix < VEC_length (pph_stream_ptr, stream->includes))
-    return VEC_index (pph_stream_ptr, stream->includes, (*next_incl_ix)++);
-  else
-    return NULL;
-}
-
-
 /* Emit the required line_map entry (those directly related to this include)
    and some properties in the line_table to STREAM, ignoring builtin and
    command-line entries.  We will write references to our direct includes
@@ -323,9 +309,7 @@ pph_get_next_include (pph_stream *stream, unsigned int *next_incl_ix)
 static void
 pph_out_line_table_and_includes (pph_stream *stream)
 {
-  unsigned int next_incl_ix = 0;
   int ix;
-  pph_stream *current_include;
 
   /* Any #include should have been fully parsed and exited at this point.  */
   gcc_assert (line_table->depth == 0);
@@ -334,12 +318,11 @@ pph_out_line_table_and_includes (pph_stream *stream)
      used to generate STREAM.  */
   pph_out_string (stream, stream->header_name);
 
-  current_include = pph_get_next_include (stream, &next_incl_ix);
-
   for (ix = PPH_NUM_IGNORED_LINE_TABLE_ENTRIES;
        ix < (int) LINEMAPS_ORDINARY_USED (line_table);
        ix++)
     {
+      pph_stream *current_include;
       struct line_map *lm = LINEMAPS_ORDINARY_MAP_AT (line_table, ix);
 
       /* FIXME pph.  We currently do not support location tracking for
@@ -361,8 +344,11 @@ pph_out_line_table_and_includes (pph_stream *stream)
       /* If LM is an entry for an included PPH image, output a line table
 	 reference to it, so the reader can load the included image at
 	 this point.  */
-      if (current_include != NULL
-	  && strcmp (current_include->header_name, LINEMAP_FILE (lm)) == 0)
+      current_include = (lm->reason == LC_ENTER
+	                 && ix > PPH_NUM_IGNORED_LINE_TABLE_ENTRIES)
+	                ? pph_stream_registry_lookup (LINEMAP_FILE (lm))
+			: NULL;
+      if (current_include)
 	{
 	  struct line_map *included_from;
 
@@ -404,8 +390,6 @@ pph_out_line_table_and_includes (pph_stream *stream)
 	    /* We should always leave this loop before the end of the
 		current line_table entries.  */
 	    gcc_assert (ix < (int) LINEMAPS_ORDINARY_USED (line_table));
-
-	  current_include = pph_get_next_include (stream, &next_incl_ix);
 	}
       else
 	{
@@ -424,10 +408,6 @@ pph_out_line_table_and_includes (pph_stream *stream)
   pph_out_uint (stream, LINEMAPS_ORDINARY_USED (line_table)
                         - PPH_NUM_IGNORED_LINE_TABLE_ENTRIES);
 
-  /* Every PPH header included should have been seen and skipped in the
-     line_table streaming above.  */
-  gcc_assert (next_incl_ix == VEC_length (pph_stream_ptr, stream->includes));
-
   pph_out_source_location (stream, line_table->highest_location);
   pph_out_source_location (stream, line_table->highest_line);
 
diff --git a/gcc/cp/pph-streamer.h b/gcc/cp/pph-streamer.h
index a32d0fe..7b406f6 100644
--- a/gcc/cp/pph-streamer.h
+++ b/gcc/cp/pph-streamer.h
@@ -250,6 +250,8 @@ enum pph_trace_end
 /* In pph-core.c.  */
 const char *pph_tree_code_text (enum tree_code code);
 void pph_dump_namespace (FILE *, tree ns);
+pph_stream *pph_stream_registry_lookup (const char *);
+void pph_stream_set_header_name (pph_stream *, const char *);
 pph_stream *pph_stream_open (const char *, const char *);
 void pph_stream_close (pph_stream *);
 void pph_add_include (pph_stream *, pph_stream *);
-- 
1.7.7.3


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



More information about the Gcc-patches mailing list