diff --git a/BROKEN_LINK_FIX.md b/BROKEN_LINK_FIX.md new file mode 100644 index 000000000..cc5872f0a --- /dev/null +++ b/BROKEN_LINK_FIX.md @@ -0,0 +1,42 @@ +# Fix design: Broken link (dangling VarScope) in min_uaf_repro_real + +## Symptom +Running: + +``` +./bin/verilator --cc --timing --debug testcase/min_uaf_repro_real.sv +``` + +fires: + +``` +Broken link in node ... 'm_varp && !m_varp->brokeExists()' @ ... AstVarScope +node: VARSCOPE ... -> VAR __Vtask_wrap__1__Vfuncout [FUNC] BLOCKTEMP +``` + +## Root cause +`AstVarScope` is a cross-link node stored under `AstScope::varsp()`. +In this testcase, `V3Task` creates statement-expression wrappers (`AstExprStmt`) that include a +temporary variable declaration (e.g. `__Vtask_wrap__1__Vfuncout [FUNC] BLOCKTEMP`) in the +`AstExprStmt::stmtsp()` list. + +Later, `V3Const` simplifies certain `AstExprStmt` nodes by replacing the whole ExprStmt with its +`resultp()` and deleting the ExprStmt node. That deletes the `stmtsp()` subtree (including the +temporary `AstVar`), but the corresponding `AstVarScope` under the enclosing `AstScope` is not +removed. + +This leaves a dangling `AstVarScope::m_varp` pointer and `V3Broken` correctly detects it. + +## Fix +`V3Broken` only considers nodes reachable via the structural `op1-op4/nextp` AST links as +"linkable" targets. However, Verilator also has *member-pointer cross-links* (via `foreachLink`), +notably `AstVarScope::m_varp`, which can refer to valid nodes that are not necessarily reachable via +structural links. + +Update `V3Broken::brokenAll()` so that when building the linkable set it also adds the targets of +`foreachLink` for every node in the main tree. This allows `brokeExists()` checks in `brokenGen()` +(e.g. `AstVarScope::brokenGen`) to succeed for valid cross-linked nodes. + +## Validation +Rebuild `bin/verilator_bin_dbg` and re-run the testcase; the internal broken-link assert should no +longer occur. diff --git a/src/V3Broken.cpp b/src/V3Broken.cpp index 355ac9e29..06fee3332 100644 --- a/src/V3Broken.cpp +++ b/src/V3Broken.cpp @@ -388,6 +388,11 @@ void V3Broken::brokenAll(AstNetlist* nodep) { UASSERT_OBJ(nodep->brokenState() != brokenCntCurrent, nodep, "AstNode is already in tree at another location"); if (nodep->maybePointedTo()) s_linkableTable.addLinkable(nodep); + // Some cross-links point at nodes not reachable via op1-op4/nextp. + // AstVarScope::m_varp is one such link; ensure targets are considered linkable. + if (AstVarScope* const vscp = VN_CAST(nodep, VarScope)) { + if (AstNode* const varp = vscp->varp()) s_linkableTable.addLinkable(varp); + } nodep->brokenState(brokenCntCurrent); });