Skip to content

Commit

Permalink
fix(js): wrong code for uninitialized pointer locals (#1011)
Browse files Browse the repository at this point in the history
## Summary

Fix wrong code being generated for uninitialized locals of `pointer`
type, which resulted in run-time crashes or other unexpected
behaviour.

## Details

* the set used in deciding whether to use a boxed pointer missed
  `tyPointer`. Since testing the set after the inclusion of `tyPointer`
  would be redundant with the `mapType` check, testing the set is
  simply removed
* the `not isIndirect` check is replaced with testing whether a boxed
  pointer is used. While a loc using an indirection implies a boxed
  representation, the extra indirection made the logic harder to
  understand
  • Loading branch information
zerbina committed Nov 4, 2023
1 parent 72fe3fc commit 4c0cd14
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 9 deletions.
14 changes: 5 additions & 9 deletions compiler/backend/jsgen.nim
Original file line number Diff line number Diff line change
Expand Up @@ -1673,13 +1673,12 @@ proc genVarInit(p: PProc, typ: PType, varName: string, storage: StorageFlags,
s: Rope

if n.kind == cnkEmpty:
let isIndirect = stfIndirect in storage
if not isIndirect and
typ.kind in {tyVar, tyPtr, tyLent, tyRef} and mapType(typ) == etyBaseIndex:
if mapType(typ) == etyBaseIndex and (stfBoxed notin storage):
lineF(p, "var $1 = null;$n", [varName])
lineF(p, "var $1_Idx = 0;$n", [varName])
else:
lineF(p, "var $1 = $2;$n", [varName, createVar(p, typ, isIndirect)])
let val = createVar(p, typ, stfIndirect in storage)
lineF(p, "var $1 = $2;$n", [varName, val])
else:
gen(p, n, a)
case mapType(typ)
Expand Down Expand Up @@ -2293,18 +2292,15 @@ proc finishProc*(p: PProc): string =
let
loc {.cursor.} = p.locals[resultId]
mname = loc.name
let returnAddress = not isIndirect(loc) and
loc.typ.kind in {tyVar, tyPtr, tyLent, tyRef} and
mapType(loc.typ) == etyBaseIndex
if returnAddress:
if mapType(loc.typ) == etyBaseIndex and (stfBoxed notin loc.storage):
resultAsgn = p.indentLine(("var $# = null;$n") % [mname])
resultAsgn.add p.indentLine("var $#_Idx = 0;$n" % [mname])
else:
let resVar = createVar(p, loc.typ, isIndirect(loc))
resultAsgn = p.indentLine(("var $# = $#;$n") % [mname, resVar])
var a: TCompRes
accessLoc(mname, loc, a)
if returnAddress:
if a.typ == etyBaseIndex:
returnStmt = "return [$#, $#];$n" % [a.address, a.res]
else:
returnStmt = "return $#;$n" % [a.res]
Expand Down
13 changes: 13 additions & 0 deletions tests/js/tdefault_init_pointer_var.nim
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
discard """
description: '''
Regression test for wrong code being generated for locals of `pointer`
type
'''
"""

proc test(): pointer =
# the result variable was affected too
var x: pointer
doAssert x == nil # this assertion failed

doAssert test() == nil # this one too

0 comments on commit 4c0cd14

Please sign in to comment.