From f3f83e71d6bd5a57e7eec04d3de6579370d132fd Mon Sep 17 00:00:00 2001 From: zerbina <100542850+zerbina@users.noreply.github.com> Date: Wed, 13 Mar 2024 00:46:04 +0100 Subject: [PATCH] macros: disallow creating special nodes manually (#1235) ## Summary Attempting to manually create a node (via `newNimNode`) of kind `nnkError`, `nnkIdent`, `nnkSym`, or `nnkType` now results in a compiler error. Creating them was previously allowed, but the compiler could crash when such incomplete nodes appear in macro output. ## Details * all node kinds where the initial state is not valid (`nkError`, `nkIdent`, `nkSym`, and `nkType`) are forbidden * the `opcNNewNimNode` implementation guards against the node kinds * a VM event, diagnostic, and report is added for the error * to not introduce a new variant, the `msg` variant is re-used for the new VM event * the untyped `nnkType` test case is removed from `tempty_type_nodes`; macros cannot create untyped `nnkType` nodes anymore --- compiler/ast/ast_types.nim | 4 ++- compiler/ast/report_enums.nim | 1 + compiler/front/cli_reporter.nim | 6 ++++- compiler/front/msgs.nim | 1 + compiler/vm/vm.nim | 19 ++++++++++---- compiler/vm/vmdef.nim | 4 ++- compiler/vm/vmrunner.nim | 1 + .../macros/tdisallow_create_special_nodes.nim | 26 +++++++++++++++++++ .../macros/tempty_type_nodes.nim | 10 +------ 9 files changed, 55 insertions(+), 17 deletions(-) create mode 100644 tests/lang_callable/macros/tdisallow_create_special_nodes.nim diff --git a/compiler/ast/ast_types.nim b/compiler/ast/ast_types.nim index c5b048d0bb77..e4fc830088c8 100644 --- a/compiler/ast/ast_types.nim +++ b/compiler/ast/ast_types.nim @@ -987,6 +987,7 @@ type adVmFieldNotFound adVmNotAField adVmFieldUnavailable + adVmCannotCreateNode adVmCannotSetChild adVmCannotAddChild adVmCannotGetChild @@ -1009,7 +1010,8 @@ type indexSpec*: tuple[usedIdx, minIdx, maxIdx: Int128] of adVmErrInternal, adVmNilAccess, adVmIllegalConv, adVmFieldUnavailable, adVmFieldNotFound, - adVmCacheKeyAlreadyExists, adVmMissingCacheKey: + adVmCacheKeyAlreadyExists, adVmMissingCacheKey, + adVmCannotCreateNode: msg*: string of adVmCannotSetChild, adVmCannotAddChild, adVmCannotGetChild, adVmUnhandledException, adVmNoType, adVmNodeNotASymbol: diff --git a/compiler/ast/report_enums.nim b/compiler/ast/report_enums.nim index 78a92bcdfd01..d4da99e7f74b 100644 --- a/compiler/ast/report_enums.nim +++ b/compiler/ast/report_enums.nim @@ -247,6 +247,7 @@ type rvmCacheKeyAlreadyExists rvmFieldNotFound rvmFieldInavailable + rvmCannotCreateNode rvmCannotSetChild rvmCannotAddChild rvmCannotGetChild diff --git a/compiler/front/cli_reporter.nim b/compiler/front/cli_reporter.nim index cf89556fe886..cee55b31a275 100644 --- a/compiler/front/cli_reporter.nim +++ b/compiler/front/cli_reporter.nim @@ -2949,6 +2949,9 @@ proc reportBody*(conf: ConfigRef, r: VMReport): string = of rvmFieldNotFound: result = "node lacks field: " & r.str + of rvmCannotCreateNode: + result = "cannot manually create a node of kind: n" & r.str + of rvmCannotSetChild: result = "cannot set child of node kind: n" & $r.ast.kind @@ -3867,7 +3870,8 @@ func astDiagToLegacyReport(conf: ConfigRef, diag: PAstDiag): Report {.inline.} = location: some location, reportInst: diag.instLoc.toReportLineInfo) of rvmErrInternal, rvmNilAccess, rvmIllegalConv, rvmFieldInavailable, - rvmFieldNotFound, rvmCacheKeyAlreadyExists, rvmMissingCacheKey: + rvmFieldNotFound, rvmCacheKeyAlreadyExists, rvmMissingCacheKey, + rvmCannotCreateNode: vmRep = VMReport( kind: kind, str: diag.vmErr.msg, diff --git a/compiler/front/msgs.nim b/compiler/front/msgs.nim index 664906ce4ad1..37dd2a33572e 100644 --- a/compiler/front/msgs.nim +++ b/compiler/front/msgs.nim @@ -391,6 +391,7 @@ func astDiagVmToLegacyReportKind*( of adVmFieldNotFound: rvmFieldNotFound of adVmNotAField: rvmNotAField of adVmFieldUnavailable: rvmFieldInavailable + of adVmCannotCreateNode: rvmCannotCreateNode of adVmCannotSetChild: rvmCannotSetChild of adVmCannotAddChild: rvmCannotAddChild of adVmCannotGetChild: rvmCannotGetChild diff --git a/compiler/vm/vm.nim b/compiler/vm/vm.nim index 8ec5a6b354bd..f36289117fa1 100644 --- a/compiler/vm/vm.nim +++ b/compiler/vm/vm.nim @@ -2752,13 +2752,21 @@ proc rawExecute(c: var TCtx, t: var VmThread, pc: var int): YieldReason = raiseVmError(VmEvent(kind: vmEvtFieldNotFound, msg: "strVal")) of opcNNewNimNode: decodeBC(rkNimNode) - var k = regs[rb].intVal + let k = regs[rb].intVal guestValidate(k in 0..ord(high(TNodeKind)), "request to create a NimNode of invalid kind") + let kind = TNodeKind(int(k)) + case kind + of nkError, nkIdent, nkSym, nkType: + # nodes that cannot be created manually + raiseVmError(VmEvent(kind: vmEvtCannotCreateNode, msg: $kind)) + of nkWithSons, nkLiterals, nkCommentStmt, nkEmpty: + discard "the uninitialized state is valid" + let cc = regs[rc].nimNode - let x = newNodeI(TNodeKind(int(k)), + let x = newNodeI(kind, if cc.kind != nkNilLit: cc.info elif c.comesFromHeuristic.line != 0'u16: @@ -2767,8 +2775,7 @@ proc rawExecute(c: var TCtx, t: var VmThread, pc: var int): YieldReason = c.callsite[1].info else: c.debug[pc]) - # prevent crashes in the compiler resulting from wrong macros: - if x.kind == nkIdent: x.ident = c.cache.emptyIdent + regs[ra].nimNode = x of opcNCopyNimNode: decodeB(rkNimNode) @@ -3017,6 +3024,7 @@ func vmEventToAstDiagVmError*(evt: VmEvent): AstDiagVmError {.inline.} = of vmEvtFieldNotFound: adVmFieldNotFound of vmEvtNotAField: adVmNotAField of vmEvtFieldUnavailable: adVmFieldUnavailable + of vmEvtCannotCreateNode: adVmCannotCreateNode of vmEvtCannotSetChild: adVmCannotSetChild of vmEvtCannotAddChild: adVmCannotAddChild of vmEvtCannotGetChild: adVmCannotGetChild @@ -3048,7 +3056,8 @@ func vmEventToAstDiagVmError*(evt: VmEvent): AstDiagVmError {.inline.} = indexSpec: evt.indexSpec) of adVmErrInternal, adVmNilAccess, adVmIllegalConv, adVmFieldUnavailable, adVmFieldNotFound, - adVmCacheKeyAlreadyExists, adVmMissingCacheKey: + adVmCacheKeyAlreadyExists, adVmMissingCacheKey, + adVmCannotCreateNode: AstDiagVmError( kind: kind, msg: evt.msg) diff --git a/compiler/vm/vmdef.nim b/compiler/vm/vmdef.nim index c0b5653ebdc9..7eb9b9935c97 100644 --- a/compiler/vm/vmdef.nim +++ b/compiler/vm/vmdef.nim @@ -598,6 +598,7 @@ type vmEvtFieldNotFound vmEvtNotAField vmEvtFieldUnavailable + vmEvtCannotCreateNode vmEvtCannotSetChild vmEvtCannotAddChild vmEvtCannotGetChild @@ -623,7 +624,8 @@ type indexSpec*: tuple[usedIdx, minIdx, maxIdx: Int128] of vmEvtErrInternal, vmEvtNilAccess, vmEvtIllegalConv, vmEvtFieldUnavailable, vmEvtFieldNotFound, - vmEvtCacheKeyAlreadyExists, vmEvtMissingCacheKey: + vmEvtCacheKeyAlreadyExists, vmEvtMissingCacheKey, + vmEvtCannotCreateNode: msg*: string of vmEvtCannotSetChild, vmEvtCannotAddChild, vmEvtCannotGetChild, vmEvtNoType, vmEvtNodeNotASymbol: diff --git a/compiler/vm/vmrunner.nim b/compiler/vm/vmrunner.nim index a2f0e2e66787..2d097ec5f4c3 100644 --- a/compiler/vm/vmrunner.nim +++ b/compiler/vm/vmrunner.nim @@ -273,6 +273,7 @@ func vmEventToLegacyReportKind(evt: VmEventKind): ReportKind {.inline.} = of vmEvtFieldNotFound: rvmFieldNotFound of vmEvtNotAField: rvmNotAField of vmEvtFieldUnavailable: rvmFieldInavailable + of vmEvtCannotCreateNode: rvmCannotCreateNode of vmEvtCannotSetChild: rvmCannotSetChild of vmEvtCannotAddChild: rvmCannotAddChild of vmEvtCannotGetChild: rvmCannotGetChild diff --git a/tests/lang_callable/macros/tdisallow_create_special_nodes.nim b/tests/lang_callable/macros/tdisallow_create_special_nodes.nim new file mode 100644 index 000000000000..4f138f39dfb5 --- /dev/null +++ b/tests/lang_callable/macros/tdisallow_create_special_nodes.nim @@ -0,0 +1,26 @@ +discard """ + description: ''' + Ensure that an error is reported when attempting to create special + atom nodes with `newNimNode` + ''' + matrix: "--errorMax:4" + action: reject +""" + +import std/macros + +static: + discard newNimNode(nnkError) #[tt.Error + ^ cannot manually create a node of kind: nnkError]# + +static: + discard newNimNode(nnkIdent) #[tt.Error + ^ cannot manually create a node of kind: nnkIdent]# + +static: + discard newNimNode(nnkSym) #[tt.Error + ^ cannot manually create a node of kind: nnkSym]# + +static: + discard newNimNode(nnkType) #[tt.Error + ^ cannot manually create a node of kind: nnkType]# diff --git a/tests/lang_callable/macros/tempty_type_nodes.nim b/tests/lang_callable/macros/tempty_type_nodes.nim index 6ab3d3fc827b..1dff4eec2428 100644 --- a/tests/lang_callable/macros/tempty_type_nodes.nim +++ b/tests/lang_callable/macros/tempty_type_nodes.nim @@ -1,6 +1,6 @@ discard """ description: ''' - Ensure that untyped `nnkEmpty` and `nnkType` nodes used in type positions + Ensure that untyped `nnkEmpty` nodes used in type positions result in a proper error ''' target: native @@ -17,11 +17,3 @@ macro test1(): untyped = newNimNode(nnkEmpty) test1() - -macro test2(): untyped = - # test case #2: untyped ``nnkType`` node in type position - nnkConv.newTree(newNimNode(nnkType)): #[tt.Error - ^ type expected, but expression has no type]# - newNimNode(nnkEmpty) - -test2()