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>

3 files changed, 108 insertions, 25 deletions

diff --git a/lib/system/exceptions.nim b/lib/system/exceptions.nim
index 3006cff19..fc8bd89f7 100644
--- a/lib/system/exceptions.nim
+++ b/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.
diff --git a/lib/system/excpt.nim b/lib/system/excpt.nim
index 089048163..1a7473a76 100644
--- a/lib/system/excpt.nim
+++ b/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)
diff --git a/lib/system/stacktraces.nim b/lib/system/stacktraces.nim
new file mode 100644
index 000000000..dce25fdc0
--- /dev/null
+++ b/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))