Re: [PATCH] Dump function on internal errors

[Richard Biener <richard dot guenther at gmail dot com>]

On Mon, May 22, 2017 at 4:19 PM, Andi Kleen <andi@firstfloor.org> wrote:
> From: Andi Kleen <ak@linux.intel.com>
>
> When a verification check fails it is useful to dump the current
> function to the dump file, so it's easier to figure out what
> actually went wrong.
>
> v2: Updated version now using a hook in internal_error, and
> also prints the pass name and the dump file name.
>
> gcc/:
>
> 2017-05-21  Andi Kleen  <ak@linux.intel.com>
>
>         * diagnostic.c (internal_error): Call hook.
>         (set_internal_error_hook): New function.
>         * diagnostic.h: Add set_internal_error_hook.
>         * passes.c: (pass_ice_hook): Add class to set
>         internal_error_hook to dump functions.
> ---
>  gcc/diagnostic.c | 18 ++++++++++++++++++
>  gcc/diagnostic.h |  1 +
>  gcc/passes.c     | 21 +++++++++++++++++++++
>  3 files changed, 40 insertions(+)
>
> diff --git a/gcc/diagnostic.c b/gcc/diagnostic.c
> index 158519606f8..8d9caade60b 100644
> --- a/gcc/diagnostic.c
> +++ b/gcc/diagnostic.c
> @@ -33,6 +33,9 @@ along with GCC; see the file COPYING3.  If not see
>  #include "diagnostic-color.h"
>  #include "edit-context.h"
>  #include "selftest.h"
> +#include "tree.h"
> +#include "tree-pass.h"
> +#include "tree-cfg.h"
>
>  #ifdef HAVE_TERMIOS_H
>  # include <termios.h>
> @@ -1396,6 +1399,10 @@ fatal_error (location_t loc, const char *gmsgid, ...)
>    gcc_unreachable ();
>  }
>
> +/* Hook to call on internal errors.  */
> +
> +static void (*internal_error_hook)();
> +
>  /* An internal consistency check has failed.  We make no attempt to
>     continue.  Note that unless there is debugging value to be had from
>     a more specific message, or some other good reason, you should use
> @@ -1403,6 +1410,9 @@ fatal_error (location_t loc, const char *gmsgid, ...)
>  void
>  internal_error (const char *gmsgid, ...)
>  {
> +  if (internal_error_hook)
> +    internal_error_hook ();
> +
>    va_list ap;
>    va_start (ap, gmsgid);
>    rich_location richloc (line_table, input_location);
> @@ -1412,6 +1422,14 @@ internal_error (const char *gmsgid, ...)
>    gcc_unreachable ();
>  }
>
> +/* Call HOOK on internal errors.  */
> +
> +void
> +set_internal_error_hook (void (*hook)())
> +{
> +  internal_error_hook = hook;
> +}
> +
>  /* Like internal_error, but no backtrace will be printed.  Used when
>     the internal error does not happen at the current location, but happened
>     somewhere else.  */
> diff --git a/gcc/diagnostic.h b/gcc/diagnostic.h
> index dbd1703e0ef..3d26a84ab44 100644
> --- a/gcc/diagnostic.h
> +++ b/gcc/diagnostic.h
> @@ -369,5 +369,6 @@ extern char *file_name_as_prefix (diagnostic_context *, const char *);
>
>  extern char *build_message_string (const char *, ...) ATTRIBUTE_PRINTF_1;
>
> +extern void set_internal_error_hook (void (*)());
>
>  #endif /* ! GCC_DIAGNOSTIC_H */
> diff --git a/gcc/passes.c b/gcc/passes.c
> index e7c5d194010..1bd92c8140a 100644
> --- a/gcc/passes.c
> +++ b/gcc/passes.c
> @@ -3040,4 +3040,25 @@ function_called_by_processed_nodes_p (void)
>    return e != NULL;
>  }
>
> +class pass_ice_hook {
> +
> +  /* Print function to dump file on internal errors.  */
> +
> +  static void ice_hook() {
> +    if (dump_file)
> +      {
> +       fprintf (stderr, "Dumping function to dump file %s for pass %s\n",
> +                dump_file_name, current_pass->name);
> +       dump_function_to_file (current_function_decl, dump_file, dump_flags);
> +      }
> +  }
> +
> + public:
> +  pass_ice_hook() {
> +    set_internal_error_hook (ice_hook);
> +  }
> +};
> +
> +static pass_ice_hook pass_ice_hook;

current_pass might be NULL so you better do set_internal_error_hook when
we start executing passes (I detest global singletons to do such stuff anyway).

Otherwise looks reasonable to me.

Richard.

> +
>  #include "gt-passes.h"
> --
> 2.12.2
>