diff mbox

Add -dB option to disable backtraces

Message ID 20170517021647.8495-1-andi@firstfloor.org
State New
Headers show

Commit Message

Andi Kleen May 17, 2017, 2:16 a.m. UTC
From: Andi Kleen <ak@linux.intel.com>

When running creduce on an ICE substantial amounts of the total
CPU time go to backtrace_qsort() (sorting dwarf of the compiler) for
printing the backtrace of the ICE. When running a reduction we don't need the
backtrace. So add a -dB option to turn it off, and make reduction
a bit faster.

Passes bootstrap and testing on x86_64-linux

gcc/:

2017-05-16  Andi Kleen  <ak@linux.intel.com>

        * diagnostic.c (diagnostic_initialize): Set disable_backtrace to
	false.
        (diagnostic_action_after_output): Check disable_backtrace.
        * diagnostic.h (struct diagnostic_context): Add
	disable_backtrace.
        * doc/invoke.texi (-dB): Document.
        * opts.c (decode_d_option): Handle -dB.
---
 gcc/diagnostic.c    | 16 ++++++++++------
 gcc/diagnostic.h    |  7 +++++++
 gcc/doc/invoke.texi |  6 ++++++
 gcc/opts.c          |  3 +++
 4 files changed, 26 insertions(+), 6 deletions(-)

Comments

Andrew Pinski May 17, 2017, 3:40 a.m. UTC | #1
On Tue, May 16, 2017 at 7:16 PM, Andi Kleen <andi@firstfloor.org> wrote:
> From: Andi Kleen <ak@linux.intel.com>
>
> When running creduce on an ICE substantial amounts of the total
> CPU time go to backtrace_qsort() (sorting dwarf of the compiler) for
> printing the backtrace of the ICE. When running a reduction we don't need the
> backtrace. So add a -dB option to turn it off, and make reduction
> a bit faster.

The other thing which you could is strip the binaries.  :)
j/k.  I think this is a good patch and a good idea.

Thanks,
Andrew

