From 80335bbe3afd79e4b1a02bb41a33acd295163e48 Mon Sep 17 00:00:00 2001 From: Rangi Date: Fri, 5 Dec 2025 15:23:42 -0500 Subject: [PATCH 1/2] Refactor `FileStackNode::printBacktrace` from recursive to iterative This avoids a potential stack overflow for very long backtraces, or for corrupt object files with cyclic backtraces --- src/asm/fstack.cpp | 59 ++++++++++++++++++++++++--------------------- src/link/fstack.cpp | 59 ++++++++++++++++++++++++--------------------- 2 files changed, 62 insertions(+), 56 deletions(-) diff --git a/src/asm/fstack.cpp b/src/asm/fstack.cpp index 6707a0ca3..2a438a7ed 100644 --- a/src/asm/fstack.cpp +++ b/src/asm/fstack.cpp @@ -57,44 +57,47 @@ static std::vector includePaths = {""}; // -I static std::deque preIncludeNames; // -P static bool failedOnMissingInclude = false; -using TraceNode = std::pair; - -static std::vector backtrace(FileStackNode const &node, uint32_t curLineNo) { - if (node.isQuiet && !tracing.loud) { - if (node.parent) { +void FileStackNode::printBacktrace(uint32_t curLineNo) const { + using TraceItem = std::pair; + std::vector items; + for (TraceItem item{this, curLineNo};;) { + auto &[node, itemLineNo] = item; + bool loud = !node->isQuiet || tracing.loud; + if (loud) { + items.emplace_back(node, itemLineNo); + } + if (!node->parent) { + assume(node->type != NODE_REPT && std::holds_alternative(node->data)); + break; + } + if (loud || node->type != NODE_REPT) { // Quiet REPT nodes will pass their interior line number up to their parent, // which is more precise than the parent's own line number (since that will be // the line number of the "REPT?" or "FOR?" itself). - return backtrace(*node.parent, node.type == NODE_REPT ? curLineNo : node.lineNo); + itemLineNo = node->lineNo; } - return {}; // LCOV_EXCL_LINE - } - - if (!node.parent) { - assume(node.type != NODE_REPT && std::holds_alternative(node.data)); - return { - {node.name(), curLineNo} - }; + node = &*node->parent; } - std::vector traceNodes = backtrace(*node.parent, node.lineNo); - if (std::holds_alternative>(node.data)) { - assume(!traceNodes.empty()); // REPT nodes use their parent's name - std::string reptName = traceNodes.back().first; - if (std::vector const &nodeIters = node.iters(); !nodeIters.empty()) { - reptName.append(NODE_SEPARATOR REPT_NODE_PREFIX); - reptName.append(std::to_string(nodeIters.front())); + using TraceNode = std::pair; + std::vector traceNodes; + traceNodes.reserve(items.size()); + for (auto &[node, itemLineNo] : reversed(items)) { + if (std::holds_alternative>(node->data)) { + assume(!traceNodes.empty()); // REPT nodes use their parent's name + std::string reptName = traceNodes.back().first; + if (std::vector const &nodeIters = node->iters(); !nodeIters.empty()) { + reptName.append(NODE_SEPARATOR REPT_NODE_PREFIX); + reptName.append(std::to_string(nodeIters.front())); + } + traceNodes.emplace_back(reptName, itemLineNo); + } else { + traceNodes.emplace_back(node->name(), itemLineNo); } - traceNodes.emplace_back(reptName, curLineNo); - } else { - traceNodes.emplace_back(node.name(), curLineNo); } - return traceNodes; -} -void FileStackNode::printBacktrace(uint32_t curLineNo) const { trace_PrintBacktrace( - backtrace(*this, curLineNo), + traceNodes, [](TraceNode const &node) { return node.first.c_str(); }, [](TraceNode const &node) { return node.second; } ); diff --git a/src/link/fstack.cpp b/src/link/fstack.cpp index 50283adb6..b8850a191 100644 --- a/src/link/fstack.cpp +++ b/src/link/fstack.cpp @@ -12,44 +12,47 @@ #include "link/warning.hpp" -using TraceNode = std::pair; - -static std::vector backtrace(FileStackNode const &node, uint32_t curLineNo) { - if (node.isQuiet && !tracing.loud) { - if (node.parent) { +void FileStackNode::printBacktrace(uint32_t curLineNo) const { + using TraceItem = std::pair; + std::vector items; + for (TraceItem item{this, curLineNo};;) { + auto &[node, itemLineNo] = item; + bool loud = !node->isQuiet || tracing.loud; + if (loud) { + items.emplace_back(node, itemLineNo); + } + if (!node->parent) { + assume(node->type != NODE_REPT && std::holds_alternative(node->data)); + break; + } + if (loud || node->type != NODE_REPT) { // Quiet REPT nodes will pass their interior line number up to their parent, // which is more precise than the parent's own line number (since that will be // the line number of the "REPT?" or "FOR?" itself). - return backtrace(*node.parent, node.type == NODE_REPT ? curLineNo : node.lineNo); + itemLineNo = node->lineNo; } - return {}; // LCOV_EXCL_LINE + node = &*node->parent; } - if (!node.parent) { - assume(node.type != NODE_REPT && std::holds_alternative(node.data)); - return { - {node.name(), curLineNo} - }; - } - - std::vector traceNodes = backtrace(*node.parent, node.lineNo); - if (std::holds_alternative>(node.data)) { - assume(!traceNodes.empty()); // REPT nodes use their parent's name - std::string reptName = traceNodes.back().first; - if (std::vector const &nodeIters = node.iters(); !nodeIters.empty()) { - reptName.append(NODE_SEPARATOR REPT_NODE_PREFIX); - reptName.append(std::to_string(nodeIters.back())); + using TraceNode = std::pair; + std::vector traceNodes; + traceNodes.reserve(items.size()); + for (auto &[node, itemLineNo] : reversed(items)) { + if (std::holds_alternative>(node->data)) { + assume(!traceNodes.empty()); // REPT nodes use their parent's name + std::string reptName = traceNodes.back().first; + if (std::vector const &nodeIters = node->iters(); !nodeIters.empty()) { + reptName.append(NODE_SEPARATOR REPT_NODE_PREFIX); + reptName.append(std::to_string(nodeIters.back())); + } + traceNodes.emplace_back(reptName, itemLineNo); + } else { + traceNodes.emplace_back(node->name(), itemLineNo); } - traceNodes.emplace_back(reptName, curLineNo); - } else { - traceNodes.emplace_back(node.name(), curLineNo); } - return traceNodes; -} -void FileStackNode::printBacktrace(uint32_t curLineNo) const { trace_PrintBacktrace( - backtrace(*this, curLineNo), + traceNodes, [](TraceNode const &node) { return node.first.c_str(); }, [](TraceNode const &node) { return node.second; } ); From e6f1f9ed1fd2d1b6c5a47d8d289f36882b8d0721 Mon Sep 17 00:00:00 2001 From: Rangi Date: Fri, 5 Dec 2025 15:35:44 -0500 Subject: [PATCH 2/2] Add missing `SPDX-License-Identifier: MIT` comments --- src/cli.cpp | 2 ++ src/link/fstack.cpp | 2 ++ test/gfx/randtilegen.cpp | 2 +- test/gfx/rgbgfx_test.cpp | 2 +- 4 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/cli.cpp b/src/cli.cpp index 4e8538b85..61634e139 100644 --- a/src/cli.cpp +++ b/src/cli.cpp @@ -1,3 +1,5 @@ +// SPDX-License-Identifier: MIT + #include "cli.hpp" #include diff --git a/src/link/fstack.cpp b/src/link/fstack.cpp index b8850a191..58b6e0c39 100644 --- a/src/link/fstack.cpp +++ b/src/link/fstack.cpp @@ -1,3 +1,5 @@ +// SPDX-License-Identifier: MIT + #include "link/fstack.hpp" #include diff --git a/test/gfx/randtilegen.cpp b/test/gfx/randtilegen.cpp index 089de8241..7568d00f0 100644 --- a/test/gfx/randtilegen.cpp +++ b/test/gfx/randtilegen.cpp @@ -1,4 +1,4 @@ -/* SPDX-License-Identifier: MIT */ +// SPDX-License-Identifier: MIT /* * Originally: diff --git a/test/gfx/rgbgfx_test.cpp b/test/gfx/rgbgfx_test.cpp index 0b88ca232..f1f1a1779 100644 --- a/test/gfx/rgbgfx_test.cpp +++ b/test/gfx/rgbgfx_test.cpp @@ -1,4 +1,4 @@ -/* SPDX-License-Identifier: MIT */ +// SPDX-License-Identifier: MIT #include #include