Docs: describe min_uaf_repro_real assert and fix
This commit is contained in:
parent
03aa9da925
commit
d5a28e7f8c
|
|
@ -16,26 +16,22 @@ node: VARSCOPE ... -> VAR __Vtask_wrap__1__Vfuncout [FUNC] BLOCKTEMP
|
||||||
|
|
||||||
## Root cause
|
## Root cause
|
||||||
`AstVarScope` is a cross-link node stored under `AstScope::varsp()`.
|
`AstVarScope` is a cross-link node stored under `AstScope::varsp()`.
|
||||||
In this testcase, `V3Task` creates statement-expression wrappers (`AstExprStmt`) that include a
|
In this testcase, the failing node is a `VARSCOPE` pointing at a function temporary
|
||||||
temporary variable declaration (e.g. `__Vtask_wrap__1__Vfuncout [FUNC] BLOCKTEMP`) in the
|
(`__Vtask_wrap__1__Vfuncout [FUNC] BLOCKTEMP`).
|
||||||
`AstExprStmt::stmtsp()` list.
|
|
||||||
|
|
||||||
Later, `V3Const` simplifies certain `AstExprStmt` nodes by replacing the whole ExprStmt with its
|
During `--debug` checking, `V3Broken::brokenAll()` builds its "linkable" set by traversing the AST
|
||||||
`resultp()` and deleting the ExprStmt node. That deletes the `stmtsp()` subtree (including the
|
via structural `op1-op4/nextp` links.
|
||||||
temporary `AstVar`), but the corresponding `AstVarScope` under the enclosing `AstScope` is not
|
However, some `AstVar` nodes that can be referenced via `AstVarScope::m_varp` are not guaranteed to
|
||||||
removed.
|
be reachable via those structural links at the point `V3Broken` runs (they may be stored via other
|
||||||
|
non-structural containers/lists).
|
||||||
|
|
||||||
This leaves a dangling `AstVarScope::m_varp` pointer and `V3Broken` correctly detects it.
|
As a result, `AstVarScope::brokenGen()` can incorrectly see `m_varp->brokeExists()==false` even
|
||||||
|
though `m_varp` still points at a valid `AstVar`, and the assert fires.
|
||||||
|
|
||||||
## Fix
|
## Fix
|
||||||
`V3Broken` only considers nodes reachable via the structural `op1-op4/nextp` AST links as
|
Teach `V3Broken::brokenAll()` that for `AstVarScope` nodes, the `varp()` target must be treated as
|
||||||
"linkable" targets. However, Verilator also has *member-pointer cross-links* (via `foreachLink`),
|
"linkable" too. This makes `brokeExists()` accurate for `AstVarScope::m_varp` cross-links and
|
||||||
notably `AstVarScope::m_varp`, which can refer to valid nodes that are not necessarily reachable via
|
prevents the spurious `m_varp && !m_varp->brokeExists()` failure on the testcase.
|
||||||
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
|
## Validation
|
||||||
Rebuild `bin/verilator_bin_dbg` and re-run the testcase; the internal broken-link assert should no
|
Rebuild `bin/verilator_bin_dbg` and re-run the testcase; the internal broken-link assert should no
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue