better nativestacktrace support; refs #15284; backport [1.2] (#15384)

* nimStackTraceOverride: enable stack traces in exceptions

This is a two-step stack trace collection scheme, because re-raised
exceptions will collect multiple stack traces but use them rarely, when
printing info about an uncaught exception, so it makes sense to only do
the cheap stack unwinding all the time and the relatively expensive
debugging information collection on-demand.

`asyncfutures` implements its own `$` proc for printing
`seq[StackTraceEntry]`, so we have to add the debugging info there, just
like we do for the private `$` proc in `system/excpt`.

* cleaned up PR #15284

Co-authored-by: Ștefan Talpalaru <stefantalpalaru@yahoo.com>

lib/pure/asyncfutures.nim

@@ -9,6 +9,8 @@
 
 import os, tables, strutils, times, heapqueue, options, deques, cstrutils
 
+import "system/stacktraces"
+
 # TODO: This shouldn't need to be included, but should ideally be exported.
 type
   CallbackFunc = proc () {.closure, gcsafe.}
@@ -293,7 +295,12 @@ proc getHint(entry: StackTraceEntry): string =
     if cmpIgnoreStyle(entry.filename, "asyncmacro.nim") == 0:
       return "Resumes an async procedure"
 
-proc `$`*(entries: seq[StackTraceEntry]): string =
+proc `$`*(stackTraceEntries: seq[StackTraceEntry]): string =
+  when defined(nimStackTraceOverride):
+    let entries = addDebuggingInfo(stackTraceEntries)
+  else:
+    let entries = stackTraceEntries
+
   result = ""
   # Find longest filename & line number combo for alignment purposes.
   var longestLeft = 0
@@ -308,10 +315,10 @@ proc `$`*(entries: seq[StackTraceEntry]): string =
   # Format the entries.
   for entry in entries:
     if entry.procname.isNil:
-      if entry.line == -10:
+      if entry.line == reraisedFromBegin:
         result.add(spaces(indent) & "#[\n")
         indent.inc(2)
-      else:
+      elif entry.line == reraisedFromEnd:
         indent.dec(2)
         result.add(spaces(indent) & "]#\n")
       continue

lib/system.nim

@@ -1383,6 +1383,12 @@ type # these work for most platforms:
   culonglong* {.importc: "unsigned long long", nodecl.} = uint64
     ## This is the same as the type ``unsigned long long`` in *C*.
 
+  # There is a disparity on macOS where Nim's `uint` is `unsigned long long` and
+  # `uintptr_t` is `unsigned long`. Even though both data types are the same
+  # size (64 bits), clang++ refuses to do automatic conversion between them.
+  cuintptr_t* {.importc: "uintptr_t", nodecl.} = uint
+    ## This is the same as the type ``uintptr_t`` in *C*.
+
   cstringArray* {.importc: "char**", nodecl.} = ptr UncheckedArray[cstring]
     ## This is binary compatible to the type ``char**`` in *C*. The array's
     ## high value is large enough to disable bounds checking in practice.

lib/system/exceptions.nim

@@ -25,6 +25,11 @@ type
                             ## rendered at a later time, we should ensure the stacktrace
                             ## data isn't invalidated; any pointer into PFrame is
                             ## subject to being invalidated so shouldn't be stored.
+    when defined(nimStackTraceOverride):
+      programCounter*: uint ## Program counter - will be used to get the rest of the info,
+                            ## when `$` is called on this type. We can't use
+                            ## "cuintptr_t" in here.
+      procnameStr*, filenameStr*: string ## GC-ed objects holding the cstrings in "procname" and "filename"
 
   Exception* {.compilerproc, magic: "Exception".} = object of RootObj ## \
     ## Base exception class.

lib/system/excpt.nim

@@ -11,6 +11,7 @@
 # use the heap (and nor exceptions) do not include the GC or memory allocator.
 
 import std/private/miscdollars
