Re: [Bug pch/14400] Cannot compile qt-x11-free-3.3.0

[Ian Lance Taylor <ian at wasabisystems dot com>]

Geoff Keating <geoffk@apple.com> writes:

> On Apr 3, 2004, at 7:38 PM, Ian Lance Taylor wrote:
> 
> > Any thoughts on how difficult it would be to, when a PCH can not be
> > loaded at the correct address, skip the PCH and try for the original
> > header files?  We'd want to issue a warning, but the result will still
> > be better then a compiler crash, particularly since the crash will be
> > awkward to work around without manually removing the PCH.  Of course,
> > the result will not necessarily be equivalent--the original headers
> > might not be present, or might have been modified since the PCH was
> > created.
> 
> The primary difficulty that I see would be in documenting this, that
> is that the result is not convenient to use.  At the moment, the
> documentation for PCH describes everything that you need to do to use
> a PCH, and it's agreed that cases where it does not work are bugs.
> Adding an extra clause that says "oh, and even if you follow these
> rules, on some platforms the compiler might just decide it doesn't
> like your PCH" does not seem very helpful.  Especially if it doesn't
> come with a list of those platforms.

Well, that is pretty much the state we are in right now.  Today, even
if you follow the listed rules, on some platforms the compiler might
decide to crash rather than use your PCH.  We can 1) ignore this
restriction, or 2) we can document it, or 3) we can fix it immediately
on every single host which gcc runs on.  I think option 3 is
unachievable, and I think that option 2 is better than option 1.

> The other difficulty is that I'm not sure that you can always back out
> of the address-space allocation.  I would probably have to see a patch
> before I could say how difficult that might be.

Any thoughts on this patch?


2004-04-07  Ian Lance Taylor  <ian@wasabisystems.com>

	* c-pch.c (struct c_pch_mmap_info): Define.
	(struct c_pch_validity): Add fields c_pch_validity_size and mmi.
	(get_ident): Bump version number.
	(pch_init): Set new c_pch_validity fields.
	(c_common_write_pch): Record mmi information in PCH.
	(c_common_valid_pch): Add pdata parameter.  Check size of v
	against value recorded in file.  Map file data into memory.
	(c_common_read_pch): Add data parameter.
	* cppfiles.c (struct _cpp_file): Add pch_data field.
	(should_stack_file): Pass pch_data to read_pch.
	(validate_pch): Pass address of pch_data to valid_pch.
	* ggc-common.c (struct mmap_info): Remove.
	(gt_pch_save): Add p_offset, p_size, and p_preferred_base
	parameters.  Change all callers.  Don't write mmi into file.
	(gt_pch_restore): Add offset, size, and preferred_base
	parameters.  Change all callers.  Don't read mmi.  Don't read
	mapped information.
	* c-common.h (c_common_valid_pch): Update declaration.
	(c_common_read_pch): Likewise.
	* cpplib.h (struct cpp_callbacks): Add void ** parameter to
	valid_pch.  Add void * parameter to read_pch.
	* ggc.h (gt_pch_save): Update declaration.
	(gt_pch_restore): Likewise.
	* doc/invoke.texi (Precompiled Headers): Document memory address
	restriction.


Index: c-common.h
===================================================================
RCS file: /cvs/gcc/gcc/gcc/c-common.h,v
retrieving revision 1.223
diff -p -u -r1.223 c-common.h
--- c-common.h	23 Mar 2004 23:47:34 -0000	1.223
+++ c-common.h	8 Apr 2004 02:07:49 -0000
@@ -1288,9 +1288,10 @@ extern void dump_time_statistics (void);
 extern bool c_dump_tree (void *, tree);
 
 extern void pch_init (void);
-extern int c_common_valid_pch (cpp_reader *pfile, const char *name, int fd);
+extern int c_common_valid_pch (cpp_reader *pfile, const char *name, int fd,
+			       void **pdata);
 extern void c_common_read_pch (cpp_reader *pfile, const char *name, int fd,
-			       const char *orig);
+			       const char *orig, void *data);
 extern void c_common_write_pch (void);
 extern void c_common_no_more_pch (void);
 extern void builtin_define_with_value (const char *, const char *, int);
Index: c-pch.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/c-pch.c,v
retrieving revision 1.21
diff -p -u -r1.21 c-pch.c
--- c-pch.c	15 Mar 2004 18:20:39 -0000	1.21
+++ c-pch.c	8 Apr 2004 02:07:49 -0000
@@ -35,6 +35,18 @@ Boston, MA 02111-1307, USA.  */
 #include "hosthooks.h"
 #include "target.h"
 
