From 1fae66e4df8cc43b4ec8ab97fff96282ef234f2e Mon Sep 17 00:00:00 2001 From: Andreas Rumpf Date: Tue, 22 Sep 2020 13:03:24 +0200 Subject: [PATCH] better nativestacktrace support; refs #15284; backport [1.2] (#15384) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 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 --- lib/pure/asyncfutures.nim | 13 +++- lib/system.nim | 6 ++ lib/system/exceptions.nim | 5 ++ lib/system/excpt.nim | 45 ++++++------ lib/system/stacktraces.nim | 83 +++++++++++++++++++++++ tests/system/tnim_stacktrace_override.nim | 18 +++++ 6 files changed, 142 insertions(+), 28 deletions(-) create mode 100644 lib/system/stacktraces.nim create mode 100644 tests/system/tnim_stacktrace_override.nim diff --git a/lib/pure/asyncfutures.nim b/lib/pure/asyncfutures.nim index 97abf72c9c..2333c5ef16 100644 --- a/lib/pure/asyncfutures.nim +++ b/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 diff --git a/lib/system.nim b/lib/system.nim index 362b5b2b99..eefa84565a 100644 --- a/lib/system.nim +++ b/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. diff --git a/lib/system/exceptions.nim b/lib/system/exceptions.nim index 3006cff19a..fc8bd89f72 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 089048163c..1a7473a76e 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.. 0: + result.add(stackTraceOverrideGetDebuggingInfo(programCounters, maxStackTraceLines)) diff --git a/tests/system/tnim_stacktrace_override.nim b/tests/system/tnim_stacktrace_override.nim new file mode 100644 index 0000000000..c75da9d9b7 --- /dev/null +++ b/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()