From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 29508 invoked by alias); 11 Sep 2014 16:20:32 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 29497 invoked by uid 89); 11 Sep 2014 16:20:31 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.7 required=5.0 tests=AWL,BAYES_00,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=ham version=3.3.2 X-HELO: mailout3.w1.samsung.com Received: from mailout3.w1.samsung.com (HELO mailout3.w1.samsung.com) (210.118.77.13) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (DES-CBC3-SHA encrypted) ESMTPS; Thu, 11 Sep 2014 16:20:29 +0000 Received: from eucpsbgm2.samsung.com (unknown [203.254.199.245]) by mailout3.w1.samsung.com (Oracle Communications Messaging Server 7u4-24.01(7.0.4.24.0) 64bit (built Nov 17 2011)) with ESMTP id <0NBQ00C6ZW6SJ690@mailout3.w1.samsung.com> for gcc-patches@gcc.gnu.org; Thu, 11 Sep 2014 17:23:16 +0100 (BST) Received: from eusync1.samsung.com ( [203.254.199.211]) by eucpsbgm2.samsung.com (EUCPMTA) with SMTP id 1B.B7.15956.8CBC1145; Thu, 11 Sep 2014 17:20:24 +0100 (BST) Received: from [106.109.130.31] by eusync1.samsung.com (Oracle Communications Messaging Server 7u4-23.01(7.0.4.23.0) 64bit (built Aug 10 2011)) with ESMTPA id <0NBQ00H7OW1ZR600@eusync1.samsung.com>; Thu, 11 Sep 2014 17:20:24 +0100 (BST) Message-id: <5411CBC7.10706@partner.samsung.com> Date: Thu, 11 Sep 2014 16:20:00 -0000 From: Maxim Ostapenko User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.1.0 MIME-version: 1.0 To: "Joseph S. Myers" , Jakub Jelinek Cc: GCC Patches , Jeff Law , Yury Gribov , Slava Garbuzov , Maxim Ostapenko , ian@airs.com, dj@redhat.com Subject: [PATCH 2/2] Add patch for debugging compiler ICEs. References: <53F357DF.6010103@partner.samsung.com> <53FEDAC2.9050306@partner.samsung.com> <20140910045724.GB17454@tucnak.redhat.com> In-reply-to: Content-type: multipart/mixed; boundary=------------090003070903070401050904 X-IsSubscribed: yes X-SW-Source: 2014-09/txt/msg00954.txt.bz2 This is a multi-part message in MIME format. --------------090003070903070401050904 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Content-length: 2783 Hi, Joseph, Thanks for your review! I've added comments for new functions and replaced POSIX subprocess interfaces with libiberty's ones. In general, when cc1 or cc1plus ICE-es, we try to reproduce the bug by running compiler 3 times and comparing stderr and stdout on each attempt with respective ones that were gotten as the result of previous compiler run (we use temporary dump files to do this). If these files are identical, we add GCC configuration (e.g. target, configure options and version), compiler command line and preprocessed source code into last dump file, containing backtrace. Following Jakub's approach, we trigger ICE_EXIT_CODE instead of FATAL_EXIT_CODE in case of DK_FATAL error to differ ICEs from other fatal errors, so try_generate_repro routine will be able to run even if fatal_error occurred in compiler. We've noticed that on rare occasion a particularly severe segfault can cause GCC to abort without ICE-ing. These (hopefully rare) errors will be missed by our patch, because SIGSEGV handler is not able to catch the signal due to corrupted stack. It could make sense to allocate separate stack for SIGSEGV handler to resolve this situation. -Maxim On 09/10/2014 08:37 PM, Joseph S. Myers wrote: > On Wed, 10 Sep 2014, Jakub Jelinek wrote: > >> On Tue, Sep 09, 2014 at 10:51:23PM +0000, Joseph S. Myers wrote: >>> 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. >> So that the driver can understand the difference between an ICE and other >> fatal errors (e.g. sorry etc.). >> Users are typically using the driver and for them it matters what exit code >> is returned from the driver, not from cc1/cc1plus etc. > Well, I think the next revision of the patch submission needs more > explanation in this area. What exit codes do cc1 and the driver now > return for (normal error, fatal error, ICE), and what do they return after > the patch, and how does the change to the fatal_error case avoid incorrect > changes if either cc1 or the driver called fatal_error (as opposed to > either cc1 or the driver having an ICE)? Maybe that explanation should be > in the form of a comment on this exit call, explaining why the > counterintuitive use of ICE_EXIT_CODE in the DK_FATAL case is correct. > --------------090003070903070401050904 Content-Type: text/x-patch; name="ICE.diff" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="ICE.diff" Content-length: 14720 2014-09-04 Jakub Jelinek Max Ostapenko * common.opt: New option. * doc/invoke.texi: Describe new option. * 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. (do_report_bug): Likewise. (append_text): Likewise. (try_generate_repro): Likewise diff --git a/gcc/common.opt b/gcc/common.opt index 7d78803..ce71f09 100644 --- a/gcc/common.opt +++ b/gcc/common.opt @@ -1120,6 +1120,11 @@ fdump-noaddr Common Report Var(flag_dump_noaddr) Suppress output of addresses in debugging dumps +freport-bug +Common Driver Var(flag_report_bug) +Collect and dump debug information into temporary file if ICE in C/C++ +compiler occured. + fdump-passes Common Var(flag_dump_passes) Init(0) Dump optimization passes diff --git a/gcc/diagnostic.c b/gcc/diagnostic.c index 73666d6..dbc928b 100644 --- a/gcc/diagnostic.c +++ b/gcc/diagnostic.c @@ -494,7 +494,10 @@ diagnostic_action_after_output (diagnostic_context *context, real_abort (); diagnostic_finish (context); fnotice (stderr, "compilation terminated.\n"); - exit (FATAL_EXIT_CODE); + /* Exit with ICE_EXIT_CODE rather then FATAL_EXIT_CODE so the driver + understands the difference between an ICE and other fatal errors + (DK_SORRY and DK_ERROR). */ + exit (ICE_EXIT_CODE); default: gcc_unreachable (); diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi index 863b382..565421c 100644 --- a/gcc/doc/invoke.texi +++ b/gcc/doc/invoke.texi @@ -6336,6 +6336,11 @@ feasible to use diff on debugging dumps for compiler invocations with different compiler binaries and/or different text / bss / data / heap / stack / dso start locations. +@item -freport-bug +@opindex freport-bug +Collect and dump debug information into temporary file if ICE in C/C++ +compiler occured. + @item -fdump-unnumbered @opindex fdump-unnumbered When doing debugging dumps, suppress instruction numbers and address output. diff --git a/gcc/gcc.c b/gcc/gcc.c index c550d9d..e32ff47 100644 --- a/gcc/gcc.c +++ b/gcc/gcc.c @@ -253,6 +253,7 @@ static void init_gcc_specs (struct obstack *, const char *, const char *, static const char *convert_filename (const char *, int, int); #endif +static void try_generate_repro (const char *prog, const char **argv); 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 **); @@ -2856,7 +2857,7 @@ execute (void) } } - if (string != commands[i].prog) + if (i && string != commands[i].prog) free (CONST_CAST (char *, string)); } @@ -2909,6 +2910,15 @@ execute (void) else if (WIFEXITED (status) && WEXITSTATUS (status) >= MIN_FATAL_STATUS) { + /* For ICEs in cc1, cc1obj, cc1plus see if it is + reproducible or not. */ + const char *p; + if (flag_report_bug + && 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); if (WEXITSTATUS (status) > greatest_status) greatest_status = WEXITSTATUS (status); ret_code = -1; @@ -2966,6 +2976,9 @@ execute (void) } } + if (commands[0].argv[0] != commands[0].prog) + free (CONST_CAST (char *, commands[0].argv[0])); + return ret_code; } } @@ -6157,6 +6170,338 @@ give_switch (int switchnum, int omit_first_word) switches[switchnum].validated = true; } +/* Print GCC configuration (e.g. version, thread model, target, + configuration_arguments) to a given FILE. */ + +static void +print_configuration (FILE *file) +{ + int n; + const char *thrmod; + + fnotice (file, "Target: %s\n", spec_machine); + fnotice (file, "Configured with: %s\n", configuration_arguments); + +#ifdef THREAD_MODEL_SPEC + /* We could have defined THREAD_MODEL_SPEC to "%*" by default, + but there's no point in doing all this processing just to get + thread_model back. */ + obstack_init (&obstack); + do_spec_1 (THREAD_MODEL_SPEC, 0, thread_model); + obstack_1grow (&obstack, '\0'); + thrmod = XOBFINISH (&obstack, const char *); +#else + thrmod = thread_model; +#endif + + fnotice (file, "Thread model: %s\n", thrmod); + + /* compiler_version is truncated at the first space when initialized + from version string, so truncate version_string at the first space + before comparing. */ + for (n = 0; version_string[n]; n++) + if (version_string[n] == ' ') + break; + + if (! strncmp (version_string, compiler_version, n) + && compiler_version[n] == 0) + fnotice (file, "gcc version %s %s\n\n", version_string, + pkgversion_string); + else + fnotice (file, "gcc driver version %s %sexecuting gcc version %s\n\n", + version_string, pkgversion_string, compiler_version); + +} + +#define RETRY_ICE_ATTEMPTS 3 + +/* Returns true if FILE1 and FILE2 contain equivalent data, 0 otherwise. */ + +static bool +files_equal_p (char *file1, char *file2) +{ + struct stat st1, st2; + off_t n, len; + int fd1, fd2; + const int bufsize = 8192; + char *buf = XNEWVEC (char, bufsize); + + fd1 = open (file1, O_RDONLY); + fd2 = open (file2, O_RDONLY); + + if (fd1 < 0 || fd2 < 0) + goto error; + + if (fstat (fd1, &st1) < 0 || fstat (fd2, &st2) < 0) + goto error; + + if (st1.st_size != st2.st_size) + goto error; + + for (n = st1.st_size; n; n -= len) + { + len = n; + if ((int) len > bufsize / 2) + len = bufsize / 2; + + if (read (fd1, buf, len) != (int) len + || read (fd2, buf + bufsize / 2, len) != (int) len) + { + goto error; + } + + if (memcmp (buf, buf + bufsize / 2, len) != 0) + goto error; + } + + free (buf); + close (fd1); + close (fd2); + + return 1; + +error: + free (buf); + close (fd1); + close (fd2); + return 0; +} + +/* Check that compiler's output doesn't differ across runs. + TEMP_STDOUT_FILES and TEMP_STDERR_FILES are arrays of files, containing + stdout and stderr for each compiler run. Return true if all of + TEMP_STDOUT_FILES and TEMP_STDERR_FILES are equivalent. */ + +static bool +check_repro (char **temp_stdout_files, char **temp_stderr_files) +{ + int i; + for (i = 0; i < RETRY_ICE_ATTEMPTS - 2; ++i) + { + if (!files_equal_p (temp_stdout_files[i], temp_stdout_files[i + 1]) + || !files_equal_p (temp_stderr_files[i], temp_stderr_files[i + 1])) + { + fnotice (stderr, "The bug is not reproducible, so it is" + " likely a hardware or OS problem.\n"); + break; + } + } + return i == RETRY_ICE_ATTEMPTS - 2; +} + +enum attempt_status { + ATTEMPT_STATUS_FAIL_TO_RUN, + ATTEMPT_STATUS_SUCCESS, + ATTEMPT_STATUS_ICE +}; + + +/* Run compiler with arguments NEW_ARGV to reproduce the ICE, storing stdout + to OUT_TEMP and stderr to ERR_TEMP. If APPEND is TRUE, append to OUT_TEMP + and ERR_TEMP instead of truncating. If EMIT_SYSTEM_INFO is TRUE, also write + GCC configuration into to ERR_TEMP. Return ATTEMPT_STATUS_FAIL_TO_RUN if + compiler failed to run, ATTEMPT_STATUS_ICE if compiled ICE-ed and + ATTEMPT_STATUS_SUCCESS otherwise. */ + +static enum attempt_status +run_attempt (const char **new_argv, const char *out_temp, + const char *err_temp, int emit_system_info, int append) +{ + + if (emit_system_info) + { + FILE *file_out = fopen (err_temp, "a"); + print_configuration (file_out); + fclose (file_out); + } + + int exit_status; + const char *errmsg; + struct pex_obj *pex; + int err; + int pex_flags = PEX_USE_PIPES | PEX_LAST; + enum attempt_status status = ATTEMPT_STATUS_FAIL_TO_RUN; + + if (append) + pex_flags |= PEX_STDOUT_APPEND | PEX_STDERR_APPEND; + + pex = pex_init (PEX_USE_PIPES, new_argv[0], NULL); + if (!pex) + fatal_error ("pex_init failed: %m"); + + errmsg = pex_run (pex, pex_flags, new_argv[0], + CONST_CAST2 (char *const *, const char **, &new_argv[1]), out_temp, + err_temp, &err); + + if (!pex_get_status (pex, 1, &exit_status)) + goto out; + + switch (WEXITSTATUS (exit_status)) + { + case ICE_EXIT_CODE: + status = ATTEMPT_STATUS_ICE; + break; + + case SUCCESS_EXIT_CODE: + status = ATTEMPT_STATUS_SUCCESS; + break; + + default: + ; + } + +out: + pex_free (pex); + return status; +} + +/* This routine adds preprocessed source code into the given ERR_FILE. + To do this, it adds "-E" to NEW_ARGV and execute RUN_ATTEMPT routine to + add information in report file. RUN_ATTEMPT should return + ATTEMPT_STATUS_SUCCESS, in other case we cannot generate the report. */ + +static void +do_report_bug (const char **new_argv, const int nargs, + char **out_file, char **err_file) +{ + int i, status; + int fd = open (*out_file, O_RDWR | O_APPEND); + if (fd < 0) + return; + write (fd, "\n//", 3); + for (i = 0; i < nargs; i++) + { + write (fd, " ", 1); + write (fd, new_argv[i], strlen (new_argv[i])); + } + write (fd, "\n\n", 2); + close (fd); + new_argv[nargs] = "-E"; + new_argv[nargs + 1] = NULL; + + status = run_attempt (new_argv, *out_file, *err_file, 0, 1); + + if (status == ATTEMPT_STATUS_SUCCESS) + { + fnotice (stderr, "Preprocessed source stored into %s file," + " please attach this to your bugreport.\n", *out_file); + /* Make sure it is not deleted. */ + free (*out_file); + *out_file = NULL; + } +} + +/* Append string STR to file FILE. */ + +static void +append_text (char *file, const char *str) +{ + int fd = open (file, O_RDWR | O_APPEND); + if (fd < 0) + return; + + write (fd, str, strlen (str)); + close (fd); +} + +/* Try to reproduce ICE. If bug is reproducible, generate report .err file + containing GCC configuration, backtrace, compiler's command line options + and preprocessed source code. */ + +static void +try_generate_repro (const char *prog, const char **argv) +{ + int i, nargs, out_arg = -1, quiet = 0, attempt; + const char **new_argv; + char *temp_files[RETRY_ICE_ATTEMPTS * 2]; + char **temp_stdout_files = &temp_files[0]; + char **temp_stderr_files = &temp_files[RETRY_ICE_ATTEMPTS]; + + if (gcc_input_filename == NULL || ! strcmp (gcc_input_filename, "-")) + return; + + for (nargs = 0; argv[nargs] != NULL; ++nargs) + /* Only retry compiler ICEs, not preprocessor ones. */ + if (! strcmp (argv[nargs], "-E")) + return; + else if (argv[nargs][0] == '-' && argv[nargs][1] == 'o') + { + if (out_arg == -1) + out_arg = nargs; + else + return; + } + /* If the compiler is going to output any time information, + it might varry between invocations. */ + else if (! strcmp (argv[nargs], "-quiet")) + quiet = 1; + else if (! strcmp (argv[nargs], "-ftime-report")) + return; + + if (out_arg == -1 || !quiet) + return; + + memset (temp_files, '\0', sizeof (temp_files)); + new_argv = XALLOCAVEC (const char *, nargs + 4); + memcpy (new_argv, argv, (nargs + 1) * sizeof (const char *)); + new_argv[nargs++] = "-frandom-seed=0"; + new_argv[nargs++] = "-fdump-noaddr"; + new_argv[nargs] = NULL; + if (new_argv[out_arg][2] == '\0') + new_argv[out_arg + 1] = "-"; + else + new_argv[out_arg] = "-o-"; + + int status; + for (attempt = 0; attempt < RETRY_ICE_ATTEMPTS; ++attempt) + { + int emit_system_info = 0; + int append = 0; + temp_stdout_files[attempt] = make_temp_file (".out"); + temp_stderr_files[attempt] = make_temp_file (".err"); + + if (attempt == RETRY_ICE_ATTEMPTS - 1) + { + append = 1; + emit_system_info = 1; + } + + if (emit_system_info) + append_text (temp_stderr_files[attempt], "/*\n"); + + status = run_attempt (new_argv, temp_stdout_files[attempt], + temp_stderr_files[attempt], emit_system_info, + append); + + if (emit_system_info) + append_text (temp_stderr_files[attempt], "*/\n"); + + if (status != ATTEMPT_STATUS_ICE) + { + fnotice (stderr, "The bug is not reproducible, so it is" + " likely a hardware or OS problem.\n"); + goto out; + } + } + + if (!check_repro (temp_stdout_files, temp_stderr_files)) + goto out; + + /* In final attempt we append compiler options and preprocesssed code to last + generated .err file with configuration and backtrace. */ + do_report_bug (new_argv, nargs, + &temp_stderr_files[RETRY_ICE_ATTEMPTS - 1], + &temp_stdout_files[RETRY_ICE_ATTEMPTS - 1]); + +out: + for (i = 0; i < RETRY_ICE_ATTEMPTS * 2; i++) + if (temp_files[i]) + { + unlink (temp_stdout_files[i]); + free (temp_stdout_files[i]); + } +} + /* Search for a file named NAME trying various prefixes including the user's -B prefix and some standard ones. Return the absolute file name found. If nothing is found, return NAME. */ @@ -6926,41 +7271,7 @@ warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.\n\n" if (verbose_flag) { - int n; - const char *thrmod; - - fnotice (stderr, "Target: %s\n", spec_machine); - fnotice (stderr, "Configured with: %s\n", configuration_arguments); - -#ifdef THREAD_MODEL_SPEC - /* We could have defined THREAD_MODEL_SPEC to "%*" by default, - but there's no point in doing all this processing just to get - thread_model back. */ - obstack_init (&obstack); - do_spec_1 (THREAD_MODEL_SPEC, 0, thread_model); - obstack_1grow (&obstack, '\0'); - thrmod = XOBFINISH (&obstack, const char *); -#else - thrmod = thread_model; -#endif - - fnotice (stderr, "Thread model: %s\n", thrmod); - - /* compiler_version is truncated at the first space when initialized - from version string, so truncate version_string at the first space - before comparing. */ - for (n = 0; version_string[n]; n++) - if (version_string[n] == ' ') - break; - - if (! strncmp (version_string, compiler_version, n) - && compiler_version[n] == 0) - fnotice (stderr, "gcc version %s %s\n", version_string, - pkgversion_string); - else - fnotice (stderr, "gcc driver version %s %sexecuting gcc version %s\n", - version_string, pkgversion_string, compiler_version); - + print_configuration (stderr); if (n_infiles == 0) return (0); } --------------090003070903070401050904--