diff --git a/Makefile b/Makefile index 33373c8c..3e4d3fb3 100644 --- a/Makefile +++ b/Makefile @@ -83,6 +83,7 @@ src/asm/lexer.o src/asm/main.o: src/asm/parser.hpp rgblink_obj := \ ${common_obj} \ src/link/assign.o \ + src/link/fstack.o \ src/link/lexer.o \ src/link/layout.o \ src/link/main.o \ diff --git a/contrib/bash_compl/_rgbasm.bash b/contrib/bash_compl/_rgbasm.bash index 22d3746a..7ee09752 100755 --- a/contrib/bash_compl/_rgbasm.bash +++ b/contrib/bash_compl/_rgbasm.bash @@ -28,6 +28,7 @@ _rgbasm_completions() { [V]="version:normal" [W]="warning:warning" [w]=":normal" + [B]="backtrace:unk" [b]="binary-digits:unk" [D]="define:unk" [E]="export-all:normal" diff --git a/contrib/bash_compl/_rgblink.bash b/contrib/bash_compl/_rgblink.bash index 03191a73..9b94fccd 100755 --- a/contrib/bash_compl/_rgblink.bash +++ b/contrib/bash_compl/_rgblink.bash @@ -12,6 +12,7 @@ _rgblink_completions() { [W]="warning:warning" [M]="no-sym-in-map:normal" [d]="dmg:normal" + [B]="backtrace:unk" [l]="linkerscript:glob-*" [m]="map:glob-*.map" [n]="sym:glob-*.sym" diff --git a/contrib/zsh_compl/_rgbasm b/contrib/zsh_compl/_rgbasm index 23435aa9..be17b3e7 100644 --- a/contrib/zsh_compl/_rgbasm +++ b/contrib/zsh_compl/_rgbasm @@ -42,6 +42,7 @@ local args=( '(-v --verbose)'{-v,--verbose}'[Enable verbose output]' -w'[Disable all warnings]' + '(-B --backtrace)'{-B,--backtrace}'+[Set backtrace depth or style]:depth:' '(-b --binary-digits)'{-b,--binary-digits}'+[Change chars for binary constants]:digit spec:' --color'[Whether to use color in output]:color:(auto always never)' '*'{-D,--define}'+[Define a string symbol]:name + value (default 1):' diff --git a/contrib/zsh_compl/_rgblink b/contrib/zsh_compl/_rgblink index de40432f..6ddccc94 100644 --- a/contrib/zsh_compl/_rgblink +++ b/contrib/zsh_compl/_rgblink @@ -28,6 +28,7 @@ local args=( '(-w --wramx)'{-w,--wramx}'[Disable WRAM banking]' '(-x --nopad)'{-x,--nopad}'[Disable padding the end of the final file]' + '(-B --backtrace)'{-B,--backtrace}'+[Set backtrace depth or style]:depth:' --color'[Whether to use color in output]:color:(auto always never)' '(-l --linkerscript)'{-l,--linkerscript}"+[Use a linker script]:linker script:_files -g '*.link'" '(-M --no-sym-in-map)'{-M,--no-sym-in-map}'[Do not output symbol names in map file]' diff --git a/include/asm/fstack.hpp b/include/asm/fstack.hpp index ab2dea0f..9c87b23e 100644 --- a/include/asm/fstack.hpp +++ b/include/asm/fstack.hpp @@ -43,7 +43,8 @@ struct FileStackNode { FileStackNode(FileStackNodeType type_, std::variant, std::string> data_) : type(type_), data(data_) {} - std::string const &dump(uint32_t curLineNo) const; + void printBacktrace(uint32_t curLineNo) const; + std::vector> backtrace(uint32_t curLineNo) const; std::string reptChain() const; }; @@ -51,7 +52,7 @@ struct MacroArgs; void fstk_VerboseOutputConfig(); -bool fstk_DumpCurrent(); +void fstk_TraceCurrent(); std::shared_ptr fstk_GetFileStack(); std::shared_ptr fstk_GetUniqueIDStr(); MacroArgs *fstk_GetCurrentMacroArgs(); diff --git a/include/asm/lexer.hpp b/include/asm/lexer.hpp index 239e08c7..c86762de 100644 --- a/include/asm/lexer.hpp +++ b/include/asm/lexer.hpp @@ -133,7 +133,7 @@ void lexer_ReachELSEBlock(); void lexer_CheckRecursionDepth(); uint32_t lexer_GetLineNo(); -void lexer_DumpStringExpansions(); +void lexer_TraceStringExpansions(); struct Capture { uint32_t lineNo; diff --git a/include/asm/warning.hpp b/include/asm/warning.hpp index c51530ac..56e2cfff 100644 --- a/include/asm/warning.hpp +++ b/include/asm/warning.hpp @@ -74,11 +74,10 @@ void fatal(char const *fmt, ...); [[gnu::format(printf, 1, 2)]] void error(char const *fmt, ...); -// Used for errors that make it impossible to assemble correctly, but don't -// affect the following code. The code will fail to assemble but the user will -// get a list of all errors at the end, making it easier to fix all of them at -// once. -void error(std::function callback); +// Used for errors that handle their own backtrace output. The code will fail +// to assemble but the user will get a list of all errors at the end, making it +// easier to fix all of them at once. +void errorNoTrace(std::function callback); void requireZeroErrors(); diff --git a/include/diagnostics.hpp b/include/diagnostics.hpp index 3a8dc7d8..0c9558e3 100644 --- a/include/diagnostics.hpp +++ b/include/diagnostics.hpp @@ -53,12 +53,15 @@ 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/include/link/fstack.hpp b/include/link/fstack.hpp new file mode 100644 index 00000000..f5036b58 --- /dev/null +++ b/include/link/fstack.hpp @@ -0,0 +1,38 @@ +// SPDX-License-Identifier: MIT + +#ifndef RGBDS_LINK_FSTACK_HPP +#define RGBDS_LINK_FSTACK_HPP + +#include +#include +#include +#include +#include + +#include "linkdefs.hpp" + +struct FileStackNode { + FileStackNodeType type; + std::variant< + std::monostate, // Default constructed; `.type` and `.data` must be set manually + std::vector, // NODE_REPT + std::string // NODE_FILE, NODE_MACRO + > + data; + + FileStackNode *parent; + // Line at which the parent context was exited; meaningless for the root level + uint32_t lineNo; + + // REPT iteration counts since last named node, in reverse depth order + std::vector &iters() { return std::get>(data); } + std::vector const &iters() const { return std::get>(data); } + // File name for files, file::macro name for macros + std::string &name() { return std::get(data); } + std::string const &name() const { return std::get(data); } + + void printBacktrace(uint32_t curLineNo) const; + std::vector> backtrace(uint32_t curLineNo) const; +}; + +#endif // RGBDS_LINK_FSTACK_HPP diff --git a/include/link/main.hpp b/include/link/main.hpp index 86389e60..2d11c2a5 100644 --- a/include/link/main.hpp +++ b/include/link/main.hpp @@ -4,13 +4,6 @@ #define RGBDS_LINK_MAIN_HPP #include -#include -#include -#include -#include - -#include "linkdefs.hpp" -#include "verbosity.hpp" struct Options { bool isDmgMode; // -d @@ -32,27 +25,4 @@ struct Options { extern Options options; -struct FileStackNode { - FileStackNodeType type; - std::variant< - std::monostate, // Default constructed; `.type` and `.data` must be set manually - std::vector, // NODE_REPT - std::string // NODE_FILE, NODE_MACRO - > - data; - - FileStackNode *parent; - // Line at which the parent context was exited; meaningless for the root level - uint32_t lineNo; - - // REPT iteration counts since last named node, in reverse depth order - std::vector &iters() { return std::get>(data); } - std::vector const &iters() const { return std::get>(data); } - // File name for files, file::macro name for macros - std::string &name() { return std::get(data); } - std::string const &name() const { return std::get(data); } - - std::string const &dump(uint32_t curLineNo) const; -}; - #endif // RGBDS_LINK_MAIN_HPP diff --git a/include/link/symbol.hpp b/include/link/symbol.hpp index e79012e2..d6c8e776 100644 --- a/include/link/symbol.hpp +++ b/include/link/symbol.hpp @@ -44,6 +44,6 @@ void sym_AddSymbol(Symbol &symbol); // Finds a symbol in all the defined symbols. Symbol *sym_GetSymbol(std::string const &name); -void sym_DumpLocalAliasedSymbols(std::string const &name); +void sym_TraceLocalAliasedSymbols(std::string const &name); #endif // RGBDS_LINK_SYMBOL_HPP diff --git a/include/link/warning.hpp b/include/link/warning.hpp index bf7c3c3f..bbb4f659 100644 --- a/include/link/warning.hpp +++ b/include/link/warning.hpp @@ -8,9 +8,12 @@ #include "diagnostics.hpp" -#define warningAt(where, ...) warning(where.src, where.lineNo, __VA_ARGS__) -#define errorAt(where, ...) error(where.src, where.lineNo, __VA_ARGS__) -#define fatalAt(where, ...) fatal(where.src, where.lineNo, __VA_ARGS__) +#define warningAt(where, ...) warning((where).src, (where).lineNo, __VA_ARGS__) +#define errorAt(where, ...) error((where).src, (where).lineNo, __VA_ARGS__) +#define fatalAt(where, ...) fatal((where).src, (where).lineNo, __VA_ARGS__) + +#define fatalTwoAt(where1, where2, ...) \ + fatalTwo(*(where1).src, (where1).lineNo, *(where2).src, (where2).lineNo, __VA_ARGS__) enum WarningLevel { LEVEL_DEFAULT, // Warnings that are enabled by default @@ -46,8 +49,6 @@ void warning(char const *fmt, ...); void error(FileStackNode const *src, uint32_t lineNo, char const *fmt, ...); [[gnu::format(printf, 1, 2)]] void error(char const *fmt, ...); -[[gnu::format(printf, 1, 2)]] -void errorNoDump(char const *fmt, ...); void scriptError(char const *name, uint32_t lineNo, char const *fmt, va_list args); @@ -56,6 +57,16 @@ void fatal(FileStackNode const *src, uint32_t lineNo, char const *fmt, ...); [[gnu::format(printf, 1, 2), noreturn]] void fatal(char const *fmt, ...); +[[gnu::format(printf, 5, 6), noreturn]] +void fatalTwo( + FileStackNode const &src1, + uint32_t lineNo1, + FileStackNode const &src2, + uint32_t lineNo2, + char const *fmt, + ... +); + void requireZeroErrors(); #endif // RGBDS_LINK_WARNING_HPP diff --git a/man/rgbasm.1 b/man/rgbasm.1 index 805a2ca6..b235058f 100644 --- a/man/rgbasm.1 +++ b/man/rgbasm.1 @@ -9,6 +9,7 @@ .Sh SYNOPSIS .Nm .Op Fl EhVvw +.Op Fl B Ar depth .Op Fl b Ar chars .Op Fl \-color Ar when .Op Fl D Ar name Ns Op = Ns Ar value @@ -52,6 +53,15 @@ is invalid because it could also be .Fl \-version . The arguments are as follows: .Bl -tag -width Ds +.It Fl B Ar depth , Fl \-backtrace Ar depth +Specifies the maximum depth for which +.Nm +will print location backtraces for warnings or errors. +Deeper backtraces than that will be abbreviated. +.Fl B Ar 0 +allows unlimited-depth backtraces. +.Fl B Ar collapse +will print the entire location trace on one line. .It Fl b Ar chars , Fl \-binary-digits Ar chars Allow two characters to be used for binary constants in addition to the default .Sq 0 diff --git a/man/rgblink.1 b/man/rgblink.1 index 4c2451ae..b36ad91d 100644 --- a/man/rgblink.1 +++ b/man/rgblink.1 @@ -9,6 +9,7 @@ .Sh SYNOPSIS .Nm .Op Fl dhMtVvwx +.Op Fl B Ar depth .Op Fl \-color Ar when .Op Fl l Ar linker_script .Op Fl m Ar map_file @@ -64,6 +65,15 @@ is invalid because it could also be .Fl \-version . The arguments are as follows: .Bl -tag -width Ds +.It Fl B Ar depth , Fl \-backtrace Ar depth +Specifies the maximum depth for which +.Nm +will print location backtraces for warnings or errors. +Deeper backtraces than that will be abbreviated. +.Fl B Ar 0 +allows unlimited-depth backtraces. +.Fl B Ar collapse +will print the entire location trace on one line. .It Fl \-color Ar when Specify when to highlight warning and error messages with color: .Ql always , diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 3979e90d..272251ca 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -60,6 +60,7 @@ set(rgbasm_src set(rgblink_src "${BISON_LINKER_SCRIPT_PARSER_OUTPUT_SOURCE}" "link/assign.cpp" + "link/fstack.cpp" "link/lexer.cpp" "link/layout.cpp" "link/main.cpp" diff --git a/src/asm/fstack.cpp b/src/asm/fstack.cpp index e3ffa9db..42d9364f 100644 --- a/src/asm/fstack.cpp +++ b/src/asm/fstack.cpp @@ -60,42 +60,79 @@ std::string FileStackNode::reptChain() const { return chain; } -std::string const &FileStackNode::dump(uint32_t curLineNo) const { +std::vector> FileStackNode::backtrace(uint32_t curLineNo) const { if (std::holds_alternative>(data)) { assume(parent); // REPT nodes use their parent's name - std::string const &lastName = parent->dump(lineNo); - style_Set(stderr, STYLE_CYAN, false); - fputs(" -> ", stderr); - style_Set(stderr, STYLE_CYAN, true); - fputs(lastName.c_str(), stderr); - fputs(reptChain().c_str(), stderr); - style_Set(stderr, STYLE_CYAN, false); - fprintf(stderr, "(%" PRIu32 ")", curLineNo); - style_Reset(stderr); - return lastName; + std::vector> nodes = parent->backtrace(lineNo); + assume(!nodes.empty()); + nodes.emplace_back(nodes.back().first + reptChain(), curLineNo); + return nodes; + } else if (parent) { + std::vector> nodes = parent->backtrace(lineNo); + nodes.emplace_back(name(), curLineNo); + return nodes; } else { - if (parent) { - parent->dump(lineNo); - style_Set(stderr, STYLE_CYAN, false); - fputs(" -> ", stderr); - } - std::string const &nodeName = name(); - style_Set(stderr, STYLE_CYAN, true); - fputs(nodeName.c_str(), stderr); - style_Set(stderr, STYLE_CYAN, false); - fprintf(stderr, "(%" PRIu32 ")", curLineNo); - style_Reset(stderr); - return nodeName; + return { + {name(), curLineNo} + }; } } -bool fstk_DumpCurrent() { - if (lexer_AtTopLevel()) { - return false; +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); + } } - assume(!contextStack.empty()); - contextStack.top().fileInfo->dump(lexer_GetLineNo()); - return true; + + style_Reset(stderr); +} + +void fstk_TraceCurrent() { + if (!lexer_AtTopLevel()) { + assume(!contextStack.empty()); + contextStack.top().fileInfo->printBacktrace(lexer_GetLineNo()); + } + lexer_TraceStringExpansions(); } // LCOV_EXCL_START diff --git a/src/asm/lexer.cpp b/src/asm/lexer.cpp index 2b0019f1..2fafe4af 100644 --- a/src/asm/lexer.cpp +++ b/src/asm/lexer.cpp @@ -838,16 +838,16 @@ uint32_t lexer_GetLineNo() { return lexerState->lineNo; } -void lexer_DumpStringExpansions() { +void lexer_TraceStringExpansions() { if (!lexerState) { return; } for (Expansion &exp : lexerState->expansions) { - // Only register EQUS expansions, not string args + // Only print EQUS expansions, not string args if (exp.name) { style_Set(stderr, STYLE_CYAN, false); - fputs("while expanding symbol \"", stderr); + fputs(" while expanding symbol \"", stderr); style_Set(stderr, STYLE_CYAN, true); fputs(exp.name->c_str(), stderr); style_Set(stderr, STYLE_CYAN, false); diff --git a/src/asm/main.cpp b/src/asm/main.cpp index f04e455b..32b7f033 100644 --- a/src/asm/main.cpp +++ b/src/asm/main.cpp @@ -34,7 +34,7 @@ static char const *dependFileName = nullptr; static std::unordered_map> stateFileSpecs; // -s // Short options -static char const *optstring = "b:D:Eg:hI:M:o:P:p:Q:r:s:VvW:wX:"; +static char const *optstring = "B:b:D:Eg:hI:M:o:P:p:Q:r:s:VvW:wX:"; // Variables for the long-only options static int longOpt; // `--color` and variants of `-M` @@ -47,6 +47,7 @@ static int longOpt; // `--color` and variants of `-M` // This is because long opt matching, even to a single char, is prioritized // over short opt matching. static option const longopts[] = { + {"backtrace", required_argument, nullptr, 'B'}, {"binary-digits", required_argument, nullptr, 'b'}, {"define", required_argument, nullptr, 'D'}, {"export-all", no_argument, nullptr, 'E'}, @@ -77,7 +78,7 @@ static option const longopts[] = { static Usage usage = { .name = "rgbasm", .flags = { - "[-EhVvw]", "[-b chars]", "[-D name[=value]]", "[-g chars]", "[-I path]", + "[-EhVvw]", "[-B depth]", "[-b chars]", "[-D name[=value]]", "[-g chars]", "[-I path]", "[-M depend_file]", "[-MC]", "[-MG]", "[-MP]", "[-MT target_file]", "[-MQ target_file]", "[-o out_file]", "[-P include_file]", "[-p pad_value]", "[-Q precision]", "[-r depth]", "[-s features:state_file]", "[-W warning]", "[-X max_errors]", "", @@ -215,7 +216,6 @@ static void verboseOutputConfig(int argc, char *argv[]) { if (options.generatePhonyDeps) { fputs("\tGenerate phony dependencies\n", stderr); } - // [-MG] [-MC] } fputs("Ready.\n", stderr); @@ -303,6 +303,24 @@ 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') { + 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) { opt_B(musl_optarg); diff --git a/src/asm/section.cpp b/src/asm/section.cpp index 537ebd21..4a08b069 100644 --- a/src/asm/section.cpp +++ b/src/asm/section.cpp @@ -281,9 +281,12 @@ static void mergeSections( break; case SECTION_NORMAL: - sectError([&]() { - fputs("Section already defined previously at ", stderr); - sect.src->dump(sect.fileLine); + errorNoTrace([&]() { + fputs("Section already defined\n", stderr); + fstk_TraceCurrent(); + fputs(" and also:\n", stderr); + sect.src->printBacktrace(sect.fileLine); + ++nbSectErrors; }); break; } diff --git a/src/asm/symbol.cpp b/src/asm/symbol.cpp index 140f503b..49205b49 100644 --- a/src/asm/symbol.cpp +++ b/src/asm/symbol.cpp @@ -113,14 +113,15 @@ std::shared_ptr Symbol::getEqus() const { return std::get>(data); } -static void dumpFilename(Symbol const &sym) { - fputs(" at ", stderr); +// Meant to be called last in an `errorNoTrace` callback +static void printBacktraces(Symbol const &sym) { + putc('\n', stderr); + fstk_TraceCurrent(); + fputs(" and also:\n", stderr); if (sym.src) { - sym.src->dump(sym.fileLine); - } else if (sym.isBuiltin) { - fputs("", stderr); + sym.src->printBacktrace(sym.fileLine); } else { - fputs("", stderr); + fprintf(stderr, " at <%s>\n", sym.isBuiltin ? "builtin" : "command-line"); } } @@ -141,37 +142,52 @@ static bool isValidIdentifier(std::string const &s) { } static void alreadyDefinedError(Symbol const &sym, char const *asType) { - if (sym.isBuiltin && !sym_FindScopedValidSymbol(sym.name)) { - // `DEF()` would return false, so we should not claim the symbol is already defined - error("'%s' is reserved for a built-in symbol", sym.name.c_str()); + auto suggestion = [&]() { + std::string s; + if (auto const &contents = sym.type == SYM_EQUS ? sym.getEqus() : nullptr; + contents && isValidIdentifier(*contents)) { + s.append(" (should it be {interpolated} to define its contents \""); + s.append(*contents); + s.append("\"?)"); + } + return s; + }; + + if (sym.isBuiltin) { + if (sym_FindScopedValidSymbol(sym.name)) { + if (std::string s = suggestion(); asType) { + error("'%s' already defined as built-in %s%s", sym.name.c_str(), asType, s.c_str()); + } else { + error("'%s' already defined as built-in%s", sym.name.c_str(), s.c_str()); + } + } else { + // `DEF()` would return false, so we should not claim the symbol is already defined, + // nor suggest to interpolate it + if (asType) { + error("'%s' is reserved for a built-in %s symbol", sym.name.c_str(), asType); + } else { + error("'%s' is reserved for a built-in symbol", sym.name.c_str()); + } + } } else { - error([&]() { + errorNoTrace([&]() { fprintf(stderr, "'%s' already defined", sym.name.c_str()); if (asType) { fprintf(stderr, " as %s", asType); } - dumpFilename(sym); - if (sym.type != SYM_EQUS) { - return; - } - if (std::string const &contents = *sym.getEqus(); isValidIdentifier(contents)) { - fprintf( - stderr, - "\n (should it be {interpolated} to define its contents \"%s\"?)", - contents.c_str() - ); - } + fputs(suggestion().c_str(), stderr); + printBacktraces(sym); }); } } static void redefinedError(Symbol const &sym) { assume(sym.isBuiltin); - if (!sym_FindScopedValidSymbol(sym.name)) { + if (sym_FindScopedValidSymbol(sym.name)) { + error("Built-in symbol '%s' cannot be redefined", sym.name.c_str()); + } else { // `DEF()` would return false, so we should not imply the symbol is already defined error("'%s' is reserved for a built-in symbol", sym.name.c_str()); - } else { - error("Built-in symbol '%s' cannot be redefined", sym.name.c_str()); } } @@ -376,9 +392,9 @@ static Symbol *createNonrelocSymbol(std::string const &symName, bool numeric) { return nullptr; // Don't allow overriding the symbol, that'd be bad! } else if (!numeric) { // The symbol has already been referenced, but it's not allowed - error([&]() { + errorNoTrace([&]() { fprintf(stderr, "'%s' already referenced", symName.c_str()); - dumpFilename(*sym); + printBacktraces(*sym); }); return nullptr; // Don't allow overriding the symbol, that'd be bad! } @@ -444,9 +460,9 @@ Symbol *sym_RedefString(std::string const &symName, std::shared_ptr if (sym->isDefined()) { alreadyDefinedError(*sym, "non-EQUS"); } else { - error([&]() { + errorNoTrace([&]() { fprintf(stderr, "'%s' already referenced", symName.c_str()); - dumpFilename(*sym); + printBacktraces(*sym); }); } return nullptr; diff --git a/src/asm/warning.cpp b/src/asm/warning.cpp index 94cbfb45..725c7e8f 100644 --- a/src/asm/warning.cpp +++ b/src/asm/warning.cpp @@ -60,6 +60,7 @@ Diagnostics warnings = { {WARNING_UNMAPPED_CHAR_1, WARNING_UNMAPPED_CHAR_2, 1}, }, .state = DiagnosticsState(), + .traceDepth = 0, .nbErrors = 0, }; // clang-format on @@ -74,19 +75,16 @@ static void printDiag( ) { style_Set(stderr, color, true); fprintf(stderr, "%s: ", type); - if (fstk_DumpCurrent()) { - putc(':', stderr); - if (flagfmt) { - style_Set(stderr, color, true); - fprintf(stderr, flagfmt, flag); - } - fputs("\n ", stderr); - } style_Reset(stderr); vfprintf(stderr, fmt, args); + if (flagfmt) { + style_Set(stderr, color, true); + putc(' ', stderr); + fprintf(stderr, flagfmt, flag); + } putc('\n', stderr); - lexer_DumpStringExpansions(); + fstk_TraceCurrent(); } static void incrementErrors() { @@ -117,16 +115,11 @@ void error(char const *fmt, ...) { incrementErrors(); } -void error(std::function callback) { +void errorNoTrace(std::function callback) { style_Set(stderr, STYLE_RED, true); fputs("error: ", stderr); - if (fstk_DumpCurrent()) { - fputs(":\n ", stderr); - } style_Reset(stderr); callback(); - putc('\n', stderr); - lexer_DumpStringExpansions(); incrementErrors(); } @@ -167,11 +160,11 @@ void warning(WarningID id, char const *fmt, ...) { break; case WarningBehavior::ENABLED: - printDiag(fmt, args, "warning", STYLE_YELLOW, " [-W%s]", flag); + printDiag(fmt, args, "warning", STYLE_YELLOW, "[-W%s]", flag); break; case WarningBehavior::ERROR: - printDiag(fmt, args, "error", STYLE_RED, " [-Werror=%s]", flag); + printDiag(fmt, args, "error", STYLE_RED, "[-Werror=%s]", flag); break; } diff --git a/src/fix/warning.cpp b/src/fix/warning.cpp index 6ffdeb33..a3714c29 100644 --- a/src/fix/warning.cpp +++ b/src/fix/warning.cpp @@ -19,6 +19,7 @@ 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 3ae0baf2..31ad13a3 100644 --- a/src/gfx/warning.cpp +++ b/src/gfx/warning.cpp @@ -22,6 +22,7 @@ Diagnostics warnings = { }, .paramWarnings = {}, .state = DiagnosticsState(), + .traceDepth = 0, .nbErrors = 0, }; // clang-format on diff --git a/src/link/fstack.cpp b/src/link/fstack.cpp new file mode 100644 index 00000000..c7038928 --- /dev/null +++ b/src/link/fstack.cpp @@ -0,0 +1,80 @@ +#include "link/fstack.hpp" + +#include +#include + +#include "style.hpp" + +#include "link/warning.hpp" + +std::vector> FileStackNode::backtrace(uint32_t curLineNo) const { + if (std::holds_alternative>(data)) { + assume(parent); // REPT nodes use their parent's name + std::vector> nodes = parent->backtrace(lineNo); + assume(!nodes.empty()); + std::string reptChain = nodes.back().first; + for (uint32_t iter : iters()) { + reptChain.append("::REPT~"); + reptChain.append(std::to_string(iter)); + } + nodes.emplace_back(reptChain, curLineNo); + return nodes; + } else if (parent) { + std::vector> nodes = parent->backtrace(lineNo); + nodes.emplace_back(name(), curLineNo); + return nodes; + } else { + return { + {name(), curLineNo} + }; + } +} + +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); +} diff --git a/src/link/main.cpp b/src/link/main.cpp index 5d3945fd..baeea9f3 100644 --- a/src/link/main.cpp +++ b/src/link/main.cpp @@ -36,7 +36,7 @@ Options options; static char const *linkerScriptName = nullptr; // -l // Short options -static char const *optstring = "dhl:m:Mn:O:o:p:S:tVvW:wx"; +static char const *optstring = "B:dhl:m:Mn:O:o:p:S:tVvW:wx"; // Variables for the long-only options static int longOpt; // `--color` @@ -49,6 +49,7 @@ static int longOpt; // `--color` // This is because long opt matching, even to a single char, is prioritized // over short opt matching. static option const longopts[] = { + {"backtrace", required_argument, nullptr, 'B'}, {"dmg", no_argument, nullptr, 'd'}, {"help", no_argument, nullptr, 'h'}, {"linkerscript", required_argument, nullptr, 'l'}, @@ -73,8 +74,8 @@ static option const longopts[] = { static Usage usage = { .name = "rgblink", .flags = { - "[-dhMtVvwx]", "[-l script]", "[-m map_file]", "[-n sym_file]", "[-O overlay_file]", - "[-o out_file]", "[-p pad_value]", "[-S spec]", " ...", + "[-dhMtVvwx]", "[-B depth]", "[-l script]", "[-m map_file]", "[-n sym_file]", + "[-O overlay_file]", "[-o out_file]", "[-p pad_value]", "[-S spec]", " ...", }, .options = { {{"-l", "--linkerscript "}, {"set the input linker script"}}, @@ -180,37 +181,6 @@ static void verboseOutputConfig(int argc, char *argv[]) { } // LCOV_EXCL_STOP -std::string const &FileStackNode::dump(uint32_t curLineNo) const { - if (std::holds_alternative>(data)) { - assume(parent); // REPT nodes use their parent's name - std::string const &lastName = parent->dump(lineNo); - style_Set(stderr, STYLE_CYAN, false); - fputs(" -> ", stderr); - style_Set(stderr, STYLE_CYAN, true); - fputs(lastName.c_str(), stderr); - for (uint32_t iter : iters()) { - fprintf(stderr, "::REPT~%" PRIu32, iter); - } - style_Set(stderr, STYLE_CYAN, false); - fprintf(stderr, "(%" PRIu32 ")", curLineNo); - style_Reset(stderr); - return lastName; - } else { - if (parent) { - parent->dump(lineNo); - style_Set(stderr, STYLE_CYAN, false); - fputs(" -> ", stderr); - } - std::string const &nodeName = name(); - style_Set(stderr, STYLE_CYAN, true); - fputs(nodeName.c_str(), stderr); - style_Set(stderr, STYLE_CYAN, false); - fprintf(stderr, "(%" PRIu32 ")", curLineNo); - style_Reset(stderr); - return nodeName; - } -} - static void parseScrambleSpec(char *spec) { // clang-format off: vertically align nested initializers static UpperMap> scrambleSpecs{ @@ -325,6 +295,21 @@ 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') { + 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/object.cpp b/src/link/object.cpp index dd5dc362..18735623 100644 --- a/src/link/object.cpp +++ b/src/link/object.cpp @@ -21,6 +21,7 @@ #include "version.hpp" #include "link/assign.hpp" +#include "link/fstack.hpp" #include "link/main.hpp" #include "link/patch.hpp" #include "link/sdas_obj.hpp" diff --git a/src/link/patch.cpp b/src/link/patch.cpp index d4bdee45..45115b63 100644 --- a/src/link/patch.cpp +++ b/src/link/patch.cpp @@ -428,7 +428,7 @@ static int32_t computeRPNExpr(Patch const &patch, std::vector const &fil } } else if (Symbol const *symbol = getSymbol(fileSymbols, value); !symbol) { errorAt(patch, "Undefined symbol \"%s\"", fileSymbols[value].name.c_str()); - sym_DumpLocalAliasedSymbols(fileSymbols[value].name); + sym_TraceLocalAliasedSymbols(fileSymbols[value].name); isError = true; } else if (std::holds_alternative