fix partially #13115 (now works for cpp; but still fails for js on openbsd) (#16167)

* fix partially #13115 properly (works for c,js,cpp,vm; still fails for js on openbsd)
* address comment: also test with -d:danger, -d:debug

lib/system/excpt.nim

@@ -13,6 +13,8 @@
 import std/private/miscdollars
 import stacktraces
 
+const noStacktraceAvailable = "No stack traceback available\n"
+
 var
   errorMessageWriter*: (proc(msg: string) {.tags: [WriteIOEffect], benign,
                                             nimcall.})
@@ -36,6 +38,10 @@ else:
   proc writeToStdErr(msg: cstring, length: int) =
     discard MessageBoxA(nil, msg, nil, 0)
 
+proc writeToStdErr(msg: string) {.inline.} =
+  # fix bug #13115: handles correctly '\0' unlike default implicit conversion to cstring
+  writeToStdErr(msg.cstring, msg.len)
+
 proc showErrorMessage(data: cstring, length: int) {.gcsafe, raises: [].} =
   var toWrite = true
   if errorMessageWriter != nil:
@@ -51,6 +57,9 @@ proc showErrorMessage(data: cstring, length: int) {.gcsafe, raises: [].} =
     else:
       writeToStdErr(data, length)
 
+proc showErrorMessage2(data: string) {.inline.} =
+  showErrorMessage(data.cstring, data.len)
+
 proc chckIndx(i, a, b: int): int {.inline, compilerproc, benign.}
 proc chckRange(i, a, b: int): int {.inline, compilerproc, benign.}
 proc chckRangeF(x, a, b: float): float {.inline, compilerproc, benign.}
@@ -123,11 +132,11 @@ proc popSafePoint {.compilerRtl, inl.} =
 proc pushCurrentException(e: sink(ref Exception)) {.compilerRtl, inl.} =
   e.up = currException
   currException = e
-  #showErrorMessage "A"
+  #showErrorMessage2 "A"
 
 proc popCurrentException {.compilerRtl, inl.} =
   currException = currException.up
-  #showErrorMessage "B"
+  #showErrorMessage2 "B"
 
 proc popCurrentExceptionEx(id: uint) {.compilerRtl.} =
   discard "only for bootstrapping compatbility"
@@ -305,7 +314,7 @@ when hasSomeStackTrace:
       auxWriteStackTraceWithOverride(s)
     elif NimStackTrace:
       if framePtr == nil:
-        add(s, "No stack traceback available\n")
+        add(s, noStacktraceAvailable)
       else:
         add(s, "Traceback (most recent call last)\n")
         auxWriteStackTrace(framePtr, s)
@@ -313,7 +322,7 @@ when hasSomeStackTrace:
       add(s, "Traceback from system (most recent call last)\n")
       auxWriteStackTraceWithBacktrace(s)
     else:
-      add(s, "No stack traceback available\n")
+      add(s, noStacktraceAvailable)
 
   proc rawWriteStackTrace(s: var seq[StackTraceEntry]) =
     when defined(nimStackTraceOverride):
@@ -363,7 +372,7 @@ proc reportUnhandledErrorAux(e: ref Exception) {.nodestroy.} =
     if onUnhandledException != nil:
       onUnhandledException(buf)
     else:
-      showErrorMessage(buf, buf.len)
+      showErrorMessage2(buf)
     `=destroy`(buf)
   else:
     # ugly, but avoids heap allocations :-)
@@ -504,16 +513,16 @@ proc writeStackTrace() =
   when hasSomeStackTrace:
     var s = ""
     rawWriteStackTrace(s)
-    cast[proc (s: cstring, length: int) {.noSideEffect, tags: [], nimcall, raises: [].}](showErrorMessage)(s, s.len)
   else:
-    cast[proc (s: cstring, length: int) {.noSideEffect, tags: [], nimcall, raises: [].}](showErrorMessage)("No stack traceback available\n", 32)
+    let s = noStacktraceAvailable
+  cast[proc (s: string) {.noSideEffect, tags: [], nimcall, raises: [].}](showErrorMessage2)(s)
 
 proc getStackTrace(): string =
   when hasSomeStackTrace:
     result = ""
     rawWriteStackTrace(result)
   else:
-    result = "No stack traceback available\n"
+    result = noStacktraceAvailable
 
 proc getStackTrace(e: ref Exception): string =
   if not isNil(e):
@@ -543,7 +552,7 @@ proc callDepthLimitReached() {.noinline.} =
       $nimCallDepthLimit & " function calls). You can change it with " &
       "-d:nimCallDepthLimit=<int> but really try to avoid deep " &
       "recursions instead.\n"
-  showErrorMessage(msg, msg.len)
+  showErrorMessage2(msg)
   quit(1)
 
 proc nimFrame(s: PFrame) {.compilerRtl, inl, raises: [].} =
@@ -627,13 +636,16 @@ when not defined(noSignalHandler) and not defined(useNimRtl):
       var buf = newStringOfCap(2000)
       rawWriteStackTrace(buf)
       processSignal(sign, buf.add) # nice hu? currying a la Nim :-)
-      showErrorMessage(buf, buf.len)
+      showErrorMessage2(buf)
       when not usesDestructors: GC_enable()
     else:
       var msg: cstring
       template asgn(y) =
         msg = y
       processSignal(sign, asgn)
+      # xxx use string for msg instead of cstring, and here use showErrorMessage2(msg)
+      # unless there's a good reason to use cstring in signal handler to avoid
+      # using gc?
       showErrorMessage(msg, msg.len)
     quit(1) # always quit when SIGABRT
 

tests/exception/t13115.nim

@@ -1,12 +1,38 @@
-discard """
+const msg = "This char is `" & '\0' & "` and works fine!"
-  exitcode: 1
-  targets: "c"
-  matrix: "-d:debug; -d:release"
-  outputsub: ''' and works fine! [Exception]'''
-"""
 
-# bug #13115
+when defined nim_t13115:
-# xxx bug: doesn't yet work for cpp
+  # bug #13115
+  template fn =
+    raise newException(Exception, msg)
+  when defined nim_t13115_static:
+    static: fn()
+  fn()
+else:
+  import std/[osproc,strformat,os,strutils]
+  proc main =
+    const nim = getCurrentCompilerExe()
+    const file = currentSourcePath
+    for b in "c js cpp".split:
+      when defined(openbsd):
+        if b == "js":
+          # xxx bug: pending #13115
+          # remove special case once nodejs updated >= 12.16.2
+          # refs https://github.com/nim-lang/Nim/pull/16167#issuecomment-738270751
+          continue
 
-var msg = "This char is `" & '\0' & "` and works fine!"
+      # save CI time by avoiding mostly redundant combinations as far as this bug is concerned
-raise newException(Exception, msg)
+      var opts = case b
+        of "c": @["", "-d:nim_t13115_static", "-d:danger", "-d:debug"]
+        of "js": @["", "-d:nim_t13115_static"]
+        else: @[""]
+
+      for opt in opts:
+        let cmd = fmt"{nim} r -b:{b} -d:nim_t13115 {opt} --hints:off {file}"
+        let (outp, exitCode) = execCmdEx(cmd)
+        when defined windows:
+          # `\0` not preserved on windows
+          doAssert "` and works fine!" in outp, cmd & "\n" & msg
+        else:
+          doAssert msg in outp, cmd & "\n" & msg
+        doAssert exitCode == 1
+  main()