[Ping v2][PATCH] Add patch for debugging compiler ICEs.

Joseph S. Myers joseph@codesourcery.com
Tue Sep 9 22:51:00 GMT 2014


On Thu, 28 Aug 2014, Maxim Ostapenko wrote:

> diff --git a/gcc/diagnostic.c b/gcc/diagnostic.c
> index 0cc7593..67b8c5b 100644
> --- a/gcc/diagnostic.c
> +++ b/gcc/diagnostic.c
> @@ -492,7 +492,7 @@ diagnostic_action_after_output (diagnostic_context *context,
>  	real_abort ();
>        diagnostic_finish (context);
>        fnotice (stderr, "compilation terminated.\n");
> -      exit (FATAL_EXIT_CODE);
> +      exit (ICE_EXIT_CODE);

Why?  This is the case for fatal_error.  FATAL_EXIT_CODE seems right for 
this, and ICE_EXIT_CODE wrong.

> diff --git a/gcc/gcc.c b/gcc/gcc.c
> index 44d0416..f7a56d1 100644
> --- a/gcc/gcc.c
> +++ b/gcc/gcc.c
> @@ -43,6 +43,13 @@ compilation is specified by a string called a "spec".  */
>  #include "params.h"
>  #include "vec.h"
>  #include "filenames.h"
> +#ifdef HAVE_UNISTD_H
> +#include <unistd.h>
> +#endif

No, unistd.h is already included in system.h, so no other (host-side) 
source files need to include it directly.

> +#if !(defined (__MSDOS__) || defined (OS2) || defined (VMS))
> +#define RETRY_ICE_SUPPORTED
> +#endif

I don't think it's a good idea to have this sort of host conditional in 
random source files if it can be avoided.  And feature tests (e.g. 
HAVE_FORK) are better than tests for particular systems.

In any case, it would be better to use libiberty's pexecute interface for 
running subprocesses, as used elsewhere in GCC, rather than using POSIX 
interfaces directly, unless there's a clear reason (which needs explaining 
in the patch submission) why it's unsuitable.  Hopefully that would 
eliminate the need for this conditional.

> +static void
> +print_configuration (void)

All the new functions need comments above them giving the semantics of the 
function and any arguments and return values.

> +static int
> +files_equal_p (char *file1, char *file2, char *buf, const int bufsize)
> +{
> +  struct stat st1, st2;
> +  size_t n, len;

> +  for (n = st1.st_size; n; n -= len)

This assignment would silently truncate a large file size on 32-bit 
platforms.  A variable that can store file sizes should be off_t not 
size_t (then you need to be careful about any mixing of signed and 
unsigned types).

-- 
Joseph S. Myers
joseph@codesourcery.com



More information about the Gcc-patches mailing list