From 9c3ce69180c103c80066065f3df45fc13096aa85 Mon Sep 17 00:00:00 2001 From: Rangi <35663410+Rangi42@users.noreply.github.com> Date: Tue, 12 Aug 2025 17:56:54 -0400 Subject: [PATCH] Factor out shared backtrace code (#1793) --- Makefile | 2 ++ include/backtrace.hpp | 69 +++++++++++++++++++++++++++++++++++++++++ include/diagnostics.hpp | 3 -- src/CMakeLists.txt | 2 ++ src/asm/fstack.cpp | 53 ++++--------------------------- src/asm/main.cpp | 17 ++-------- src/asm/warning.cpp | 1 - src/backtrace.cpp | 21 +++++++++++++ src/fix/warning.cpp | 1 - src/gfx/warning.cpp | 1 - src/link/fstack.cpp | 53 ++++--------------------------- src/link/lexer.cpp | 52 ++++--------------------------- src/link/main.cpp | 15 ++------- src/link/warning.cpp | 1 - 14 files changed, 118 insertions(+), 173 deletions(-) create mode 100644 include/backtrace.hpp create mode 100644 src/backtrace.cpp diff --git a/Makefile b/Makefile index 3e4d3fb3..77f00688 100644 --- a/Makefile +++ b/Makefile @@ -73,6 +73,7 @@ rgbasm_obj := \ src/asm/symbol.o \ src/asm/warning.o \ src/extern/utf8decoder.o \ + src/backtrace.o \ src/linkdefs.o \ src/opmath.o \ src/util.o \ @@ -96,6 +97,7 @@ rgblink_obj := \ src/link/symbol.o \ src/link/warning.o \ src/extern/utf8decoder.o \ + src/backtrace.o \ src/linkdefs.o \ src/opmath.o \ src/util.o \ diff --git a/include/backtrace.hpp b/include/backtrace.hpp new file mode 100644 index 00000000..16fcbfcf --- /dev/null +++ b/include/backtrace.hpp @@ -0,0 +1,69 @@ +// SPDX-License-Identifier: MIT + +#ifndef RGBDS_BACKTRACE_HPP +#define RGBDS_BACKTRACE_HPP + +#include +#include +#include +#include +#include + +#include "style.hpp" + +static constexpr uint64_t TRACE_COLLAPSE = UINT64_MAX; + +extern uint64_t traceDepth; + +bool trace_ParseTraceDepth(char const *arg); + +template +void trace_PrintBacktrace(std::vector const &stack, M getName, N getLineNo) { + auto printLocation = [&](T const &item) { + style_Set(stderr, STYLE_CYAN, true); + fputs(getName(item), stderr); + style_Set(stderr, STYLE_CYAN, false); + fprintf(stderr, "(%" PRIu32 ")", getLineNo(item)); + }; + + size_t n = stack.size(); + + if (traceDepth == TRACE_COLLAPSE) { + fputs(" ", stderr); // Just three spaces; the fourth will be handled by the loop + for (size_t i = 0; i < n; ++i) { + style_Reset(stderr); + fprintf(stderr, " %s ", i == 0 ? "at" : "<-"); + printLocation(stack[n - i - 1]); + } + putc('\n', stderr); + } else if (traceDepth == 0 || static_cast(traceDepth) >= n) { + for (size_t i = 0; i < n; ++i) { + style_Reset(stderr); + fprintf(stderr, " %s ", i == 0 ? "at" : "<-"); + printLocation(stack[n - i - 1]); + putc('\n', stderr); + } + } else { + size_t last = traceDepth / 2; + size_t first = traceDepth - last; + size_t skipped = n - traceDepth; + for (size_t i = 0; i < first; ++i) { + style_Reset(stderr); + fprintf(stderr, " %s ", i == 0 ? "at" : "<-"); + printLocation(stack[n - i - 1]); + putc('\n', stderr); + } + style_Reset(stderr); + fprintf(stderr, " ...%zu more%s\n", skipped, last ? "..." : ""); + for (size_t i = n - last; i < n; ++i) { + style_Reset(stderr); + fputs(" <- ", stderr); + printLocation(stack[n - i - 1]); + putc('\n', stderr); + } + } + + style_Reset(stderr); +} + +#endif // RGBDS_BACKTRACE_HPP diff --git a/include/diagnostics.hpp b/include/diagnostics.hpp index 0c9558e3..3a8dc7d8 100644 --- a/include/diagnostics.hpp +++ b/include/diagnostics.hpp @@ -53,15 +53,12 @@ struct DiagnosticsState { bool warningsAreErrors = false; }; -static constexpr uint64_t TRACE_COLLAPSE = UINT64_MAX; - template struct Diagnostics { std::vector> metaWarnings; std::vector> warningFlags; std::vector> paramWarnings; DiagnosticsState state; - uint64_t traceDepth; uint64_t nbErrors; void incrementErrors() { diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 272251ca..57359c22 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -51,6 +51,7 @@ set(rgbasm_src "asm/symbol.cpp" "asm/warning.cpp" "extern/utf8decoder.cpp" + "backtrace.cpp" "linkdefs.cpp" "opmath.cpp" "util.cpp" @@ -72,6 +73,7 @@ set(rgblink_src "link/symbol.cpp" "link/warning.cpp" "extern/utf8decoder.cpp" + "backtrace.cpp" "linkdefs.cpp" "opmath.cpp" "util.cpp" diff --git a/src/asm/fstack.cpp b/src/asm/fstack.cpp index 1b5e6515..d3bbe04d 100644 --- a/src/asm/fstack.cpp +++ b/src/asm/fstack.cpp @@ -11,11 +11,11 @@ #include #include +#include "backtrace.hpp" #include "diagnostics.hpp" #include "helpers.hpp" #include "linkdefs.hpp" #include "platform.hpp" // S_ISDIR (stat macro) -#include "style.hpp" #include "verbosity.hpp" #include "asm/lexer.hpp" @@ -78,53 +78,12 @@ std::vector> FileStackNode::backtrace(uint32_t } } -static void printNode(std::pair const &node) { - style_Set(stderr, STYLE_CYAN, true); - fputs(node.first.c_str(), stderr); - style_Set(stderr, STYLE_CYAN, false); - fprintf(stderr, "(%" PRIu32 ")", node.second); -} - void FileStackNode::printBacktrace(uint32_t curLineNo) const { - std::vector> nodes = backtrace(curLineNo); - size_t n = nodes.size(); - - if (warnings.traceDepth == TRACE_COLLAPSE) { - fputs(" ", stderr); // Just three spaces; the fourth will be handled by the loop - for (size_t i = 0; i < n; ++i) { - style_Reset(stderr); - fprintf(stderr, " %s ", i == 0 ? "at" : "<-"); - printNode(nodes[n - i - 1]); - } - putc('\n', stderr); - } else if (warnings.traceDepth == 0 || static_cast(warnings.traceDepth) >= n) { - for (size_t i = 0; i < n; ++i) { - style_Reset(stderr); - fprintf(stderr, " %s ", i == 0 ? "at" : "<-"); - printNode(nodes[n - i - 1]); - putc('\n', stderr); - } - } else { - size_t last = warnings.traceDepth / 2; - size_t first = warnings.traceDepth - last; - size_t skipped = n - warnings.traceDepth; - for (size_t i = 0; i < first; ++i) { - style_Reset(stderr); - fprintf(stderr, " %s ", i == 0 ? "at" : "<-"); - printNode(nodes[n - i - 1]); - putc('\n', stderr); - } - style_Reset(stderr); - fprintf(stderr, " ...%zu more%s\n", skipped, last ? "..." : ""); - for (size_t i = n - last; i < n; ++i) { - style_Reset(stderr); - fputs(" <- ", stderr); - printNode(nodes[n - i - 1]); - putc('\n', stderr); - } - } - - style_Reset(stderr); + trace_PrintBacktrace( + backtrace(curLineNo), + [](std::pair const &node) { return node.first.c_str(); }, + [](std::pair const &node) { return node.second; } + ); } void fstk_TraceCurrent() { diff --git a/src/asm/main.cpp b/src/asm/main.cpp index 47112a3d..213ed5a1 100644 --- a/src/asm/main.cpp +++ b/src/asm/main.cpp @@ -11,6 +11,7 @@ #include #include +#include "backtrace.hpp" #include "diagnostics.hpp" #include "extern/getopt.hpp" #include "helpers.hpp" @@ -303,23 +304,11 @@ int main(int argc, char *argv[]) { switch (ch) { char *endptr; - case 'B': { - if (!strcasecmp(musl_optarg, "collapse")) { - warnings.traceDepth = TRACE_COLLAPSE; - break; - } - - warnings.traceDepth = strtoul(musl_optarg, &endptr, 0); - - if (musl_optarg[0] == '\0' || *endptr != '\0') { + case 'B': + if (!trace_ParseTraceDepth(musl_optarg)) { fatal("Invalid argument for option '-B'"); } - - if (warnings.traceDepth >= UINT64_MAX) { - fatal("Argument for option '-B' is too large"); - } break; - } case 'b': if (strlen(musl_optarg) == 2) { diff --git a/src/asm/warning.cpp b/src/asm/warning.cpp index 725c7e8f..fc0f3918 100644 --- a/src/asm/warning.cpp +++ b/src/asm/warning.cpp @@ -60,7 +60,6 @@ Diagnostics warnings = { {WARNING_UNMAPPED_CHAR_1, WARNING_UNMAPPED_CHAR_2, 1}, }, .state = DiagnosticsState(), - .traceDepth = 0, .nbErrors = 0, }; // clang-format on diff --git a/src/backtrace.cpp b/src/backtrace.cpp new file mode 100644 index 00000000..b1fdb6b8 --- /dev/null +++ b/src/backtrace.cpp @@ -0,0 +1,21 @@ +// SPDX-License-Identifier: MIT + +#include "backtrace.hpp" + +#include // strtoul + +#include "platform.hpp" // strcasecmp + +uint64_t traceDepth = 0; + +bool trace_ParseTraceDepth(char const *arg) { + if (!strcasecmp(arg, "collapse")) { + traceDepth = TRACE_COLLAPSE; + return true; + } + + char *endptr; + traceDepth = strtoul(arg, &endptr, 0); + + return arg[0] != '\0' && *endptr == '\0' && traceDepth != TRACE_COLLAPSE; +} diff --git a/src/fix/warning.cpp b/src/fix/warning.cpp index 537ea8aa..b6a154fc 100644 --- a/src/fix/warning.cpp +++ b/src/fix/warning.cpp @@ -19,7 +19,6 @@ Diagnostics warnings = { }, .paramWarnings = {}, .state = DiagnosticsState(), - .traceDepth = 0, .nbErrors = 0, }; // clang-format on diff --git a/src/gfx/warning.cpp b/src/gfx/warning.cpp index d4421a2a..5a631d68 100644 --- a/src/gfx/warning.cpp +++ b/src/gfx/warning.cpp @@ -22,7 +22,6 @@ Diagnostics warnings = { }, .paramWarnings = {}, .state = DiagnosticsState(), - .traceDepth = 0, .nbErrors = 0, }; // clang-format on diff --git a/src/link/fstack.cpp b/src/link/fstack.cpp index c7038928..f37f6db6 100644 --- a/src/link/fstack.cpp +++ b/src/link/fstack.cpp @@ -3,7 +3,7 @@ #include #include -#include "style.hpp" +#include "backtrace.hpp" #include "link/warning.hpp" @@ -30,51 +30,10 @@ std::vector> FileStackNode::backtrace(uint32_t } } -static void printNode(std::pair const &node) { - style_Set(stderr, STYLE_CYAN, true); - fputs(node.first.c_str(), stderr); - style_Set(stderr, STYLE_CYAN, false); - fprintf(stderr, "(%" PRIu32 ")", node.second); -} - void FileStackNode::printBacktrace(uint32_t curLineNo) const { - std::vector> nodes = backtrace(curLineNo); - size_t n = nodes.size(); - - if (warnings.traceDepth == TRACE_COLLAPSE) { - fputs(" ", stderr); // Just three spaces; the fourth will be handled by the loop - for (size_t i = 0; i < n; ++i) { - style_Reset(stderr); - fprintf(stderr, " %s ", i == 0 ? "at" : "<-"); - printNode(nodes[n - i - 1]); - } - putc('\n', stderr); - } else if (warnings.traceDepth == 0 || static_cast(warnings.traceDepth) >= n) { - for (size_t i = 0; i < n; ++i) { - style_Reset(stderr); - fprintf(stderr, " %s ", i == 0 ? "at" : "<-"); - printNode(nodes[n - i - 1]); - putc('\n', stderr); - } - } else { - size_t last = warnings.traceDepth / 2; - size_t first = warnings.traceDepth - last; - size_t skipped = n - warnings.traceDepth; - for (size_t i = 0; i < first; ++i) { - style_Reset(stderr); - fprintf(stderr, " %s ", i == 0 ? "at" : "<-"); - printNode(nodes[n - i - 1]); - putc('\n', stderr); - } - style_Reset(stderr); - fprintf(stderr, " ...%zu more%s\n", skipped, last ? "..." : ""); - for (size_t i = n - last; i < n; ++i) { - style_Reset(stderr); - fputs(" <- ", stderr); - printNode(nodes[n - i - 1]); - putc('\n', stderr); - } - } - - style_Reset(stderr); + trace_PrintBacktrace( + backtrace(curLineNo), + [](std::pair const &node) { return node.first.c_str(); }, + [](std::pair const &node) { return node.second; } + ); } diff --git a/src/link/lexer.cpp b/src/link/lexer.cpp index da9be6d2..68e6c810 100644 --- a/src/link/lexer.cpp +++ b/src/link/lexer.cpp @@ -9,9 +9,9 @@ #include #include +#include "backtrace.hpp" #include "helpers.hpp" #include "itertools.hpp" -#include "style.hpp" #include "util.hpp" #include "link/warning.hpp" @@ -28,52 +28,12 @@ struct LexerStackEntry { static std::vector lexerStack; -static void printStackEntry(LexerStackEntry const &context) { - style_Set(stderr, STYLE_CYAN, true); - fputs(context.path.c_str(), stderr); - style_Set(stderr, STYLE_CYAN, false); - fprintf(stderr, "(%" PRIu32 ")", context.lineNo); -} - void lexer_TraceCurrent() { - size_t n = lexerStack.size(); - - if (warnings.traceDepth == TRACE_COLLAPSE) { - fputs(" ", stderr); // Just three spaces; the fourth will be handled by the loop - for (size_t i = 0; i < n; ++i) { - style_Reset(stderr); - fprintf(stderr, " %s ", i == 0 ? "at" : "<-"); - printStackEntry(lexerStack[n - i - 1]); - } - putc('\n', stderr); - } else if (warnings.traceDepth == 0 || static_cast(warnings.traceDepth) >= n) { - for (size_t i = 0; i < n; ++i) { - style_Reset(stderr); - fprintf(stderr, " %s ", i == 0 ? "at" : "<-"); - printStackEntry(lexerStack[n - i - 1]); - putc('\n', stderr); - } - } else { - size_t last = warnings.traceDepth / 2; - size_t first = warnings.traceDepth - last; - size_t skipped = n - warnings.traceDepth; - for (size_t i = 0; i < first; ++i) { - style_Reset(stderr); - fprintf(stderr, " %s ", i == 0 ? "at" : "<-"); - printStackEntry(lexerStack[n - i - 1]); - putc('\n', stderr); - } - style_Reset(stderr); - fprintf(stderr, " ...%zu more%s\n", skipped, last ? "..." : ""); - for (size_t i = n - last; i < n; ++i) { - style_Reset(stderr); - fputs(" <- ", stderr); - printStackEntry(lexerStack[n - i - 1]); - putc('\n', stderr); - } - } - - style_Reset(stderr); + trace_PrintBacktrace( + lexerStack, + [](LexerStackEntry const &context) { return context.path.c_str(); }, + [](LexerStackEntry const &context) { return context.lineNo; } + ); } void lexer_IncludeFile(std::string &&path) { diff --git a/src/link/main.cpp b/src/link/main.cpp index 5aa509fa..d02496e5 100644 --- a/src/link/main.cpp +++ b/src/link/main.cpp @@ -10,6 +10,7 @@ #include #include +#include "backtrace.hpp" #include "diagnostics.hpp" #include "extern/getopt.hpp" #include "helpers.hpp" // assume @@ -295,21 +296,11 @@ int main(int argc, char *argv[]) { // Parse options for (int ch; (ch = musl_getopt_long_only(argc, argv, optstring, longopts, nullptr)) != -1;) { switch (ch) { - case 'B': { - if (!strcasecmp(musl_optarg, "collapse")) { - warnings.traceDepth = TRACE_COLLAPSE; - break; - } - char *endptr; - warnings.traceDepth = strtoul(musl_optarg, &endptr, 0); - if (musl_optarg[0] == '\0' || *endptr != '\0') { + case 'B': + if (!trace_ParseTraceDepth(musl_optarg)) { fatal("Invalid argument for option '-B'"); } - if (warnings.traceDepth >= UINT64_MAX) { - fatal("Argument for option '-B' is too large"); - } break; - } case 'd': options.isDmgMode = true; options.isWRAM0Mode = true; diff --git a/src/link/warning.cpp b/src/link/warning.cpp index 451a5d31..f3c2380b 100644 --- a/src/link/warning.cpp +++ b/src/link/warning.cpp @@ -26,7 +26,6 @@ Diagnostics warnings = { }, .paramWarnings = {}, .state = DiagnosticsState(), - .traceDepth = 0, .nbErrors = 0, }; // clang-format on