From e9f2d3f009d0c9da59758a6e14cddf1cebae1f59 Mon Sep 17 00:00:00 2001 From: whitequark Date: Fri, 24 Apr 2020 19:37:47 +0000 Subject: [PATCH] kernel: Trap in `log_error()` when a debugger is attached. The workflow of debugging fatal pass errors in Yosys is flawed in three ways: 1. Running Yosys under a debugger is sufficient for the debugger to catch some fatal errors (segfaults, aborts, STL exceptions) but not others (`log_error()`, `log_cmd_error()`). This is neither obvious nor easy to remember. 2. To catch Yosys-specific fatal errors, it is necessary to set a breakpoint at `logv_error_with_prefix()`, or at least, `logv_error()`. This is neither obvious nor easy to remember, and GDB's autocomplete takes many seconds to suggest function names due to the large amount of symbols in Yosys. 3. If a breakpoint is not set and Yosys encounters with such a fatal error, the process terminates. When debugging a crash that takes a long time to reproduce (or a nondeterministic crash) this can waste a significant amount of time. To solve this problem, add a macro `YS_DEBUGTRAP` that acts as a hard breakpoint (if available), and a macro `YS_DEBUGTRAP_IF_DEBUGGING` that acts as a hard breakpoint only if debugger is present. Then, use `YS_DEBUGTRAP_IF_DEBUGGING` in `logv_error_with_prefix()` to obviate the need for a breakpoint on nearly every platform. Co-Authored-By: Alberto Gonzalez --- kernel/log.cc | 7 +++++-- kernel/log.h | 40 +++++++++++++++++++++++++++++++++++++++- 2 files changed, 44 insertions(+), 3 deletions(-) diff --git a/kernel/log.cc b/kernel/log.cc index d84a4381e..a21ba480a 100644 --- a/kernel/log.cc +++ b/kernel/log.cc @@ -354,6 +354,9 @@ static void logv_error_with_prefix(const char *prefix, if (check_expected_logs) log_check_expected(); + + YS_DEBUGTRAP_IF_DEBUGGING; + #ifdef EMSCRIPTEN log_files = backup_log_files; throw 0; @@ -673,7 +676,7 @@ void log_check_expected() } if (item.second.current_count != item.second.expected_count) { log_warn_regexes.clear(); - log_error("Expected warning pattern '%s' found %d time(s), instead of %d time(s) !\n", + log_error("Expected warning pattern '%s' found %d time(s), instead of %d time(s) !\n", item.second.pattern.c_str(), item.second.current_count, item.second.expected_count); } } @@ -700,7 +703,7 @@ void log_check_expected() _exit(0); #else _Exit(0); - #endif + #endif } else { display_error_log_msg = false; log_warn_regexes.clear(); diff --git a/kernel/log.h b/kernel/log.h index 5478482ac..dee5d44d7 100644 --- a/kernel/log.h +++ b/kernel/log.h @@ -50,9 +50,12 @@ std::regex_constants::egrep) #endif -#ifndef _WIN32 +#if defined(_WIN32) +# include +#else # include # include +# include #endif #if defined(_MSC_VER) @@ -69,6 +72,41 @@ YOSYS_NAMESPACE_BEGIN #define S__LINE__sub1(x) S__LINE__sub2(x) #define S__LINE__ S__LINE__sub1(__LINE__) +// YS_DEBUGTRAP is a macro that is functionally equivalent to a breakpoint +// if the platform provides such functionality, and does nothing otherwise. +// If no debugger is attached, it starts a just-in-time debugger if available, +// and crashes the process otherwise. +#if defined(_WIN32) +# define YS_DEBUGTRAP __debugbreak() +#else +# ifndef __has_builtin +// __has_builtin is a GCC/Clang extension; on a different compiler (or old enough GCC/Clang) +// that does not have it, using __has_builtin(...) is a syntax error. +# define __has_builtin(x) 0 +# endif +# if __has_builtin(__builtin_debugtrap) +# define YS_DEBUGTRAP __builtin_debugtrap() +# elif defined(__unix__) +# define YS_DEBUGTRAP raise(SIGTRAP) +# else +# define YS_DEBUGTRAP do {} while(0) +# endif +#endif + +// YS_DEBUGTRAP_IF_DEBUGGING is a macro that is functionally equivalent to a breakpoint +// if a debugger is attached, and does nothing otherwise. +#if defined(_WIN32) +# define YS_DEBUGTRAP_IF_DEBUGGING do { if (IsDebuggerPresent()) DebugBreak(); } while(0) +#elif defined(__unix__) +// There is no reliable (or portable) *nix equivalent of IsDebuggerPresent(). However, +// debuggers will stop when SIGTRAP is raised, even if the action is set to ignore. +# define YS_DEBUGTRAP_IF_DEBUGGING do { \ + sighandler_t old = signal(SIGTRAP, SIG_IGN); raise(SIGTRAP); signal(SIGTRAP, old); \ + } while(0) +#else +# define YS_DEBUGTRAP_IF_DEBUGGING do {} while(0) +#endif + struct log_cmd_error_exception { }; extern std::vector log_files; -- 2.30.2