Re: [PATCH][Ping v5] Add patch for debugging compiler ICEs
Thanks Jeff and Jakub, I've reposted ICE debugging patch into gcc-patches mailing list (https://gcc.gnu.org/ml/gcc-patches/2014-08/msg00285.html). -Maxim On 08/01/2014 11:29 PM, Jeff Law wrote: On 08/01/14 02:00, Jakub Jelinek wrote: On Thu, Jul 24, 2014 at 04:39:28PM +0400, Maxim Ostapenko wrote: Ping. Don't want to review a patch I wrote partially, so just a few comments: 1) IMHO it should be configure time selectable (not sure about the default, but for non-release branches IMHO it should default to off, for release branches I don't know). The point is that while it is useful for people to report gcc bugs in production compilers, it is not useful for gcc developers on their boxes, having ICEs take 3 times as long is not desirable for people who deal with those all the time We definitely want the ability to select. Waiting on an ICE to trigger 3 times would be painful for the developers. So I'd think that enabled for the release branches and disabled for the trunk would be the right default. 2) recently a bug has been reported that some ICEs which print RTL to stderr are considered non-reproduceable, because the addresses e.g. in call_insn fndecl change through address space randomization. For the retries the driver should supposedly append -fdump-noaddr. Right. I think we discussed this at Cauldron. I think it's conceptually ok with those two changes, then it's just reviewing the details. If you or Maxim could make those changes and repost, I'll cover the review. jeff
Re: [PATCH][Ping v5] Add patch for debugging compiler ICEs
On 08/01/2014 07:53 PM, Jakub Jelinek wrote: I think we should use David Malcolm's approach i.e. add some --report-bug flag to driver. This could be enabled by default at configure time via --with-spec. -freport-bug or whatever we call it should not be, at least if it attempts to communicate over the network, should not be required though, lots of people do e.g. package builds in various chroots etc. and you really don't want to perform any network activity from there. Oh sure, the actual "backend" of --report-bug could vary from just generating the repro and storing it in /tmp (what your patch did in the first place) to David's full blown automatic Bugzilla submission. -Y
Re: [PATCH][Ping v5] Add patch for debugging compiler ICEs
On 08/01/14 02:00, Jakub Jelinek wrote: On Thu, Jul 24, 2014 at 04:39:28PM +0400, Maxim Ostapenko wrote: Ping. Don't want to review a patch I wrote partially, so just a few comments: 1) IMHO it should be configure time selectable (not sure about the default, but for non-release branches IMHO it should default to off, for release branches I don't know). The point is that while it is useful for people to report gcc bugs in production compilers, it is not useful for gcc developers on their boxes, having ICEs take 3 times as long is not desirable for people who deal with those all the time We definitely want the ability to select. Waiting on an ICE to trigger 3 times would be painful for the developers. So I'd think that enabled for the release branches and disabled for the trunk would be the right default. 2) recently a bug has been reported that some ICEs which print RTL to stderr are considered non-reproduceable, because the addresses e.g. in call_insn fndecl change through address space randomization. For the retries the driver should supposedly append -fdump-noaddr. Right. I think we discussed this at Cauldron. I think it's conceptually ok with those two changes, then it's just reviewing the details. If you or Maxim could make those changes and repost, I'll cover the review. jeff
Re: [PATCH][Ping v5] Add patch for debugging compiler ICEs
On Fri, Aug 01, 2014 at 12:13:18PM +0400, Yury Gribov wrote: > On 08/01/2014 12:00 PM, Jakub Jelinek wrote: > >Don't want to review a patch I wrote partially, so just a few comments: > >1) IMHO it should be configure time selectable (not sure about the default, > >but for non-release branches IMHO it should default to off, for release > >branches I don't know). The point is that while it is useful for > >people to report gcc bugs in production compilers, it is not useful for > >gcc developers on their boxes, having ICEs take 3 times as long is not > >desirable for people who deal with those all the time > > I think we should use David Malcolm's approach i.e. add some --report-bug > flag > to driver. This could be enabled by default at configure time via > --with-spec. -freport-bug or whatever we call it should not be, at least if it attempts to communicate over the network, should not be required though, lots of people do e.g. package builds in various chroots etc. and you really don't want to perform any network activity from there. Still, having info whether an ICE was reproduceable or not is useful in that case, and having preprocessed source left in /tmp with the path printed to stderr is also useful, you can e.g. grab those /tmp/cc*.out files from the chroot and allow people to report later on. The ICE message can of course suggest an option or script or command line to run to report the bug or navigate the user through bug reporting process. Jakub
Re: [PATCH][Ping v5] Add patch for debugging compiler ICEs
On Fri, Aug 01, 2014 at 08:43:27AM -0700, Andi Kleen wrote: > Jakub Jelinek writes: > > Don't want to review a patch I wrote partially, so just a few comments: > > 1) IMHO it should be configure time selectable (not sure about the default, > > but for non-release branches IMHO it should default to off, for release > > branches I don't know). The point is that while it is useful for > > people to report gcc bugs in production compilers, it is not useful for > > gcc developers on their boxes, having ICEs take 3 times as long is not > > desirable for people who deal with those all the time > > It may also not be desirable for LTO builds, which can take very long > by themselves. Indeed, and preparing preprocessed sources for all the files is hard in that case anyway. But as it is keyed on cc1 substring being present in the compiler's name and lto uses lto1, it shouldn't trigger for that. Only if you get ICEs with -flto already when compiling from C/C++ source, but then it doesn't take very long usually and there is preprocessed source available. Jakub
Re: [PATCH][Ping v5] Add patch for debugging compiler ICEs
Jakub Jelinek writes: > Don't want to review a patch I wrote partially, so just a few comments: > 1) IMHO it should be configure time selectable (not sure about the default, > but for non-release branches IMHO it should default to off, for release > branches I don't know). The point is that while it is useful for > people to report gcc bugs in production compilers, it is not useful for > gcc developers on their boxes, having ICEs take 3 times as long is not > desirable for people who deal with those all the time It may also not be desirable for LTO builds, which can take very long by themselves. -Andi -- a...@linux.intel.com -- Speaking for myself only
Re: [PATCH][Ping v5] Add patch for debugging compiler ICEs
On 08/01/2014 12:00 PM, Jakub Jelinek wrote: Don't want to review a patch I wrote partially, so just a few comments: 1) IMHO it should be configure time selectable (not sure about the default, but for non-release branches IMHO it should default to off, for release branches I don't know). The point is that while it is useful for people to report gcc bugs in production compilers, it is not useful for gcc developers on their boxes, having ICEs take 3 times as long is not desirable for people who deal with those all the time I think we should use David Malcolm's approach i.e. add some --report-bug flag to driver. This could be enabled by default at configure time via --with-spec. 2) recently a bug has been reported that some ICEs which print RTL to stderr are considered non-reproduceable, because the addresses e.g. in call_insn fndecl change through address space randomization. For the etries the driver should supposedly append -fdump-noaddr. +1 -Y
Re: [PATCH][Ping v5] Add patch for debugging compiler ICEs
On Thu, Jul 24, 2014 at 04:39:28PM +0400, Maxim Ostapenko wrote: > Ping. Don't want to review a patch I wrote partially, so just a few comments: 1) IMHO it should be configure time selectable (not sure about the default, but for non-release branches IMHO it should default to off, for release branches I don't know). The point is that while it is useful for people to report gcc bugs in production compilers, it is not useful for gcc developers on their boxes, having ICEs take 3 times as long is not desirable for people who deal with those all the time 2) recently a bug has been reported that some ICEs which print RTL to stderr are considered non-reproduceable, because the addresses e.g. in call_insn fndecl change through address space randomization. For the retries the driver should supposedly append -fdump-noaddr. Jakub
[PATCH][Ping v5] Add patch for debugging compiler ICEs
Ping.
Original Message
Subject:[PATCH][Ping v4] Add patch for debugging compiler ICEs
Date: Fri, 11 Jul 2014 17:44:28 +0400
From: Maxim Ostapenko
To: GCC Patches
CC: Yury Gribov , Slava Garbuzov
, Jakub Jelinek ,
tsaund...@mozilla.com, Maxim Ostapenko
Ping. Added small changes due to previous discussion in community.
Original Message
Subject:[PATCH][Ping v3] Add patch for debugging compiler ICEs
Date: Fri, 04 Jul 2014 18:32:44 +0400
From: Maxim Ostapenko
To: GCC Patches
CC: Yury Gribov , Slava Garbuzov
, Jakub Jelinek ,
tsaund...@mozilla.com, Maxim Ostapenko
Ping.
Original Message
Subject:[PATCH][Ping v2] Add patch for debugging compiler ICEs
Date: Thu, 26 Jun 2014 19:46:08 +0400
From: Maxim Ostapenko
To: GCC Patches
CC: Yury Gribov , Slava Garbuzov
, Jakub Jelinek ,
tsaund...@mozilla.com, Maxim Ostapenko
Ping.
Original Message
Subject:[PATCH][Ping] Add patch for debugging compiler ICEs
Date: Wed, 11 Jun 2014 18:15:27 +0400
From: Maxim Ostapenko
To: GCC Patches
CC: Yury Gribov , Slava Garbuzov
, Jakub Jelinek ,
tsaund...@mozilla.com, chefm...@gmail.com
Ping.
Original Message
Subject:[PATCH] Add patch for debugging compiler ICEs
Date: Mon, 02 Jun 2014 19:21:14 +0400
From: Maxim Ostapenko
To: GCC Patches
CC: Yury Gribov , Slava Garbuzov
, Jakub Jelinek ,
tsaund...@mozilla.com, chefm...@gmail.com
Hi,
A years ago there was a discussion
(https://gcc.gnu.org/ml/gcc-patches/2004-01/msg02437.html) about debugging
compiler ICEs that resulted in a patch from Jakub, which dumps
useful information into temporary file, but for some reasons this patch wasn't
applied to trunk.
This is the resurrected patch with added GCC version information into generated
repro file.
-Maxim
2014-06-02 Jakub Jelinek
Max Ostapenko
* diagnostic.c (diagnostic_action_after_output): Exit with
ICE_EXIT_CODE instead of FATAL_EXIT_CODE.
* gcc.c (execute): Don't free first string early, but at the end
of the function. Call retry_ice if compiler exited with
ICE_EXIT_CODE.
(main): Factor out common code.
(print_configuration): New function.
(try_fork): Likewise.
(redirect_stdout_stderr): Likewise.
(files_equal_p): Likewise.
(check_repro): Likewise.
(run_attempt): Likewise.
(generate_preprocessed_code): Likewise.
(append_text): Likewise.
(try_generate_repro): Likewise.
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);
default:
gcc_unreachable ();
diff --git a/gcc/gcc.c b/gcc/gcc.c
index 6cd08ea..045363c 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
+#endif
+
+#if !(defined (__MSDOS__) || defined (OS2) || defined (VMS))
+#define RETRY_ICE_SUPPORTED
+#endif
/* By default there is no special suffix for target executables. */
/* FIXME: when autoconf is fixed, remove the host check - dj */
@@ -253,6 +260,9 @@ static void init_gcc_specs (struct obstack *, const char *, const char *,
static const char *convert_filename (const char *, int, int);
#endif
+#ifdef RETRY_ICE_SUPPORTED
+static void try_generate_repro (const char *prog, const char **argv);
+#endif
static const char *getenv_spec_function (int, const char **);
static const char *if_exists_spec_function (int, const char **);
static const char *if_exists_else_spec_function (int, const char **);
@@ -2850,7 +2860,7 @@ execute (void)
}
}
- if (string != commands[i].prog)
+ if (i && string != commands[i].prog)
free (CONST_CAST (char *, string));
}
@@ -2903,6 +2913,16 @@ execute (void)
else if (WIFEXITED (status)
&& WEXITSTATUS (status) >= MIN_FATAL_STATUS)
{
+#ifdef RETRY_ICE_SUPPORTED
+ /* For ICEs in cc1, cc1obj, cc1plus see if it is
+ reproducible or not. */
+ const char *p;
+ if (WEXITSTATUS (status) == ICE_EXIT_CODE
+ && i == 0
+ && (p = strrchr (commands[0].argv[0], DIR_SEPARATOR))
+ && ! strncmp (p + 1, "cc1", 3))
+ try_generate_repro (commands[0].prog, commands[0].argv);
+#endif
if (WEXITSTATUS (status) > greatest_status)
greatest_status = WEXITSTATUS (status);
ret_code = -1;
@@ -2960,6 +2980,9 @@ execute (void)
}
}
+ if (commands[0].argv[0] != commands[0].prog)
+ free (CONST_CAST (char *, commands[0].argv[0]));
+
return ret_code;
}
}
@@ -6151,6 +6174,341 @@ give_switch (int switchnum, int omit_first_word)
