Skip to content

Commit

Permalink
fix: crash when passing NimNode to static parameter (#1449)
Browse files Browse the repository at this point in the history
## Summary

* fix a compiler crash when passing a NimNode to a `static` parameter
* fix a compiler crash when evaluating a `NimNode`-returning constant
  expression

## Details

There were two problems:
* `nkNimNodeLit` wasn't handled in constant data expression by `mirgen`
* `NimNode` values returned directly from a VM invocation weren't
  wrapped in `nkNimNodeLit` trees

For handling `NimNode` values in `vm.regToNode`, the existing
deserialization logic for `NimNode` values in `vmcompilerserdes` is
moved to a separate procedure, so that it can be used by `regToNode`.

The `opcRepr` implementation relied on `regToNode` always returning
an unwrapped `PNode` -- it is adjusted to manually handle `NimNode`
values.

A test covering both issues is added.

Fixes #1448.
  • Loading branch information
zerbina authored Sep 5, 2024
1 parent 64aa3b5 commit 3955af7
Show file tree
Hide file tree
Showing 4 changed files with 38 additions and 12 deletions.
2 changes: 2 additions & 0 deletions compiler/mir/mirgen.nim
Original file line number Diff line number Diff line change
Expand Up @@ -2490,6 +2490,8 @@ proc constDataToMir*(env: var MirEnv, n: PNode): MirTree =
bu.use toFloatLiteral(env, n)
of nkStrLiterals:
bu.use strLiteral(env, n.strVal, typ)
of nkNimNodeLit:
bu.use astLiteral(env, n[0], n.typ)
of nkHiddenStdConv, nkHiddenSubConv:
# doesn't translate to a MIR node itself, but the type overrides
# that of the sub-expression
Expand Down
15 changes: 11 additions & 4 deletions compiler/vm/vm.nim
Original file line number Diff line number Diff line change
Expand Up @@ -500,7 +500,11 @@ proc regToNode*(c: TCtx, x: TFullReg; typ: PType, info: TLineInfo): PNode =
# TODO: validate the address
result = c.deserialize(c.allocator.makeLocHandle(x.addrVal, x.addrTyp), typ, info)
of rkHandle, rkLocation: result = c.deserialize(x.handle, typ, info)
of rkNimNode: result = x.nimNode
of rkNimNode:
if typ.sym != nil and typ.sym.magic == mPNimrodNode:
result = c.deserializeNimNode(x.nimNode, typ, info)
else:
result = x.nimNode

# ---- exception handling ----

Expand Down Expand Up @@ -2245,7 +2249,6 @@ proc rawExecute(c: var TCtx, t: var VmThread, pc: var int): YieldReason =

of opcRepr:
# Turn the provided value into its string representation. Used for:
# - implementing the general ``repr`` when not using the ``repr`` v2
# - rendering an AST to its text representation (``repr`` for
# ``NimNode``)
# - rendering the discriminant value for a ``FieldDefect``'s message
Expand All @@ -2261,9 +2264,13 @@ proc rawExecute(c: var TCtx, t: var VmThread, pc: var int): YieldReason =

checkHandle(regs[rb])

let str = renderTree(c.regToNode(regs[rb], typ.nimType, c.debug[pc]),
{renderNoComments, renderDocComments})
let n =
if typ.nimType.sym != nil and typ.nimType.sym.magic == mPNimrodNode:
regs[rb].nimNode
else:
c.regToNode(regs[rb], typ.nimType, c.debug[pc])

let str = renderTree(n, {renderNoComments, renderDocComments})
regs[ra].strVal.newVmString(str, c.allocator)
of opcQuit:
return YieldReason(kind: yrkQuit, exitCode: regs[ra].intVal.int)
Expand Down
19 changes: 11 additions & 8 deletions compiler/vm/vmcompilerserdes.nim
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,16 @@ proc deserializeArray*(
if hasError:
result = c.config.wrapError(result)

proc deserializeNimNode*(c: TCtx, n: PNode, formal: PType,
info: TLineInfo): PNode =
## Makes sure `n` (which represent a NimNode value) is valid and turns it
## into a proper NimNode literal.
if cyclicTree(n):
result = c.config.newError(
newNodeIT(nkEmpty, info, formal),
PAstDiag(kind: adCyclicTree, cyclic: n))
else:
result = newTreeIT(nkNimNodeLit, info, formal): n

proc deserialize(c: TCtx, m: VmMemoryRegion, vt: PVmType, formal, t: PType, info: TLineInfo): PNode =
let
Expand Down Expand Up @@ -270,14 +280,7 @@ proc deserialize(c: TCtx, m: VmMemoryRegion, vt: PVmType, formal, t: PType, info
result = deserializeRef(c, atom.refVal, vt.targetType, formal, t, info)
else:
assert vt.kind == akPNode

if unlikely(cyclicTree(atom.nodeVal)):
result = c.config.newError(
wrongNode(),
PAstDiag(kind: adCyclicTree, cyclic: atom.nodeVal))
else:
# XXX: not doing a full tree-copy here might lead to issues
result = newTreeIT(nkNimNodeLit, info, formal): atom.nodeVal
result = deserializeNimNode(c, atom.nodeVal, formal, info)

of tyProc:
case t.callConv
Expand Down
14 changes: 14 additions & 0 deletions tests/lang_callable/macros/tstatic_nim_node_parameter.nim
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
discard """
description: '''
Ensure that a NimNode can be passed as a static parameter to a macro.
'''
action: compile
"""

import std/macros

macro m(n: static NimNode) =
doAssert n.kind == nnkStmtList
doAssert n.len == 0

m(newStmtList())

0 comments on commit 3955af7

Please sign in to comment.