From: whitequark <whitequark@whitequark.org>
Date: Fri, 24 Apr 2020 19:37:47 +0000 (+0000)
Subject: kernel: Trap in `log_error()` when a debugger is attached.
X-Git-Tag: working-ls180~574^2
X-Git-Url: https://git.libre-soc.org/?a=commitdiff_plain;h=e9f2d3f009d0c9da59758a6e14cddf1cebae1f59;p=yosys.git

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 <boqwxp@airmail.cc>
---

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 <intrin.h>
+#else
 #  include <sys/time.h>
 #  include <sys/resource.h>
+#  include <signal.h>
 #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<FILE*> log_files;