summary refs log tree commit diff stats
diff options
context:
space:
mode:
authorringabout <43030857+ringabout@users.noreply.github.com>2024-02-01 23:51:07 +0800
committerGitHub <noreply@github.com>2024-02-01 16:51:07 +0100
commit7d9721007c8c82804450bd32bbf3bbaf806a52f2 (patch)
tree87d385878e2fb5a4b78d2b3390b1289a3d9874f5
parent519d976f6241023b05b06188e6d96245d0a6a2fe (diff)
downloadNim-7d9721007c8c82804450bd32bbf3bbaf806a52f2.tar.gz
fixes regression #22909; don't optimize result init if statements can raise which corrupts the compiler (#23271)
fixes #22909
required by https://github.com/nim-lang/Nim/pull/23267

```nim
proc foo: string =
  assert false
  result = ""
```

In the function `foo`, `assert false` raises an exception, which can
cause `result` to be uninitialized if the default result initialization
is optimized out
-rw-r--r--compiler/cgen.nim28
-rw-r--r--tests/arc/tarc_macro.nim13
2 files changed, 31 insertions, 10 deletions
diff --git a/compiler/cgen.nim b/compiler/cgen.nim
index 3a6d9a75a..3c309db98 100644
--- a/compiler/cgen.nim
+++ b/compiler/cgen.nim
@@ -1033,7 +1033,7 @@ proc easyResultAsgn(n: PNode): PNode =
 type
   InitResultEnum = enum Unknown, InitSkippable, InitRequired
 
-proc allPathsAsgnResult(n: PNode): InitResultEnum =
+proc allPathsAsgnResult(p: BProc; n: PNode): InitResultEnum =
   # Exceptions coming from calls don't have not be considered here:
   #
   # proc bar(): string = raise newException(...)
@@ -1048,7 +1048,7 @@ proc allPathsAsgnResult(n: PNode): InitResultEnum =
   #   echo "a was not written to"
   #
   template allPathsInBranch(it) =
-    let a = allPathsAsgnResult(it)
+    let a = allPathsAsgnResult(p, it)
     case a
     of InitRequired: return InitRequired
     of InitSkippable: discard
@@ -1060,7 +1060,7 @@ proc allPathsAsgnResult(n: PNode): InitResultEnum =
   case n.kind
   of nkStmtList, nkStmtListExpr:
     for it in n:
-      result = allPathsAsgnResult(it)
+      result = allPathsAsgnResult(p, it)
       if result != Unknown: return result
   of nkAsgn, nkFastAsgn, nkSinkAsgn:
     if n[0].kind == nkSym and n[0].sym.kind == skResult:
@@ -1068,6 +1068,8 @@ proc allPathsAsgnResult(n: PNode): InitResultEnum =
       else: result = InitRequired
     elif containsResult(n):
       result = InitRequired
+    else:
+      result = allPathsAsgnResult(p, n[1])
   of nkReturnStmt:
     if n.len > 0:
       if n[0].kind == nkEmpty and result != InitSkippable:
@@ -1076,7 +1078,7 @@ proc allPathsAsgnResult(n: PNode): InitResultEnum =
         # initialized. This avoids cases like #9286 where this heuristic lead to
         # wrong code being generated.
         result = InitRequired
-      else: result = allPathsAsgnResult(n[0])
+      else: result = allPathsAsgnResult(p, n[0])
   of nkIfStmt, nkIfExpr:
     var exhaustive = false
     result = InitSkippable
@@ -1102,9 +1104,9 @@ proc allPathsAsgnResult(n: PNode): InitResultEnum =
   of nkWhileStmt:
     # some dubious code can assign the result in the 'while'
     # condition and that would be fine. Everything else isn't:
-    result = allPathsAsgnResult(n[0])
+    result = allPathsAsgnResult(p, n[0])
     if result == Unknown:
-      result = allPathsAsgnResult(n[1])
+      result = allPathsAsgnResult(p, n[1])
       # we cannot assume that the 'while' loop is really executed at least once:
       if result == InitSkippable: result = Unknown
   of harmless:
@@ -1129,9 +1131,17 @@ proc allPathsAsgnResult(n: PNode): InitResultEnum =
     allPathsInBranch(n[0])
     for i in 1..<n.len:
       if n[i].kind == nkFinally:
-        result = allPathsAsgnResult(n[i].lastSon)
+        result = allPathsAsgnResult(p, n[i].lastSon)
       else:
         allPathsInBranch(n[i].lastSon)
+  of nkCallKinds:
+    if canRaiseDisp(p, n[0]):
+      result = InitRequired
+    else:
+      for i in 0..<n.safeLen:
+        allPathsInBranch(n[i])
+  of nkRaiseStmt:
+    result = InitRequired
   else:
     for i in 0..<n.safeLen:
       allPathsInBranch(n[i])
@@ -1188,7 +1198,7 @@ proc genProcAux*(m: BModule, prc: PSym) =
         assignLocalVar(p, resNode)
         assert(res.loc.r != "")
         if p.config.selectedGC in {gcArc, gcAtomicArc, gcOrc} and
-            allPathsAsgnResult(procBody) == InitSkippable:
+            allPathsAsgnResult(p, procBody) == InitSkippable:
           # In an ideal world the codegen could rely on injectdestructors doing its job properly
           # and then the analysis step would not be required.
           discard "result init optimized out"
@@ -1208,7 +1218,7 @@ proc genProcAux*(m: BModule, prc: PSym) =
       # global is either 'nil' or points to valid memory and so the RC operation
       # succeeds without touching not-initialized memory.
       if sfNoInit in prc.flags: discard
-      elif allPathsAsgnResult(procBody) == InitSkippable: discard
+      elif allPathsAsgnResult(p, procBody) == InitSkippable: discard
       else:
         resetLoc(p, res.loc)
       if skipTypes(res.typ, abstractInst).kind == tyArray:
diff --git a/tests/arc/tarc_macro.nim b/tests/arc/tarc_macro.nim
index ea7d279fd..33ade1da4 100644
--- a/tests/arc/tarc_macro.nim
+++ b/tests/arc/tarc_macro.nim
@@ -43,4 +43,15 @@ macro bar2() =
     doAssert &%&%y == 1 # unary operator => need to escape
     doAssert y &% y == 2 # binary operator => no need to escape
     doAssert y == 3
-bar2()
\ No newline at end of file
+bar2()
+
+block:
+  macro foo(a: openArray[string] = []): string =
+    echo a # Segfault doesn't happen if this is removed
+    newLit ""
+
+  proc bar(a: static[openArray[string]] = []) =
+    const tmp = foo(a)
+
+  # bug #22909
+  doAssert not compiles(bar())