+import stacktraces
 
 var
   errorMessageWriter*: (proc(msg: string) {.tags: [WriteIOEffect], benign,
@@ -137,20 +138,6 @@ const
   hasSomeStackTrace = NimStackTrace or defined(nimStackTraceOverride) or
     (defined(nativeStackTrace) and nativeStackTraceSupported)
 
-when defined(nimStackTraceOverride):
-  type StackTraceOverrideProc* = proc (): string {.nimcall, noinline, benign, raises: [], tags: [].}
-    ## Procedure type for overriding the default stack trace.
-
-  var stackTraceOverrideGetTraceback: StackTraceOverrideProc = proc(): string {.noinline.} =
-    result = "Stack trace override procedure not registered.\n"
-
-  proc registerStackTraceOverride*(overrideProc: StackTraceOverrideProc) =
-    ## Override the default stack trace inside rawWriteStackTrace() with your
-    ## own procedure.
-    stackTraceOverrideGetTraceback = overrideProc
-
-  proc auxWriteStackTraceWithOverride(s: var string) =
-    add(s, stackTraceOverrideGetTraceback())
 
 when defined(nativeStacktrace) and nativeStackTraceSupported:
   type
@@ -168,13 +155,13 @@ when defined(nativeStacktrace) and nativeStackTraceSupported:
 
   when not hasThreadSupport:
     var
-      tempAddresses: array[0..127, pointer] # should not be alloc'd on stack
+      tempAddresses: array[maxStackTraceLines, pointer] # should not be alloc'd on stack
       tempDlInfo: TDl_info
 
   proc auxWriteStackTraceWithBacktrace(s: var string) =
     when hasThreadSupport:
       var
-        tempAddresses: array[0..127, pointer] # but better than a threadvar
+        tempAddresses: array[maxStackTraceLines, pointer] # but better than a threadvar
         tempDlInfo: TDl_info
     # This is allowed to be expensive since it only happens during crashes
     # (but this way you don't need manual stack tracing)
@@ -202,11 +189,7 @@ when defined(nativeStacktrace) and nativeStackTraceSupported:
 
 when hasSomeStackTrace and not hasThreadSupport:
   var
-    tempFrames: array[0..127, PFrame] # should not be alloc'd on stack
-
-const
-  reraisedFromBegin = -10
-  reraisedFromEnd = -100
+    tempFrames: array[maxStackTraceLines, PFrame] # should not be alloc'd on stack
 
 template reraisedFrom(z): untyped =
   StackTraceEntry(procname: nil, line: z, filename: nil)
@@ -253,7 +236,12 @@ template addFrameEntry(s: var string, f: StackTraceEntry|PFrame) =
       for i in first..<f.frameMsgLen: add(s, frameMsgBuf[i])
   add(s, "\n")
 
-proc `$`(s: seq[StackTraceEntry]): string =
+proc `$`(stackTraceEntries: seq[StackTraceEntry]): string =
+  when defined(nimStackTraceOverride):
+    let s = addDebuggingInfo(stackTraceEntries)
+  else:
+    let s = stackTraceEntries
+
   result = newStringOfCap(2000)
   for i in 0 .. s.len-1:
     if s[i].line == reraisedFromBegin: result.add "[[reraised from:\n"
@@ -265,7 +253,7 @@ when hasSomeStackTrace:
   proc auxWriteStackTrace(f: PFrame, s: var string) =
     when hasThreadSupport:
       var
-        tempFrames: array[0..127, PFrame] # but better than a threadvar
+        tempFrames: array[maxStackTraceLines, PFrame] # but better than a threadvar
     const
       firstCalls = 32
     var
@@ -324,7 +312,9 @@ when hasSomeStackTrace:
       add(s, "No stack traceback available\n")
 
   proc rawWriteStackTrace(s: var seq[StackTraceEntry]) =
-    when NimStackTrace:
+    when defined(nimStackTraceOverride):
+      auxWriteStackTraceWithOverride(s)
+    elif NimStackTrace:
       auxWriteStackTrace(framePtr, s)
     else:
       s = @[]
@@ -468,7 +458,12 @@ proc raiseExceptionEx(e: sink(ref Exception), ename, procname, filename: cstring
   if e.name.isNil: e.name = ename
   when hasSomeStackTrace:
     when defined(nimStackTraceOverride):
-      e.trace = @[]
+      if e.trace.len == 0:
+        rawWriteStackTrace(e.trace)
+      else:
+        e.trace.add reraisedFrom(reraisedFromBegin)
+        auxWriteStackTraceWithOverride(e.trace)
+        e.trace.add reraisedFrom(reraisedFromEnd)
     elif NimStackTrace:
       if e.trace.len == 0:
         rawWriteStackTrace(e.trace)

lib/system/stacktraces.nim

@@ -0,0 +1,83 @@
+#
+#
+#            Nim's Runtime Library
+#        (c) Copyright 2015 Andreas Rumpf
+#
+#    See the file "copying.txt", included in this
+#    distribution, for details about the copyright.
+#
+
+# Additional code for customizable stack traces. Unstable API, for internal
+# usage only.
+
+const
+  reraisedFromBegin* = -10
+  reraisedFromEnd* = -100
+  maxStackTraceLines* = 128
+
+when defined(nimStackTraceOverride):
+  ## Procedure types for overriding the default stack trace.
+  type
+    cuintptr_t {.importc: "uintptr_t", nodecl.} = uint
+      ## This is the same as the type ``uintptr_t`` in C.
+
+    StackTraceOverrideGetTracebackProc* = proc (): string {.
+      nimcall, gcsafe, locks: 0, raises: [], tags: [].}
+    StackTraceOverrideGetProgramCountersProc* = proc (maxLength: cint): seq[cuintptr_t] {.
+      nimcall, gcsafe, locks: 0, raises: [], tags: [].}
+    StackTraceOverrideGetDebuggingInfoProc* =
+      proc (programCounters: seq[cuintptr_t], maxLength: cint): seq[StackTraceEntry] {.
+        nimcall, gcsafe, locks: 0, raises: [], tags: [].}
+
+  # Default procedures (not normally used, because people opting in on this
+  # override are supposed to register their own versions).
+  var
+    stackTraceOverrideGetTraceback: StackTraceOverrideGetTracebackProc =
+      proc (): string {.nimcall, gcsafe, locks: 0, raises: [], tags: [].} =
+        discard
+        #result = "Stack trace override procedure not registered.\n"
+    stackTraceOverrideGetProgramCounters: StackTraceOverrideGetProgramCountersProc =
+      proc (maxLength: cint): seq[cuintptr_t] {.nimcall, gcsafe, locks: 0, raises: [], tags: [].} =
+        discard
+    stackTraceOverrideGetDebuggingInfo: StackTraceOverrideGetDebuggingInfoProc =
+      proc (programCounters: seq[cuintptr_t], maxLength: cint): seq[StackTraceEntry] {.
+        nimcall, gcsafe, locks: 0, raises: [], tags: [].} =
+          discard
+
+  # Custom procedure registration.
+  proc registerStackTraceOverride*(overrideProc: StackTraceOverrideGetTracebackProc) =
+    ## Override the default stack trace inside rawWriteStackTrace() with your
+    ## own procedure.
+    stackTraceOverrideGetTraceback = overrideProc
+  proc registerStackTraceOverrideGetProgramCounters*(overrideProc: StackTraceOverrideGetProgramCountersProc) =
+    stackTraceOverrideGetProgramCounters = overrideProc
+  proc registerStackTraceOverrideGetDebuggingInfo*(overrideProc: StackTraceOverrideGetDebuggingInfoProc) =
+    stackTraceOverrideGetDebuggingInfo = overrideProc
+
+  # Custom stack trace manipulation.
+  proc auxWriteStackTraceWithOverride*(s: var string) =
+    add(s, stackTraceOverrideGetTraceback())
+
+  proc auxWriteStackTraceWithOverride*(s: var seq[StackTraceEntry]) =
+    let programCounters = stackTraceOverrideGetProgramCounters(maxStackTraceLines)
+    if s.len == 0:
+      s = newSeqOfCap[StackTraceEntry](programCounters.len)
+    for programCounter in programCounters:
+      s.add(StackTraceEntry(programCounter: cast[uint](programCounter)))
+
+  # We may have more stack trace lines in the output, due to inlined procedures.
+  proc addDebuggingInfo*(s: seq[StackTraceEntry]): seq[StackTraceEntry] =
+    var programCounters: seq[cuintptr_t]
+    # We process program counters in groups from complete stack traces, because
+    # we have logic that keeps track of certain functions being inlined or not.
+    for entry in s:
+      if entry.procname.isNil and entry.programCounter != 0:
+        programCounters.add(cast[cuintptr_t](entry.programCounter))
+      elif entry.procname.isNil and (entry.line == reraisedFromBegin or entry.line == reraisedFromEnd):
+        result.add(stackTraceOverrideGetDebuggingInfo(programCounters, maxStackTraceLines))
+        programCounters = @[]
+        result.add(entry)
+      else:
+        result.add(entry)
+    if programCounters.len > 0:
+      result.add(stackTraceOverrideGetDebuggingInfo(programCounters, maxStackTraceLines))

tests/system/tnim_stacktrace_override.nim

@@ -0,0 +1,18 @@
+discard """
+  cmd: "nim c -d:nimStacktraceOverride $file"
+  output: '''begin
+Traceback (most recent call last, using override)
+Error: unhandled exception: stack trace produced [ValueError]
+'''
+  exitcode: 1
+"""
+
+import asyncfutures
+
+proc main =
+  echo "begin"
+  if true:
+    raise newException(ValueError, "stack trace produced")
+  echo "unreachable"
+
+main()