This patch adds a check for #include_next and refuses to generate a PPH image for ...
12 years, 3 months ago
(2012-01-18 22:26:52 UTC)
#1
This patch adds a check for #include_next and refuses to generate a
PPH image for files that use it.
Since the discovery of #include_next comes well into the
pre-processing stage (after we've initialized PPH data), we need to
undo what we did and disable PPH. To do that, there is a new function
pph_disable_output() which closes and unlinks the output file. This
needed changes to pph_stream_close() to not bother writing the
encoding buffers.
To decide whether we see the #include_next command in the primary
source file, I exported the function cpp_in_primary_file() from
libcpp.
Finally, the patch adds checks to avoid getting into an infinite loop
when processing includes. Some system files include each other and if
we produce PPH images for both files, we were getting into an infinite
mutually-referential cycle.
libcpp/ChangeLog.pph
* internal.h (cpp_in_primary_file): Move ...
* files.c (cpp_in_primary_file): ... here.
* include/cpplib.h (cpp_in_primary_file): Declare.
gcc/cp/ChangeLog.pph
* pph-core.c (pph_include_handler): If the primary file
uses #include_next, emit a warning and call
pph_disable_output.
(pph_stream_close_1): Factor out of ...
(pph_stream_close): ... here.
(pph_stream_close_no_flush): New.
(pph_streamer_finish): Call pph_writer_finish if
pph_writer_enabled_p returns true.
Call pph_reader_finish if pph_reader_enabled_p returns
true.
* pph-in.c (pph_in_include): Assert that we are not
trying to read STREAM from STREAM.
* pph-out.c (pph_out_line_table_and_includes): Do not
emit a line table reference to STREAM.
(pph_disable_output): New.
* pph-streamer.h (pph_stream_close_no_flush): Declare.
(pph_disable_output): Declare.
gcc/testsuite/ChangeLog.pph
* g++.dg/pph/d0include-next.h: New.
git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/branches/pph@183291
138bc75d-0d04-0410-961f-82ee72b054a4
---
gcc/cp/ChangeLog.pph | 20 +++++++++
gcc/cp/pph-core.c | 67 +++++++++++++++++++++++++---
gcc/cp/pph-in.c | 3 +
gcc/cp/pph-out.c | 24 +++++++++-
gcc/cp/pph-streamer.h | 2 +
gcc/testsuite/ChangeLog.pph | 4 ++
gcc/testsuite/g++.dg/pph/d0include-next.h | 6 +++
libcpp/ChangeLog.pph | 6 +++
libcpp/files.c | 9 ++++
libcpp/include/cpplib.h | 1 +
libcpp/internal.h | 7 ---
11 files changed, 132 insertions(+), 17 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/pph/d0include-next.h
diff --git a/gcc/cp/ChangeLog.pph b/gcc/cp/ChangeLog.pph
index 39ac194..47d9bc04 100644
--- a/gcc/cp/ChangeLog.pph
+++ b/gcc/cp/ChangeLog.pph
@@ -1,3 +1,23 @@
+2012-01-18 Diego Novillo <dnovillo@google.com>
+
+ * pph-core.c (pph_include_handler): If the primary file
+ uses #include_next, emit a warning and call
+ pph_disable_output.
+ (pph_stream_close_1): Factor out of ...
+ (pph_stream_close): ... here.
+ (pph_stream_close_no_flush): New.
+ (pph_streamer_finish): Call pph_writer_finish if
+ pph_writer_enabled_p returns true.
+ Call pph_reader_finish if pph_reader_enabled_p returns
+ true.
+ * pph-in.c (pph_in_include): Assert that we are not
+ trying to read STREAM from STREAM.
+ * pph-out.c (pph_out_line_table_and_includes): Do not
+ emit a line table reference to STREAM.
+ (pph_disable_output): New.
+ * pph-streamer.h (pph_stream_close_no_flush): Declare.
+ (pph_disable_output): Declare.
+
2012-01-17 Lawrence Crowl <crowl@google.com>
* pph.h (pph_files_read): New.
diff --git a/gcc/cp/pph-core.c b/gcc/cp/pph-core.c
index f470cfc..e7cf444 100644
--- a/gcc/cp/pph-core.c
+++ b/gcc/cp/pph-core.c
@@ -669,6 +669,21 @@ pph_include_handler (cpp_reader *reader,
fprintf (pph_logfile, "%c\n", angle_brackets ? '>' : '"');
}
+ /* If we find a #include_next directive in the primary file,
+ refuse to generate a PPH image for it. #include_next cannot
+ be resolved from the primary source file, so generating an
+ image for it would cause an infinite self-referential loop
+ in the line table. */
+ if (cpp_in_primary_file (reader)
+ && strcmp ((const char *)dname, "include_next") == 0)
+ {
+ warning_at (loc, OPT_Winvalid_pph, "#include_next found in the "
+ "primary source file. PPH generation disabled for %s",
+ LOCATION_FILE (loc));
+ pph_disable_output ();
+ return true;
+ }
+
read_text_file_p = true;
pph_file = query_pph_include_map (name);
if (pph_file != NULL
@@ -1026,10 +1041,15 @@ pph_stream_open (const char *name, const char *mode)
}
-/* Close PPH stream STREAM. */
+/* Close PPH stream STREAM. If FLUSH_P is true and STREAM was being
+ written to, then STREAM's encoding buffers are flushed before
+ closing it. Otherwise, STREAM is closed without flushing internal
+ buffers and its associated file is removed. This is used when an
+ exceptional condition occurs that prevents us from generating a PPH
+ image. */
-void
-pph_stream_close (pph_stream *stream)
+static void
+pph_stream_close_1 (pph_stream *stream, bool flush_p)
{
/* STREAM can be NULL if it could not be properly opened. An error
has already been emitted, so avoid crashing here. */
@@ -1039,13 +1059,23 @@ pph_stream_close (pph_stream *stream)
if (flag_pph_tracer >= 1)
fprintf (pph_logfile, "PPH: Closing %s\n", stream->name);
- /* If we were writing to STREAM, flush all the memory buffers. This
- does the actual writing of all the pickled data structures. */
- if (stream->write_p)
+ /* If we were writing to STREAM and the caller tells us to, flush
+ all the memory buffers. This does the actual writing of all the
+ pickled data structures. */
+ if (stream->write_p && flush_p)
pph_flush_buffers (stream);
fclose (stream->file);
+ /* If we were writing but the caller did not want STREAM's buffers
+ flushed, remove the PPH file. */
+ if (stream->write_p && !flush_p)
+ {
+ if (flag_pph_tracer >= 1)
+ fprintf (pph_logfile, "PPH: Removing %s", stream->name);
+ unlink (stream->name);
+ }
+
/* Deallocate all memory used. */
stream->file = NULL;
VEC_free (pph_cache_entry, heap, stream->cache.v);
@@ -1078,6 +1108,25 @@ pph_stream_close (pph_stream *stream)
}
+/* Close PPH stream STREAM. If STREAM was being written to, flush its
+ encoding buffers. */
+
+void
+pph_stream_close (pph_stream *stream)
+{
+ pph_stream_close_1 (stream, true);
+}
+
+
+/* Close PPH stream STREAM. If STREAM was being written to, do not
+ flush its encoding buffers and remove the associated file. */
+
+void
+pph_stream_close_no_flush (pph_stream *stream)
+{
+ pph_stream_close_1 (stream, false);
+}
+
/********************************************************** stream callbacks */
@@ -1232,10 +1281,12 @@ pph_streamer_finish (void)
pph_stream *image;
/* Finalize the writer. */
- pph_writer_finish ();
+ if (pph_writer_enabled_p ())
+ pph_writer_finish ();
/* Finalize the reader. */
- pph_reader_finish ();
+ if (pph_reader_enabled_p ())
+ pph_reader_finish ();
/* Close any images read during parsing. */
FOR_EACH_VEC_ELT (pph_stream_ptr, pph_stream_registry.v, i, image)
diff --git a/gcc/cp/pph-in.c b/gcc/cp/pph-in.c
index aa82e3a..2dbad83 100644
--- a/gcc/cp/pph-in.c
+++ b/gcc/cp/pph-in.c
@@ -327,6 +327,9 @@ pph_in_include (pph_stream *stream)
old_loc_offset = pph_loc_offset;
include_name = pph_in_string (stream);
+
+ /* We should not be trying to include STREAM again. */
+ gcc_assert (strcmp (include_name, stream->name) != 0);
include_stream = pph_read_file (include_name);
/* Add INCLUDE_STREAM, and the images included by it, to the list
diff --git a/gcc/cp/pph-out.c b/gcc/cp/pph-out.c
index f60438d..e80ad7b 100644
--- a/gcc/cp/pph-out.c
+++ b/gcc/cp/pph-out.c
@@ -343,12 +343,15 @@ 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. */
+ this point. Make sure we do not emit self-referential
+ entries (even if a file is properly guarded against double
+ inclusion, there will be linemap entries in the line table
+ for it). */
current_include = (lm->reason == LC_ENTER
&& ix > PPH_NUM_IGNORED_LINE_TABLE_ENTRIES)
? pph_stream_registry_lookup (LINEMAP_FILE (lm))
: NULL;
- if (current_include)
+ if (current_include && current_include != stream)
{
struct line_map *included_from;
@@ -2794,3 +2797,20 @@ pph_writer_add_include (pph_stream *include)
{
pph_add_include (pph_out_stream, include);
}
+
+
+/* Disable PPH generation. Used when we discover that the file that we
+ are currently converting into PPH is not compatible. This does not
+ necessarily stop compilation, so make sure that the PPH file is
+ not generated. */
+
+void
+pph_disable_output (void)
+{
+ /* If we are not generating a PPH image, do nothing. */
+ if (!pph_out_stream)
+ return;
+
+ pph_stream_close_no_flush (pph_out_stream);
+ pph_out_file = NULL;
+}
diff --git a/gcc/cp/pph-streamer.h b/gcc/cp/pph-streamer.h
index 21b1c24..b641776 100644
--- a/gcc/cp/pph-streamer.h
+++ b/gcc/cp/pph-streamer.h
@@ -254,6 +254,7 @@ 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_stream_close_no_flush (pph_stream *);
void pph_add_include (pph_stream *, pph_stream *);
void pph_trace_marker (enum pph_record_marker marker, enum pph_tag tag);
void pph_trace_tree (tree, enum pph_trace_end, enum pph_trace_kind);
@@ -277,6 +278,7 @@ void pph_writer_init (void);
void pph_writer_finish (void);
void pph_out_location (pph_stream *, location_t);
void pph_out_tree (pph_stream *, tree);
+void pph_disable_output (void);
/* In pph-in.c. */
void pph_init_read (pph_stream *);
diff --git a/gcc/testsuite/ChangeLog.pph b/gcc/testsuite/ChangeLog.pph
index 282f4d7..f7e3bdf 100644
--- a/gcc/testsuite/ChangeLog.pph
+++ b/gcc/testsuite/ChangeLog.pph
@@ -1,3 +1,7 @@
+2012-01-18 Diego Novillo <dnovillo@google.com>
+
+ * g++.dg/pph/d0include-next.h: New.
+
2011-01-17 Lawrence Crowl <crowl@google.com>
* g++.dg/pph/x6dynarray3.cc: Add expected overload failures.
diff --git a/gcc/testsuite/g++.dg/pph/d0include-next.h
b/gcc/testsuite/g++.dg/pph/d0include-next.h
new file mode 100644
index 0000000..e4e9366
--- /dev/null
+++ b/gcc/testsuite/g++.dg/pph/d0include-next.h
@@ -0,0 +1,6 @@
+/* { dg-options "-Winvalid-pph" } */
+/* We do not support #include_next in PPH images. */
+#ifndef D0INCLUDE_NEXT_H
+#define D0INCLUDE_NEXT_H
+#include_next "d0include-next.h" { dg-warning ".*PPH generation disabled" }
+#endif
diff --git a/libcpp/ChangeLog.pph b/libcpp/ChangeLog.pph
index f876c8f..a74344f 100644
--- a/libcpp/ChangeLog.pph
+++ b/libcpp/ChangeLog.pph
@@ -1,3 +1,9 @@
+2012-01-18 Diego Novillo <dnovillo@google.com>
+
+ * internal.h (cpp_in_primary_file): Move ...
+ * files.c (cpp_in_primary_file): ... here.
+ * include/cpplib.h (cpp_in_primary_file): Declare.
+
2011-11-22 Diego Novillo <dnovillo@google.com>
* include/line-map.h (linemap_dump): Declare.
diff --git a/libcpp/files.c b/libcpp/files.c
index 5e9f3ea..74c2783 100644
--- a/libcpp/files.c
+++ b/libcpp/files.c
@@ -1936,3 +1936,12 @@ cpp_return_at_eof (cpp_buffer *buffer, bool val)
{
buffer->return_at_eof = val;
}
+
+
+/* Return true if PFILE is currently reading the primary file. */
+
+bool
+cpp_in_primary_file (cpp_reader *pfile)
+{
+ return pfile->line_table->depth == 1;
+}
diff --git a/libcpp/include/cpplib.h b/libcpp/include/cpplib.h
index 250ef57..0e3f121 100644
--- a/libcpp/include/cpplib.h
+++ b/libcpp/include/cpplib.h
@@ -1070,6 +1070,7 @@ extern void cpp_return_at_eof (cpp_buffer *, bool);
/* Return the path name of the main file iff it does not have a
multiple include guard. */
extern const char *cpp_main_missing_guard (cpp_reader *pfile);
+extern bool cpp_in_primary_file (cpp_reader *);
/* In pch.c */
struct save_macro_data;
diff --git a/libcpp/internal.h b/libcpp/internal.h
index 30484ca..9b35507 100644
--- a/libcpp/internal.h
+++ b/libcpp/internal.h
@@ -646,13 +646,6 @@ cpp_in_system_header (cpp_reader *pfile)
#define CPP_PEDANTIC(PF) CPP_OPTION (PF, cpp_pedantic)
#define CPP_WTRADITIONAL(PF) CPP_OPTION (PF, cpp_warn_traditional)
-static inline int cpp_in_primary_file (cpp_reader *);
-static inline int
-cpp_in_primary_file (cpp_reader *pfile)
-{
- return pfile->line_table->depth == 1;
-}
-
/* In macro.c */
extern void _cpp_free_definition (cpp_hashnode *);
extern bool _cpp_create_definition (cpp_reader *, cpp_hashnode *);
--
1.7.7.3
--
This patch is available for review at http://codereview.appspot.com/5558045
On 1/28/12 5:00 PM, Gabriel Charette wrote: > Why do you need two functions: pph_stream_close ...
12 years, 3 months ago
(2012-01-29 17:42:49 UTC)
#3
On 1/28/12 5:00 PM, Gabriel Charette wrote:
> Why do you need two functions: pph_stream_close and
> pph_stream_close_no_flush only to call pph_stream_close_1 with a
> different flag? Seems like leaving
> pph_stream_close_1 as pph_stream_close and simply making the calls
> themselves with true/false flags is simpler...
Well, I find it clearer to have more explicit names for the API than
using flags.
Diego.
Issue 5558045: [pph] Refuse to generate PPH with #include_next
(Closed)
Created 12 years, 3 months ago by Diego Novillo
Modified 12 years, 1 month ago
Reviewers: gcharette1
Base URL:
Comments: 0
