From 2c5c453ab8cc13a3ea392d01d50239e65e772f8b Mon Sep 17 00:00:00 2001 From: Rangi Date: Fri, 5 Dec 2025 15:23:42 -0500 Subject: [PATCH] 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 6707a0ca..2a438a7e 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 + 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.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 50283adb..b8850a19 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; } );