diff mbox

[pph] Fix references to external PPH files (issue5476043)

Message ID 20111208212220.16342AE1DB@tobiano.tor.corp.google.com
State New
Headers show

Commit Message

Diego Novillo Dec. 8, 2011, 9:22 p.m. UTC 
This fixes a bug in the creation of external references to PPH images.

We save references to external PPH files when we save the line table for
the parent header.  Sometimes we were not saving a proper reference
because the path names of the header as found in the line map could
not be matched with the external PPH file.

This fixes the problem by encoding the path name of the original
header file into each PPH file.  This way, we do not have to play
guessing games when saving the line table.  We are still dependent on
the path names of the original headers to be in the same location that
the PPH was generated from.  If a header moves to a different
location, then we won't be able to match it to a PPH image.

The final solution will involve being cognizant of the path scheme
used by libcpp, but this will serve for the time being.

Tested on x86_64.  Committed.


Diego.

	* pph-streamer.h (struct pph_stream): Add field HEADER_NAME.
	* pph-in.c (pph_in_line_table_and_includes): Read it.
	* pph-out.c (pph_init_write): Initialize it from input_filename.
	(pph_filename_eq_ignoring_path): Remove.
	(pph_out_line_table_and_includes): Write STREAM->HEADER_NAME.
	Compare the file name in LM against STREAM->HEADER_NAME to
	decide whether to write a reference to an external PPH.


--
This patch is available for review at http://codereview.appspot.com/5476043
diff mbox

Patch

diff --git a/gcc/cp/pph-in.c b/gcc/cp/pph-in.c
index 52a56aa..323516d 100644
--- a/gcc/cp/pph-in.c
+++ b/gcc/cp/pph-in.c
@@ -355,6 +355,10 @@  pph_in_line_table_and_includes (pph_stream *stream)
   used_before = LINEMAPS_ORDINARY_USED (line_table);
   first = true;
 
+  /* Read the path name of the original text header file that was
+     used to generate STREAM.  */
+  stream->header_name = 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.  */
   top_includer_ix = used_before - 1;
diff --git a/gcc/cp/pph-out.c b/gcc/cp/pph-out.c
index e970f86..01c364a 100644
--- a/gcc/cp/pph-out.c
+++ b/gcc/cp/pph-out.c
@@ -65,6 +65,10 @@  pph_init_write (pph_stream *stream)
   /* Associate STREAM with stream->encoder.w.ob so we can recover it from the
      streamer hooks.  */
   stream->encoder.w.ob->sdata = (void *) 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);
 }
 
 
@@ -280,45 +284,6 @@  pph_out_include (pph_stream *stream, pph_stream *include,
 }
 
 
-/* Compare filenames of a header and it's potentially corresponding pph file,
-   stripping the path passed in and the extension. Returns true if HEADER_PATH
-   and PPH_PATH end with the same filename. We expect HEADER_PATH to end in .h
-   and PPH_PATH to end in .pph.
-
-   FIXME pph: We should not need to do this if we handled include paths
-   correctly, but for now the linemap holds full paths and the stream's includes
-   list only holds the include name.  Also, the stream's includes hold pph
-   filenames where as the line_table as header filenames.  */
-
-static bool
-pph_filename_eq_ignoring_path (const char *header_path, const char *pph_path)
-{
-  const char *header_name = lbasename (header_path);
-  const char *pph_name = lbasename (pph_path);
-
-  const char *header_ext = strchr (header_name, '.');
-  const char *pph_ext = strchr (pph_name, '.');
-
-  unsigned int name_length;
-
-  if (header_ext != NULL)
-    {
-      name_length = header_ext - header_name;
-      gcc_assert (strcmp (header_ext, ".h") == 0);
-    }
-  else
-    /* Some headers do not have a .h suffix, but will still
-       have a .pph suffix after being pph'ed.  */
-    name_length = strlen (header_name);
-
-  gcc_assert (strcmp (pph_ext, ".pph") == 0);
-
-  /* Compare the filenames without their extension.  */
-  return pph_ext - pph_name == name_length
-         && strncmp (header_name, pph_name, name_length) == 0;
-}
-
-
 /* 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.  */
@@ -351,6 +316,10 @@  pph_out_line_table_and_includes (pph_stream *stream)
   /* Any #include should have been fully parsed and exited at this point.  */
   gcc_assert (line_table->depth == 0);
 
+  /* Write the path name of the original text header file that was
+     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;
@@ -379,8 +348,7 @@  pph_out_line_table_and_includes (pph_stream *stream)
 	 reference to it, so the reader can load the included image at
 	 this point.  */
       if (current_include != NULL
-	  && pph_filename_eq_ignoring_path (LINEMAP_FILE (lm),
-	                                    current_include->name))
+	  && strcmp (current_include->header_name, LINEMAP_FILE (lm)) == 0)
 	{
 	  struct line_map *included_from;
 
diff --git a/gcc/cp/pph-streamer.h b/gcc/cp/pph-streamer.h
index e31b15e..0b8af3d 100644
--- a/gcc/cp/pph-streamer.h
+++ b/gcc/cp/pph-streamer.h
@@ -164,6 +164,16 @@  struct pph_stream {
   /* Path name of the PPH file.  */
   const char *name;
 
+  /* Path name of the original text header file.  This is the full
+     path name of the header as found by the pre-processor after
+     doing include path expansion.  Note that this may not necessarily
+     be an absolute path name.
+
+     We rely on this path name to identify the corresponding PPH name
+     when deciding whether to load external PPH files in
+     pph_in_line_table_and_includes.  */
+  const char *header_name;
+
   /* FILE object associated with it.  */
   FILE *file;