+/* This structure holds information about the mapped PCH data, which
+   is where the garbage collected objects are stored.  This holds the
+   offset in the file, the size of the data, and the address at which
+   we want to load it in memory.  */
+
+struct c_pch_mmap_info
+{
+  size_t offset;
+  size_t size;
+  void *preferred_base;
+};
+
 /* This structure is read very early when validating the PCH, and
    might be read for a PCH which is for a completely different compiler
    for a different operating system.  Thus, it should really only contain
@@ -48,12 +60,14 @@ Boston, MA 02111-1307, USA.  */
 
 struct c_pch_validity
 {
+  unsigned char c_pch_validity_size;
   unsigned char host_machine_length;
   unsigned char target_machine_length;
   unsigned char version_length;
   unsigned char debug_info_type;
   void (*pch_init) (void);
   size_t target_data_length;
+  struct c_pch_mmap_info mmi;
 };
 
 struct c_pch_header 
@@ -84,7 +98,7 @@ static const char *
 get_ident(void)
 {
   static char result[IDENT_LENGTH];
-  static const char template[IDENT_LENGTH] = "gpch.012";
+  static const char template[IDENT_LENGTH] = "gpch.013";
   static const char c_language_chars[] = "Co+O";
   
   memcpy (result, template, IDENT_LENGTH);
@@ -115,14 +129,19 @@ pch_init (void)
   if (strlen (host_machine) > 255 || strlen (target_machine) > 255
       || strlen (version_string) > 255)
     abort ();
-  
+
+  v.c_pch_validity_size = sizeof (v);
   v.host_machine_length = strlen (host_machine);
   v.target_machine_length = strlen (target_machine);
   v.version_length = strlen (version_string);
   v.debug_info_type = write_symbols;
   v.pch_init = &pch_init;
   target_validity = targetm.get_pch_validity (&v.target_data_length);
-  
+  /* These fields are filled in at the end.  */
+  v.mmi.offset = (size_t) -1;
+  v.mmi.size = (size_t) -1;
+  v.mmi.preferred_base = (void *) -1;
+
   if (fwrite (partial_pch, IDENT_LENGTH, 1, f) != 1
       || fwrite (&v, sizeof (v), 1, f) != 1
       || fwrite (host_machine, v.host_machine_length, 1, f) != 1
@@ -155,6 +174,7 @@ c_common_write_pch (void)
   long asm_file_end;
   long written;
   struct c_pch_header h;
+  struct c_pch_mmap_info mmi;
 
   (*debug_hooks->handle_pch) (1);
 
@@ -187,10 +207,14 @@ c_common_write_pch (void)
   /* asm_out_file can be written afterwards, so must be flushed first.  */
   fflush (asm_out_file);
 
-  gt_pch_save (pch_outfile);
+  gt_pch_save (pch_outfile, &mmi.offset, &mmi.size, &mmi.preferred_base);
   cpp_write_pch_state (parse_in, pch_outfile);
 
-  if (fseek (pch_outfile, 0, SEEK_SET) != 0
+  if (fseek (pch_outfile,
+	     IDENT_LENGTH + offsetof (struct c_pch_validity, mmi),
+	     SEEK_SET) != 0
+      || fwrite (&mmi, sizeof (mmi), 1, pch_outfile) != 1
+      || fseek (pch_outfile, 0, SEEK_SET) != 0
       || fwrite (get_ident (), IDENT_LENGTH, 1, pch_outfile) != 1)
     fatal_error ("can't write %s: %m", pch_file);
 
@@ -200,10 +224,11 @@ c_common_write_pch (void)
 /* Check the PCH file called NAME, open on FD, to see if it can be
    used in this compilation.  Return 1 if valid, 0 if the file can't
    be used now but might be if it's seen later in the compilation, and
-   2 if this file could never be used in the compilation.  */
+   2 if this file could never be used in the compilation.  When
+   returning 1, set *PDATA to the file data we read in.  */
 
 int
-c_common_valid_pch (cpp_reader *pfile, const char *name, int fd)
+c_common_valid_pch (cpp_reader *pfile, const char *name, int fd, void **pdata)
 {
   int sizeread;
   int result;
@@ -212,6 +237,7 @@ c_common_valid_pch (cpp_reader *pfile, c
   int strings_length;
   const char *pch_ident;
   struct c_pch_validity v;
+  struct c_pch_mmap_info *data;
 
   /* Perform a quick test of whether this is a valid
      precompiled header for the current language.  */
@@ -248,6 +274,14 @@ c_common_valid_pch (cpp_reader *pfile, c
   if (read (fd, &v, sizeof (v)) != sizeof (v))
     fatal_error ("can't read %s: %m", name);
 
+  if (v.c_pch_validity_size != sizeof (v))
+    {
+      if (cpp_get_options (pfile)->warn_invalid_pch)
+	cpp_error (pfile, CPP_DL_WARNING,
+		   "%s: created on host with different type sizes", name);
+      return 2;
+    }
+
   strings_length = (v.host_machine_length + v.target_machine_length 
 		    + v.version_length);
   if (read (fd, short_strings, strings_length) != strings_length)
@@ -338,8 +372,36 @@ c_common_valid_pch (cpp_reader *pfile, c
   result = cpp_valid_state (pfile, name, fd);
   if (result == -1)
     return 2;
-  else
-    return result == 0;
+  else if (result != 0)
+    return 0;
+
+  /* Read in the mapped file data.  */
+  result = host_hooks.gt_pch_use_address (v.mmi.preferred_base, v.mmi.size,
+					  fd, v.mmi.offset);
+  if (result < 0)
+    {
+      if (cpp_get_options (pfile)->warn_invalid_pch)
+	cpp_error (pfile, CPP_DL_WARNING,
+		   "%s: could not load at required memory address", name);
+      return 0;
+    }
+
+  if (result == 0)
+    {
+      off_t cur = lseek (fd, 0, SEEK_CUR);
+
+      if (cur == (off_t) -1
+	  || lseek (fd, v.mmi.offset, SEEK_SET) != v.mmi.offset
+	  || (size_t) read (fd, v.mmi.preferred_base, v.mmi.size) != v.mmi.size
+	  || lseek (fd, cur, SEEK_SET) != cur)
+	fatal_error ("can't read PCH file: %m");
+    }
+
+  data = xmalloc (sizeof (*data));
+  *data = v.mmi;
+  *pdata = data;
+
+  return 1;
 }
 
 /* Load in the PCH file NAME, open on FD.  It was originally searched for
@@ -347,13 +409,15 @@ c_common_valid_pch (cpp_reader *pfile, c
 
 void
 c_common_read_pch (cpp_reader *pfile, const char *name,
-		   int fd, const char *orig_name ATTRIBUTE_UNUSED)
+		   int fd, const char *orig_name ATTRIBUTE_UNUSED,
+		   void *data)
 {
   FILE *f;
   struct c_pch_header h;
   char *buf;
   unsigned long written;
   struct save_macro_data *smd;
+  struct c_pch_mmap_info *mmi = (struct c_pch_mmap_info *) data;
   
   f = fdopen (fd, "rb");
   if (f == NULL)
@@ -385,12 +449,13 @@ c_common_read_pch (cpp_reader *pfile, co
 
   cpp_prepare_state (pfile, &smd);
 
-  gt_pch_restore (f);
+  gt_pch_restore (f, mmi->offset, mmi->size, mmi->preferred_base);
 
   if (cpp_read_state (pfile, name, f, smd) != 0)
     return;
 
   fclose (f);
+  free (mmi);
 }
 
 /* Indicate that no more PCH files should be read.  */
Index: cppfiles.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/cppfiles.c,v
retrieving revision 1.205
diff -p -u -r1.205 cppfiles.c
--- cppfiles.c	4 Mar 2004 00:18:45 -0000	1.205
+++ cppfiles.c	8 Apr 2004 02:07:50 -0000
@@ -66,6 +66,9 @@ struct _cpp_file
   /* The full path of the pch file.  */
   const char *pchname;
 
+  /* Data returned by valid_pch if this is a PCH.  */
+  void *pch_data;
+
   /* The file's path with the basename stripped.  NULL if it hasn't
      been calculated yet.  */
   const char *dir_name;
@@ -613,7 +616,8 @@ should_stack_file (cpp_reader *pfile, _c
   /* Handle PCH files immediately; don't stack them.  */
   if (file->pch)
     {
-      pfile->cb.read_pch (pfile, file->path, file->fd, file->pchname);
+      pfile->cb.read_pch (pfile, file->path, file->fd, file->pchname,
+			  file->pch_data);
       close (file->fd);
       file->fd = -1;
       return false;
@@ -1283,7 +1287,8 @@ validate_pch (cpp_reader *pfile, _cpp_fi
   file->path = pchname;
   if (open_file (file))
     {
-      valid = 1 & pfile->cb.valid_pch (pfile, pchname, file->fd);
+      valid = 1 & pfile->cb.valid_pch (pfile, pchname, file->fd,
+				       &file->pch_data);
 
       if (!valid)
 	{
Index: cpplib.h
===================================================================
RCS file: /cvs/gcc/gcc/gcc/cpplib.h,v
retrieving revision 1.280
diff -p -u -r1.280 cpplib.h
--- cpplib.h	30 Mar 2004 19:18:49 -0000	1.280
+++ cpplib.h	8 Apr 2004 02:07:52 -0000
@@ -406,8 +406,8 @@ struct cpp_callbacks
   void (*undef) (cpp_reader *, unsigned int, cpp_hashnode *);
   void (*ident) (cpp_reader *, unsigned int, const cpp_string *);
   void (*def_pragma) (cpp_reader *, unsigned int);
-  int (*valid_pch) (cpp_reader *, const char *, int);
-  void (*read_pch) (cpp_reader *, const char *, int, const char *);
+  int (*valid_pch) (cpp_reader *, const char *, int, void **);
+  void (*read_pch) (cpp_reader *, const char *, int, const char *, void *);
   missing_header_cb missing_header;
 };
 
Index: ggc-common.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/ggc-common.c,v
retrieving revision 1.86
diff -p -u -r1.86 ggc-common.c
--- ggc-common.c	20 Mar 2004 05:21:41 -0000	1.86
+++ ggc-common.c	8 Apr 2004 02:07:52 -0000
@@ -410,19 +410,12 @@ write_pch_globals (const struct ggc_root
 	}
 }
 
-/* Hold the information we need to mmap the file back in.  */
-
-struct mmap_info
-{
-  size_t offset;
-  size_t size;
-  void *preferred_base;
-};
-
-/* Write out the state of the compiler to F.  */
+/* Write out the state of the compiler to F.  Return information about
+   mapped GC memory.  */
 
 void
-gt_pch_save (FILE *f)
+gt_pch_save (FILE *f, size_t *p_offset, size_t *p_size,
+	     void **p_preferred_base)
 {
   const struct ggc_root_tab *const *rt;
   const struct ggc_root_tab *rti;
@@ -430,8 +423,10 @@ gt_pch_save (FILE *f)
   struct traversal_state state;
   char *this_object = NULL;
   size_t this_object_size = 0;
-  struct mmap_info mmi;
   size_t page_size = getpagesize();
+  size_t offset;
+  size_t size;
+  void *preferred_base;
 
   gt_pch_save_stringpool ();
 
@@ -453,16 +448,16 @@ gt_pch_save (FILE *f)
   state.count = 0;
   htab_traverse (saving_htab, call_count, &state);
 
-  mmi.size = ggc_pch_total_size (state.d);
+  size = ggc_pch_total_size (state.d);
 
   /* Try to arrange things so that no relocation is necessary, but
      don't try very hard.  On most platforms, this will always work,
      and on the rest it's a lot of work to do better.  
      (The extra work goes in HOST_HOOKS_GT_PCH_GET_ADDRESS and
      HOST_HOOKS_GT_PCH_USE_ADDRESS.)  */
-  mmi.preferred_base = host_hooks.gt_pch_get_address (mmi.size, fileno (f));
-      
-  ggc_pch_this_base (state.d, mmi.preferred_base);
+  preferred_base = host_hooks.gt_pch_get_address (size, fileno (f));
+
+  ggc_pch_this_base (state.d, preferred_base);
 
   state.ptrs = xmalloc (state.count * sizeof (*state.ptrs));
   state.ptrs_i = 0;
@@ -484,20 +479,21 @@ gt_pch_save (FILE *f)
   /* Pad the PCH file so that the mmapped area starts on a page boundary.  */
   {
     long o;
-    o = ftell (state.f) + sizeof (mmi);
+    o = ftell (state.f);
     if (o == -1)
       fatal_error ("can't get position in PCH file: %m");
-    mmi.offset = page_size - o % page_size;
-    if (mmi.offset == page_size)
-      mmi.offset = 0;
-    mmi.offset += o;
+    offset = page_size - o % page_size;
+    if (offset == page_size)
+      offset = 0;
+    offset += o;
   }
-  if (fwrite (&mmi, sizeof (mmi), 1, state.f) != 1)
-    fatal_error ("can't write PCH file: %m");
-  if (mmi.offset != 0
-      && fseek (state.f, mmi.offset, SEEK_SET) != 0)
+  if (fseek (state.f, offset, SEEK_SET) != 0)
     fatal_error ("can't write padding to PCH file: %m");
 
+  *p_offset = offset;
+  *p_size = size;
+  *p_preferred_base = preferred_base;
+
   /* Actually write out the objects.  */
   for (i = 0; i < state.count; i++)
     {
@@ -527,16 +523,16 @@ gt_pch_save (FILE *f)
   htab_delete (saving_htab);
 }
 
-/* Read the state of the compiler back in from F.  */
+/* Read the state of the compiler back in from F.  SIZE, OFFSET and
+   PREFERRED_BASE were returned from gt_pch_save.  The mapped data has
+   already been read in at PREFERRED_BASE.  */
 
 void
-gt_pch_restore (FILE *f)
+gt_pch_restore (FILE *f, size_t offset, size_t size, void *preferred_base)
 {
   const struct ggc_root_tab *const *rt;
   const struct ggc_root_tab *rti;
   size_t i;
-  struct mmap_info mmi;
-  int result;
 
   /* Delete any deletable objects.  This makes ggc_pch_read much
      faster, as it can be sure that no GCable objects remain other
@@ -566,23 +562,10 @@ gt_pch_restore (FILE *f)
 		   sizeof (void *), 1, f) != 1)
 	  fatal_error ("can't read PCH file: %m");
 
-  if (fread (&mmi, sizeof (mmi), 1, f) != 1)
-    fatal_error ("can't read PCH file: %m");
-
-  result = host_hooks.gt_pch_use_address (mmi.preferred_base, mmi.size,
-					  fileno (f), mmi.offset);
-  if (result < 0)
-    fatal_error ("had to relocate PCH");
-  if (result == 0)
-    {
-      if (fseek (f, mmi.offset, SEEK_SET) != 0
-	  || fread (mmi.preferred_base, mmi.size, 1, f) != 1)
-	fatal_error ("can't read PCH file: %m");
-    }
-  else if (fseek (f, mmi.offset + mmi.size, SEEK_SET) != 0)
+  if (fseek (f, offset + size, SEEK_SET) != 0)
     fatal_error ("can't read PCH file: %m");
 
-  ggc_pch_read (f, mmi.preferred_base);
+  ggc_pch_read (f, preferred_base);
 
   gt_pch_restore_stringpool ();
 }
Index: ggc.h
===================================================================
RCS file: /cvs/gcc/gcc/gcc/ggc.h,v
retrieving revision 1.66
diff -p -u -r1.66 ggc.h
--- ggc.h	3 Mar 2004 11:25:48 -0000	1.66
+++ ggc.h	8 Apr 2004 02:07:52 -0000
@@ -272,10 +272,12 @@ extern void ggc_collect	(void);
 extern size_t ggc_get_size (const void *);
 
 /* Write out all GCed objects to F.  */
-extern void gt_pch_save (FILE *f);
+extern void gt_pch_save (FILE *f, size_t *offset, size_t *size,
+			 void **preferred_base);
 
 /* Read objects previously saved with gt_pch_save from F.  */
-extern void gt_pch_restore (FILE *f);
+extern void gt_pch_restore (FILE *f, size_t offset, size_t size,
+			    void *preferred_base);
 
 /* Statistics.  */
 
Index: doc/invoke.texi
===================================================================
RCS file: /cvs/gcc/gcc/gcc/doc/invoke.texi,v
retrieving revision 1.440
diff -p -u -r1.440 invoke.texi
--- doc/invoke.texi	5 Apr 2004 19:23:05 -0000	1.440
+++ doc/invoke.texi	8 Apr 2004 02:08:00 -0000
@@ -11408,6 +11408,12 @@ precompiled header was generated, or mus
 header, which usually means that the they don't appear in the
 precompiled header at all.
 @item
+The precompiled header must be loaded at a particular memory location
+selected when the precompiled header is created.  GCC will do this in
+most cases on most operating systems, but doing it reliably in all
+cases requires operating system support, which is currently not
+available for all operating systems.
+@item
 Certain command-line options must be defined in the same way as when the
 precompiled header was generated.  At present, it's not clear which
 options are safe to change and which are not; the safest choice is to