This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Give a better error for PCH with exec-shield-randomize


> From: Ian Lance Taylor <ian@wasabisystems.com>
> Date: 04 Mar 2004 11:55:55 -0500

> Geoff Keating <geoffk@geoffk.org> writes:
> 
> > It seems like this is a fairly complex and fragile workaround for an
> > OS bug.  Is there some reason you can't just fix your kernel?  If not,
> > the autoconf test for HAVE_MINCORE should be extended to test for the
> > bug and ignore mincore() if it doesn't work.  Perhaps you could just
> > test for linux in general, since the whole mincore() stuff itself
> > was written to work around a Solaris "feature".
> 
> It might be slightly complex, but I don't think it's all that fragile.
> The garbage collector has to keep track of pages which it has mapped.
> If the garbage collector has mapped pages which overlap with the
> memory region required by PCH, then PCH isn't going to work on any OS.
> I expect that it's faster to test the garbage collector pages than it
> is to make a system call for each page.
> 
> I cleaned up the patch to always check the garbage collector, and to
> only check mincore if it is supported.  Remember that this code is
> only executed if we failed to get the expected address when calling
> mmap().
> 
> > This code:
> > 
> > +      /* A Linux kernel with exec-shield-randomize set to a non-zero
> > +	 value won't work.  Give a nice error message for this common
> > +	 case.  */
> > +      {
> > +	FILE *pf;
> > +
> > +	pf = fopen ("/proc/sys/kernel/exec-shield-randomize", "r");
> > +	if (pf != NULL)
> > 
> > is Linux-specific code in the generic part of the compiler, and should
> > not be there.
> 
> OK, I added another host hook.
> 
> What do you think of this patch?

I really don't like the ggc_pch_overlap part.  I don't like it for
three reasons:

1. If it turns out that the pages do overlap, the correct response
   would be to have the garbage collector free the pages.  No GC pages
   are live while a PCH is being loaded.

2. If mincore() doesn't work, it shouldn't be used.  GGC is not the
   only source of anonymous mapped pages, libc can create them too
   (via malloc, for instance).

3. This code is untestable; it should never be used in a working
   installation, and in a non-working installation, it's probably
   better to simply refuse to load a PCH file (test for
   exec-shield-randomize before loading).  There's not much point
   going to great lengths to make a crippled installation slightly
   less crippled.  You could make this test in a gt_pch_use_address
   hook, which would simplify both parts of this patch.

   In fact, the ideal thing to do is to change c_common_valid_pch
   so that it can call a host hook, and have that hook check whether
   exec-shield-randomise is in use and refuse to load any PCH files
   in that case.

The host hook part looks reasonable, if that's what you really wanted
to do, except that I think the message about exec-shield-randomise
should be fatal_error() not just inform(), since it completely
explains what the problem is and the extra message will most likely
just confuse users.

> Ian
> 
> 
> 2004-03-04  Ian Lance Taylor  <ian@wasabisystems.com>
> 
> 	* ggc-common.c (gt_pch_restore): If we didn't get the address we
> 	wanted, call ggc_pch_overlap to check whether the garbage
> 	collector is using the pages we need.  Don't munmap the address
> 	unless we are going to try to mmap again.  When reading from the
> 	file, read into the memory we allocated.  When handling a
> 	relocation failure, call gt_pch_relocation_failure host hook.
> 	* ggc.h (ggc_pch_overlap): Declare.
> 	* ggc-page.c (ggc_pch_overlap): New function.
> 	* ggc-zone.c (ggc_pch_overlap): New function.
> 	* hosthooks.h (struct host_hooks): Add comments.  Add
> 	gt_pch_relocation_failure.
> 	* hosthooks-def.h (HOST_HOOKS_INITIALIZER): Add
> 	HOST_HOOKS_GT_PCH_RELOCATION_FAILURE.
> 	* config.host (*-*-linux*): New case.
> 	* config/host-linux.c: New file.
> 	* config/x-linux: New file.
> 	* doc/hostconfig.texi (Host Common): Document
> 	HOST_HOOKS_GT_PCH_RELOCATION_FAILURE.
> 
> 
> Index: config.host
> ===================================================================
> RCS file: /cvs/gcc/gcc/gcc/config.host,v
> retrieving revision 2.7
> diff -p -u -r2.7 config.host
> --- config.host	14 Oct 2003 03:41:41 -0000	2.7
> +++ config.host	4 Mar 2004 16:50:13 -0000
> @@ -152,4 +152,8 @@ case ${host} in
>      out_host_hook_obj=host-darwin.o
>      host_xmake_file=rs6000/x-darwin
>      ;;
> +  *-*-linux*)
> +    out_host_hook_obj=host-linux.o
> +    host_xmake_file=x-linux
> +    ;;
>  esac
> Index: ggc-common.c
> ===================================================================
> RCS file: /cvs/gcc/gcc/gcc/ggc-common.c,v
> retrieving revision 1.83
> diff -p -u -r1.83 ggc-common.c
> --- ggc-common.c	3 Mar 2004 11:25:47 -0000	1.83
> +++ ggc-common.c	4 Mar 2004 16:50:19 -0000
> @@ -608,37 +608,53 @@ gt_pch_restore (FILE *f)
>  		   PROT_READ | PROT_WRITE, MAP_PRIVATE,
>  		   fileno (f), mmi.offset);
>        
> -#if HAVE_MINCORE
>        if (addr != mmi.preferred_base)
>  	{
> -	  size_t page_size = getpagesize();
> -	  char one_byte;
> -	  
> -	  if (addr != (void *) MAP_FAILED)
> -	    munmap (addr, mmi.size);
> -	  
> -	  /* We really want to be mapped at mmi.preferred_base
> -	     so we're going to resort to MAP_FIXED.  But before,
> -	     make sure that we can do so without destroying a
> -	     previously mapped area, by looping over all pages
> -	     that would be affected by the fixed mapping.  */
> -	  errno = 0;
> -	  
> -	  for (i = 0; i < mmi.size; i+= page_size)
> -	    if (mincore ((char *)mmi.preferred_base + i, page_size, 
> -			 (void *)&one_byte) == -1
> -		&& errno == ENOMEM)
> -	      continue; /* The page is not mapped.  */
> -	    else
> -	      break;
> -	  
> -	  if (i >= mmi.size)
> -	    addr = mmap (mmi.preferred_base, mmi.size, 
> -			 PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_FIXED,
> -			 fileno (f), mmi.offset);
> -	}
> +	  bool overlap = false;
> +
> +	  /* We didn't get the address we wanted.  See if it might be
> +	     possible to force it by using MAP_FIXED.  Check whether
> +	     the garbage collector has allocated any pages in the area
> +	     we need.  If not, use mincore to check whether any of the
> +	     pages have been allocated for other purposes.  If not,
> +	     try using MAP_FIXED.  (We have to check the garbage
> +	     collector pages separately because the Linux kernel, at
> +	     least around 2.4.20, returns ENOMEM for an anonymous
> +	     mmap, such as the ones used by the garbage
> +	     collector.)  */
> +	  if (ggc_pch_overlap (mmi.preferred_base, mmi.size))
> +	    overlap = true;
> +
> +#if HAVE_MINCORE
> +	  if (! overlap)
> +	    {
> +	      size_t page_size = getpagesize ();
> +	      char one_byte;
> +
> +	      errno = 0;
> +	      for (i = 0; i < mmi.size; i += page_size)
> +		{
> +		  if (mincore ((char *) mmi.preferred_base + i, page_size,
> +			       (void *) &one_byte) == 0
> +		      || errno != ENOMEM)
> +		    {
> +		      overlap = true;
> +		      break;
> +		    }
> +		}
> +	    }
>  #endif /* HAVE_MINCORE */
> -      
> +
> +	  if (! overlap)
> +	    {
> +	      if (addr != (void *) MAP_FAILED)
> +		munmap (addr, mmi.size);
> +	      addr = mmap (mmi.preferred_base, mmi.size, 
> +			   PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_FIXED,
> +			   fileno (f), mmi.offset);
> +	    }
> +	}
> +
>        needs_read = addr == (void *) MAP_FAILED;
>  
>  #else /* HAVE_MMAP_FILE */
> @@ -651,7 +667,7 @@ gt_pch_restore (FILE *f)
>    if (needs_read)
>      {
>        if (fseek (f, mmi.offset, SEEK_SET) != 0
> -	  || fread (&mmi, mmi.size, 1, f) != 1)
> +	  || fread (addr, mmi.size, 1, f) != 1)
>  	fatal_error ("can't read PCH file: %m");
>      }
>    else if (fseek (f, mmi.offset + mmi.size, SEEK_SET) != 0)
> @@ -679,7 +695,8 @@ gt_pch_restore (FILE *f)
>  		*ptr += (size_t)addr - (size_t)mmi.preferred_base;
>  	    }
>  
> -      sorry ("had to relocate PCH");
> +      (*host_hooks.gt_pch_relocation_failure) ();
> +      fatal_error ("had to relocate PCH");
>      }
>  
>    gt_pch_restore_stringpool ();
> Index: ggc-page.c
> ===================================================================
> RCS file: /cvs/gcc/gcc/gcc/ggc-page.c,v
> retrieving revision 1.90
> diff -p -u -r1.90 ggc-page.c
> --- ggc-page.c	3 Mar 2004 11:25:47 -0000	1.90
> +++ ggc-page.c	4 Mar 2004 16:50:20 -0000
> @@ -2404,3 +2404,27 @@ ggc_pch_read (FILE *f, void *addr)
>    /* Update the statistics.  */
>    G.allocated = G.allocated_last_gc = offs - (char *)addr;
>  }
> +
> +/* Return whether any of the pages we have allocated overlap with the
> +   memory range BASE to SIZE.  */
> +
> +int
> +ggc_pch_overlap (char *base, size_t size)
> +{
> +#ifdef USING_MMAP
> +  unsigned int order;
> +  page_entry *p;
> +
> +  for (order = 2; order < NUM_ORDERS; order++)
> +    {
> +      for (p = G.pages[order]; p != NULL; p = p->next)
> +	if (p->page + p->bytes > base && p->page < base + size)
> +	  return 1;
> +    }
> +  for (p = G.free_pages; p != NULL; p = p->next)
> +    if (p->page + p->bytes > base && p->page < base + size)
> +      return 1;
> +#endif
> +
> +  return 0;
> +}
> Index: ggc-zone.c
> ===================================================================
> RCS file: /cvs/gcc/gcc/gcc/ggc-zone.c,v
> retrieving revision 2.14
> diff -p -u -r2.14 ggc-zone.c
> --- ggc-zone.c	4 Mar 2004 04:25:12 -0000	2.14
> +++ ggc-zone.c	4 Mar 2004 16:50:20 -0000
> @@ -1412,3 +1412,27 @@ ggc_pch_read (FILE *f, void *addr)
>    entry->next = entry->zone->pages;
>    entry->zone->pages = entry;
>  }
> +
> +int
> +ggc_pch_overlap (char *base, size_t size)
> +{
> +  struct alloc_zone *zone;
> +
> +  for (zone = G.zones; zone; zone = zone->next_zone)
> +    {
> +      page_entry *p;
> +
> +      for (p = zone->pages; p; p = p->next)
> +	{
> +	  if (p->page + G.pagesize > base && p->page < base + size)
> +	    return 1;
> +	}
> +      for (p = zone->free_pages; p; p = p->next)
> +	{
> +	  if (p->page + G.pagesize > base && p->page < base + size)
> +	    return 1;
> +	}
> +    }
> +
> +  return 0;
> +}
> 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	4 Mar 2004 16:50:20 -0000
> @@ -200,6 +200,10 @@ extern void ggc_pch_finish (struct ggc_p
>     parameter.  Set up the GC implementation for the new objects.  */
>  extern void ggc_pch_read (FILE *, void *);
>  
> +/* Return whether any of the pages we have allocated overlap with the
> +   memory range BASE to SIZE.  */
> +extern int ggc_pch_overlap (char *, size_t);
> +
>  
>  /* Allocation.  */
>  
> Index: hosthooks-def.h
> ===================================================================
> RCS file: /cvs/gcc/gcc/gcc/hosthooks-def.h,v
> retrieving revision 1.3
> diff -p -u -r1.3 hosthooks-def.h
> --- hosthooks-def.h	29 Jul 2003 23:36:46 -0000	1.3
> +++ hosthooks-def.h	4 Mar 2004 16:50:20 -0000
> @@ -1,5 +1,5 @@
>  /* Default macros to initialize the lang_hooks data structure.
> -   Copyright 2003 Free Software Foundation, Inc.
> +   Copyright 2003, 2004 Free Software Foundation, Inc.
>  
>  This file is part of GCC.
>  
> @@ -26,12 +26,14 @@ Boston, MA 02111-1307, USA.  */
>  #define HOST_HOOKS_EXTRA_SIGNALS hook_void_void
>  #define HOST_HOOKS_GT_PCH_GET_ADDRESS hook_voidp_size_t_null
>  #define HOST_HOOKS_GT_PCH_USE_ADDRESS hook_bool_voidp_size_t_false
> +#define HOST_HOOKS_GT_PCH_RELOCATION_FAILURE hook_void_void
>  
>  /* The structure is defined in hosthooks.h.  */
>  #define HOST_HOOKS_INITIALIZER {		\
>    HOST_HOOKS_EXTRA_SIGNALS,			\
>    HOST_HOOKS_GT_PCH_GET_ADDRESS,		\
> -  HOST_HOOKS_GT_PCH_USE_ADDRESS			\
> +  HOST_HOOKS_GT_PCH_USE_ADDRESS,		\
> +  HOST_HOOKS_GT_PCH_RELOCATION_FAILURE		\
>  }
>  
>  #endif /* GCC_HOST_HOOKS_DEF_H */
> Index: hosthooks.h
> ===================================================================
> RCS file: /cvs/gcc/gcc/gcc/hosthooks.h,v
> retrieving revision 1.4
> diff -p -u -r1.4 hosthooks.h
> --- hosthooks.h	29 Jul 2003 23:36:46 -0000	1.4
> +++ hosthooks.h	4 Mar 2004 16:50:20 -0000
> @@ -1,5 +1,5 @@
>  /* The host_hooks data structure.
> -   Copyright 2003 Free Software Foundation, Inc.
> +   Copyright 2003, 2004 Free Software Foundation, Inc.
>  
>  This file is part of GCC.
>  
> @@ -23,10 +23,19 @@ Boston, MA 02111-1307, USA.  */
>  
>  struct host_hooks
>  {
> +  /* Set up handling for host-specific signals.  */
>    void (*extra_signals) (void);
>  
> +  /* When creating a PCH, return the address of the space into which
> +     the PCH will be loaded, or NULL to use the default algorithm.  */
>    void * (*gt_pch_get_address) (size_t);
> +  /* When loading a PCH, if the address is the one which would have
> +     been returned by GT_PCH_GET_ADDRESS, and there is enough room for
> +     the size, return true.  Otherwise, return false.  */
>    bool (*gt_pch_use_address) (void *, size_t);
> +  /* Report host-specific information when failing to relocate a
> +     PCH.  */
> +  void (*gt_pch_relocation_failure) (void);
>  
>    /* Whenever you add entries here, make sure you adjust hosthooks-def.h.  */
>  };
> Index: config/host-linux.c
> ===================================================================
> RCS file: config/host-linux.c
> diff -N config/host-linux.c
> --- /dev/null	1 Jan 1970 00:00:00 -0000
> +++ config/host-linux.c	4 Mar 2004 16:50:21 -0000
> @@ -0,0 +1,60 @@
> +/* GNU/Linux host-specific hook definitions.
> +   Copyright (C) 2004 Free Software Foundation, Inc.
> +   Contributed by Ian Lance Taylor <ian@wasabisystems.com>.
> +
> +   This file is part of GCC.
> +
> +   GCC is free software; you can redistribute it and/or modify it
> +   under the terms of the GNU General Public License as published
> +   by the Free Software Foundation; either version 2, or (at your
> +   option) any later version.
> +
> +   GCC is distributed in the hope that it will be useful, but WITHOUT
> +   ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
> +   or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public
> +   License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with GCC; see the file COPYING.  If not, write to the
> +   Free Software Foundation, 59 Temple Place - Suite 330, Boston,
> +   MA 02111-1307, USA.  */
> +
> +#include "config.h"
> +#include "system.h"
> +#include "coretypes.h"
> +#include "hosthooks.h"
> +#include "hosthooks-def.h"
> +#include "toplev.h"
> +
> +static void linux_gt_pch_relocation_failure (void);
> +
> +#undef HOST_HOOKS_GT_PCH_RELOCATION_FAILURE
> +#define HOST_HOOKS_GT_PCH_RELOCATION_FAILURE linux_gt_pch_relocation_failure
> +
> +/* A Linux kernel with exec-shield-randomize set to a non-zero value
> +   won't work.  Give a more informative error message for this common
> +   case.  */
> +
> +static void
> +linux_gt_pch_relocation_failure (void)
> +{
> +  FILE *pf;
> +
> +  pf = fopen ("/proc/sys/kernel/exec-shield-randomize", "r");
> +  if (pf != NULL)
> +    {
> +      char buf[100];
> +      size_t c;
> +
> +      c = fread (buf, 1, sizeof buf - 1, pf);
> +      if (c > 0)
> +	{
> +	  buf[c] = '\0';
> +	  if (atoi (buf) > 0)
> +	    inform ("PCH is not compatible with exec-shield-randomize");
> +	}
> +      fclose (pf);
> +    }
> +}
> +
> +const struct host_hooks host_hooks = HOST_HOOKS_INITIALIZER;
> Index: config/x-linux
> ===================================================================
> RCS file: config/x-linux
> diff -N config/x-linux
> --- /dev/null	1 Jan 1970 00:00:00 -0000
> +++ config/x-linux	4 Mar 2004 16:50:21 -0000
> @@ -0,0 +1,4 @@
> +host-linux.o : $(srcdir)/config/host-linux.c $(CONFIG_H) $(SYSTEM_H) \
> +  coretypes.h hosthooks.h hosthooks-def.h toplev.h
> +	$(CC) -c $(ALL_CFLAGS) $(ALL_CPPFLAGS) $(INCLUDES) \
> +		$(srcdir)/config/host-linux.c
> Index: doc/hostconfig.texi
> ===================================================================
> RCS file: /cvs/gcc/gcc/gcc/doc/hostconfig.texi,v
> retrieving revision 1.7
> diff -p -u -r1.7 hostconfig.texi
> --- doc/hostconfig.texi	29 Jul 2003 23:36:47 -0000	1.7
> +++ doc/hostconfig.texi	4 Mar 2004 16:50:27 -0000
> @@ -75,6 +75,12 @@ indicate an out-of-date PCH file (built 
>  and such a PCH file won't work.
>  @end deftypefn
>  
> +@deftypefn {Host Hook} void HOST_HOOKS_GT_PCH_RELOCATION_FAILURE (void)
> +This host hook is called when a PCH file can not be loaded into the
> +same memory area in which it was created.  It may be used to generate
> +a host-specific error message.
> +@end deftypefn
> +
>  @node Filesystem
>  @section Host Filesystem
>  @cindex configuration file
> 


-- 
- Geoffrey Keating <geoffk@geoffk.org>


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]