>
> Passes bootstrap and testing on x86_64-linux
>
> gcc/:
>
> 2017-05-16  Andi Kleen  <ak@linux.intel.com>
>
>         * diagnostic.c (diagnostic_initialize): Set disable_backtrace to
>         false.
>         (diagnostic_action_after_output): Check disable_backtrace.
>         * diagnostic.h (struct diagnostic_context): Add
>         disable_backtrace.
>         * doc/invoke.texi (-dB): Document.
>         * opts.c (decode_d_option): Handle -dB.
> ---
>  gcc/diagnostic.c    | 16 ++++++++++------
>  gcc/diagnostic.h    |  7 +++++++
>  gcc/doc/invoke.texi |  6 ++++++
>  gcc/opts.c          |  3 +++
>  4 files changed, 26 insertions(+), 6 deletions(-)
>
> diff --git a/gcc/diagnostic.c b/gcc/diagnostic.c
> index 158519606f8..bbac3f384d4 100644
> --- a/gcc/diagnostic.c
> +++ b/gcc/diagnostic.c
> @@ -156,6 +156,7 @@ diagnostic_initialize (diagnostic_context *context, int n_opts)
>      context->caret_chars[i] = '^';
>    context->show_option_requested = false;
>    context->abort_on_error = false;
> +  context->disable_backtrace = false;
>    context->show_column = false;
>    context->pedantic_errors = false;
>    context->permissive = false;
> @@ -500,13 +501,16 @@ diagnostic_action_after_output (diagnostic_context *context,
>      case DK_ICE:
>      case DK_ICE_NOBT:
>        {
> -       struct backtrace_state *state = NULL;
> -       if (diag_kind == DK_ICE)
> -         state = backtrace_create_state (NULL, 0, bt_err_callback, NULL);
>         int count = 0;
> -       if (state != NULL)
> -         backtrace_full (state, 2, bt_callback, bt_err_callback,
> -                         (void *) &count);
> +       if (!context->disable_backtrace)
> +         {
> +           struct backtrace_state *state = NULL;
> +           if (diag_kind == DK_ICE)
> +             state = backtrace_create_state (NULL, 0, bt_err_callback, NULL);
> +           if (state != NULL)
> +             backtrace_full (state, 2, bt_callback, bt_err_callback,
> +                             (void *) &count);
> +         }
>
>         if (context->abort_on_error)
>           real_abort ();
> diff --git a/gcc/diagnostic.h b/gcc/diagnostic.h
> index dbd1703e0ef..440b547c6da 100644
> --- a/gcc/diagnostic.h
> +++ b/gcc/diagnostic.h
> @@ -120,6 +120,9 @@ struct diagnostic_context
>    /* True if we should raise a SIGABRT on errors.  */
>    bool abort_on_error;
>
> +  /* If true don't print backtraces for internal errors.  */
> +  bool disable_backtrace;
> +
>    /* True if we should show the column number on diagnostics.  */
>    bool show_column;
>
> @@ -245,6 +248,10 @@ diagnostic_inhibit_notes (diagnostic_context * context)
>  #define diagnostic_abort_on_error(DC) \
>    (DC)->abort_on_error = true
>
> +/* Don't print backtraces for internal errors.  */
> +#define diagnostic_disable_backtrace(DC) \
> +  (DC)->disable_backtrace = true;
> +
>  /* This diagnostic_context is used by front-ends that directly output
>     diagnostic messages without going through `error', `warning',
>     and similar functions.  */
> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> index 715830a1a43..677e78c7078 100644
> --- a/gcc/doc/invoke.texi
> +++ b/gcc/doc/invoke.texi
> @@ -12889,6 +12889,12 @@ Produce all the dumps listed above.
>  @opindex dA
>  Annotate the assembler output with miscellaneous debugging information.
>
> +@item -dB
> +@opindex dB
> +Do not print backtraces on compiler crashes. This speeds up reducing
> +test cases for compiler crashes, because printing backtraces is relatively
> +slow.
> +
>  @item -dD
>  @opindex dD
>  Dump all macro definitions, at the end of preprocessing, in addition to
> diff --git a/gcc/opts.c b/gcc/opts.c
> index ffedb10f18f..0c1da107714 100644
> --- a/gcc/opts.c
> +++ b/gcc/opts.c
> @@ -2593,6 +2593,9 @@ decode_d_option (const char *arg, struct gcc_options *opts,
>        case 'a':
>         opts->x_flag_dump_all_passed = true;
>         break;
> +      case 'B':
> +       diagnostic_disable_backtrace (dc);
> +       break;
>
>        default:
>           warning_at (loc, 0, "unrecognized gcc debugging option: %c", c);
> --
> 2.12.2
>
Andrew Pinski May 17, 2017, 3:43 a.m. UTC | #2
On Tue, May 16, 2017 at 7:16 PM, Andi Kleen <andi@firstfloor.org> wrote:
> From: Andi Kleen <ak@linux.intel.com>
>
> When running creduce on an ICE substantial amounts of the total
> CPU time go to backtrace_qsort() (sorting dwarf of the compiler) for
> printing the backtrace of the ICE. When running a reduction we don't need the
> backtrace. So add a -dB option to turn it off, and make reduction
> a bit faster.

Now let me comment on the patch itself because I think it can be
simplified a lot.

>
> Passes bootstrap and testing on x86_64-linux
>
> gcc/:
>
> 2017-05-16  Andi Kleen  <ak@linux.intel.com>
>
>         * diagnostic.c (diagnostic_initialize): Set disable_backtrace to
>         false.
>         (diagnostic_action_after_output): Check disable_backtrace.
>         * diagnostic.h (struct diagnostic_context): Add
>         disable_backtrace.
>         * doc/invoke.texi (-dB): Document.
>         * opts.c (decode_d_option): Handle -dB.
> ---
>  gcc/diagnostic.c    | 16 ++++++++++------
>  gcc/diagnostic.h    |  7 +++++++
>  gcc/doc/invoke.texi |  6 ++++++
>  gcc/opts.c          |  3 +++
>  4 files changed, 26 insertions(+), 6 deletions(-)
>
> diff --git a/gcc/diagnostic.c b/gcc/diagnostic.c
> index 158519606f8..bbac3f384d4 100644
> --- a/gcc/diagnostic.c
> +++ b/gcc/diagnostic.c
> @@ -156,6 +156,7 @@ diagnostic_initialize (diagnostic_context *context, int n_opts)
>      context->caret_chars[i] = '^';
>    context->show_option_requested = false;
>    context->abort_on_error = false;
> +  context->disable_backtrace = false;
>    context->show_column = false;
>    context->pedantic_errors = false;
>    context->permissive = false;
> @@ -500,13 +501,16 @@ diagnostic_action_after_output (diagnostic_context *context,
>      case DK_ICE:
>      case DK_ICE_NOBT:
>        {
> -       struct backtrace_state *state = NULL;
> -       if (diag_kind == DK_ICE)
> -         state = backtrace_create_state (NULL, 0, bt_err_callback, NULL);
>         int count = 0;
> -       if (state != NULL)
> -         backtrace_full (state, 2, bt_callback, bt_err_callback,
> -                         (void *) &count);
> +       if (!context->disable_backtrace)
> +         {
> +           struct backtrace_state *state = NULL;
> +           if (diag_kind == DK_ICE)

Why not put && !context->disable_backtrace in this if statement
instead of wrapping the whole thing in an another if statement?

That is:
       struct backtrace_state *state = NULL;
       if (diag_kind == DK_ICE && !context->disable_backtrace)
         state = backtrace_create_state (NULL, 0, bt_err_callback, NULL);
       int count = 0;
       if (state != NULL)
          backtrace_full (state, 2, bt_callback, bt_err_callback,
                                  (void *) &count);

Changing only one line and instead of many.

Thanks,
Andrew Pinski

> +             state = backtrace_create_state (NULL, 0, bt_err_callback, NULL);
> +           if (state != NULL)
> +             backtrace_full (state, 2, bt_callback, bt_err_callback,
> +                             (void *) &count);
> +         }
>
>         if (context->abort_on_error)
>           real_abort ();
> diff --git a/gcc/diagnostic.h b/gcc/diagnostic.h
> index dbd1703e0ef..440b547c6da 100644
> --- a/gcc/diagnostic.h
> +++ b/gcc/diagnostic.h
> @@ -120,6 +120,9 @@ struct diagnostic_context
>    /* True if we should raise a SIGABRT on errors.  */
>    bool abort_on_error;
>
> +  /* If true don't print backtraces for internal errors.  */
> +  bool disable_backtrace;
> +
>    /* True if we should show the column number on diagnostics.  */
>    bool show_column;
>
> @@ -245,6 +248,10 @@ diagnostic_inhibit_notes (diagnostic_context * context)
>  #define diagnostic_abort_on_error(DC) \
>    (DC)->abort_on_error = true
>
> +/* Don't print backtraces for internal errors.  */
> +#define diagnostic_disable_backtrace(DC) \
> +  (DC)->disable_backtrace = true;
> +
>  /* This diagnostic_context is used by front-ends that directly output
>     diagnostic messages without going through `error', `warning',
>     and similar functions.  */
> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> index 715830a1a43..677e78c7078 100644
> --- a/gcc/doc/invoke.texi
> +++ b/gcc/doc/invoke.texi
> @@ -12889,6 +12889,12 @@ Produce all the dumps listed above.
>  @opindex dA
>  Annotate the assembler output with miscellaneous debugging information.
>
> +@item -dB
> +@opindex dB
> +Do not print backtraces on compiler crashes. This speeds up reducing
> +test cases for compiler crashes, because printing backtraces is relatively
> +slow.
> +
>  @item -dD
>  @opindex dD
>  Dump all macro definitions, at the end of preprocessing, in addition to
> diff --git a/gcc/opts.c b/gcc/opts.c
> index ffedb10f18f..0c1da107714 100644
> --- a/gcc/opts.c
> +++ b/gcc/opts.c
> @@ -2593,6 +2593,9 @@ decode_d_option (const char *arg, struct gcc_options *opts,
>        case 'a':
>         opts->x_flag_dump_all_passed = true;
>         break;
> +      case 'B':
> +       diagnostic_disable_backtrace (dc);
> +       break;
>
>        default:
>           warning_at (loc, 0, "unrecognized gcc debugging option: %c", c);
> --
> 2.12.2
>
Markus Trippelsdorf May 17, 2017, 5:26 a.m. UTC | #3
On 2017.05.16 at 19:16 -0700, Andi Kleen wrote:
> From: Andi Kleen <ak@linux.intel.com>
> 
> When running creduce on an ICE substantial amounts of the total
> CPU time go to backtrace_qsort() (sorting dwarf of the compiler) for
> printing the backtrace of the ICE. When running a reduction we don't need the
> backtrace. So add a -dB option to turn it off, and make reduction
> a bit faster.

It is useful when reducing compiler segfaults, because here one should
grep for a symbol in the backtrace to not end up with an unrelated
crash.
Andi Kleen May 17, 2017, 2:13 p.m. UTC | #4
On Tue, May 16, 2017 at 08:40:02PM -0700, Andrew Pinski wrote:
> On Tue, May 16, 2017 at 7:16 PM, Andi Kleen <andi@firstfloor.org> wrote:
> > From: Andi Kleen <ak@linux.intel.com>
> >
> > When running creduce on an ICE substantial amounts of the total
> > CPU time go to backtrace_qsort() (sorting dwarf of the compiler) for
> > printing the backtrace of the ICE. When running a reduction we don't need the
> > backtrace. So add a -dB option to turn it off, and make reduction
> > a bit faster.
> 
> The other thing which you could is strip the binaries.  :)
> j/k.  I think this is a good patch and a good idea.

AFAIK the sort is for the unwind tables. strip removes .dwarf*, but not .eh_*
It can't because that would break C++ exceptions.

-Andi
mihailo.stojanovic--- via Gcc-patches May 17, 2017, 2:49 p.m. UTC | #5
On Wed, May 17, 2017 at 7:13 AM, Andi Kleen <ak@linux.intel.com> wrote:
> On Tue, May 16, 2017 at 08:40:02PM -0700, Andrew Pinski wrote:
>> On Tue, May 16, 2017 at 7:16 PM, Andi Kleen <andi@firstfloor.org> wrote:
>> > From: Andi Kleen <ak@linux.intel.com>
>> >
>> > When running creduce on an ICE substantial amounts of the total
>> > CPU time go to backtrace_qsort() (sorting dwarf of the compiler) for
>> > printing the backtrace of the ICE. When running a reduction we don't need the
>> > backtrace. So add a -dB option to turn it off, and make reduction
>> > a bit faster.
>>
>> The other thing which you could is strip the binaries.  :)
>> j/k.  I think this is a good patch and a good idea.
>
> AFAIK the sort is for the unwind tables. strip removes .dwarf*, but not .eh_*
> It can't because that would break C++ exceptions.

This is libbacktrace.  The calls to backtrace_qsort are for the DWARF
information.  libbacktrace sorts the debug info so it can do faster
lookups of PC values.

That said I'm surprised it takes that much time.  The DWARF info is
usually mostly sorted, and backtrace_qsort is optimized for
information that is mostly sorted.  If you feel like spending time on
this it might be worth figuring out why it is slow.

Ian
diff mbox

Patch

diff --git a/gcc/diagnostic.c b/gcc/diagnostic.c
index 158519606f8..bbac3f384d4 100644
--- a/gcc/diagnostic.c
+++ b/gcc/diagnostic.c
@@ -156,6 +156,7 @@  diagnostic_initialize (diagnostic_context *context, int n_opts)
     context->caret_chars[i] = '^';
   context->show_option_requested = false;
   context->abort_on_error = false;
+  context->disable_backtrace = false;
   context->show_column = false;
   context->pedantic_errors = false;
   context->permissive = false;
@@ -500,13 +501,16 @@  diagnostic_action_after_output (diagnostic_context *context,
     case DK_ICE:
     case DK_ICE_NOBT:
       {
-	struct backtrace_state *state = NULL;
-	if (diag_kind == DK_ICE)
-	  state = backtrace_create_state (NULL, 0, bt_err_callback, NULL);
 	int count = 0;
-	if (state != NULL)
-	  backtrace_full (state, 2, bt_callback, bt_err_callback,
-			  (void *) &count);
+	if (!context->disable_backtrace)
+	  {
+	    struct backtrace_state *state = NULL;
+	    if (diag_kind == DK_ICE)
+	      state = backtrace_create_state (NULL, 0, bt_err_callback, NULL);
+	    if (state != NULL)
+	      backtrace_full (state, 2, bt_callback, bt_err_callback,
+			      (void *) &count);
+	  }
 
 	if (context->abort_on_error)
 	  real_abort ();
diff --git a/gcc/diagnostic.h b/gcc/diagnostic.h
index dbd1703e0ef..440b547c6da 100644
--- a/gcc/diagnostic.h
+++ b/gcc/diagnostic.h
@@ -120,6 +120,9 @@  struct diagnostic_context
   /* True if we should raise a SIGABRT on errors.  */
   bool abort_on_error;
 
+  /* If true don't print backtraces for internal errors.  */
+  bool disable_backtrace;
+
   /* True if we should show the column number on diagnostics.  */
   bool show_column;
 
@@ -245,6 +248,10 @@  diagnostic_inhibit_notes (diagnostic_context * context)
 #define diagnostic_abort_on_error(DC) \
   (DC)->abort_on_error = true
 
+/* Don't print backtraces for internal errors.  */
+#define diagnostic_disable_backtrace(DC) \
+  (DC)->disable_backtrace = true;
+
 /* This diagnostic_context is used by front-ends that directly output
    diagnostic messages without going through `error', `warning',
    and similar functions.  */
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 715830a1a43..677e78c7078 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -12889,6 +12889,12 @@  Produce all the dumps listed above.
 @opindex dA
 Annotate the assembler output with miscellaneous debugging information.
 
+@item -dB
+@opindex dB
+Do not print backtraces on compiler crashes. This speeds up reducing
+test cases for compiler crashes, because printing backtraces is relatively
+slow.
+
 @item -dD
 @opindex dD
 Dump all macro definitions, at the end of preprocessing, in addition to
diff --git a/gcc/opts.c b/gcc/opts.c
index ffedb10f18f..0c1da107714 100644
--- a/gcc/opts.c
+++ b/gcc/opts.c
@@ -2593,6 +2593,9 @@  decode_d_option (const char *arg, struct gcc_options *opts,
       case 'a':
 	opts->x_flag_dump_all_passed = true;
 	break;
+      case 'B':
+	diagnostic_disable_backtrace (dc);
+	break;
 
       default:
 	  warning_at (loc, 0, "unrecognized gcc debugging option: %c", c);