combine nim classic backtrace with C++ backtraces to get full backtrace + other TODO's regarding `nimStackTraceOverride`

[timotheecour]

/cc @stefantalpalaru what do you think of this?

• when code throws from a C++ function called by a nim code, nim classic backtrace doesn't show the c++ backtrace
• the nim native backtrace (introduced in nim-lang#12922) does show the C++ backtrace, and has its use cases (eg faster), but doesn't show as complete info as the nim classic backtrace (depending on -d:release etc, as well as extensions like nim-lang#13351 which would be hard with native backtrace)

• we can get best of both worlds by combining both to get a complete backtrace
• Note: as for -d:nativeStackTrace itself, i couldn't make it work
• [EDIT] proc getBacktrace*(): string {.noinline.} could be more efficient see status-im/nim-libbacktrace#3
• -d:nimStackTraceOverride doesn't honor --excessivestacktrace, eg i got some non-absplute paths shown eg:

Traceback (most recent call last, using override)
lib/system.nim(2133) main
lib/system.nim(2126) NimMain
lib/system.nim(2118) NimMainInner
/Users/timothee/git_clone/nim/timn/tests/nim/all/t10308.nim(13) t10308
/Users/timothee/git_clone/nim/timn/tests/nim/all/t10308.nim(12) fun1
/Users/timothee/git_clone/nim/timn/tests/nim/all/t10308.nim(11) fun2
/Users/timothee/git_clone/nim/timn/tests/nim/all/t10308.nim(10) fun3
lib/system/assertions.nim(27) failedAssertImpl
lib/system/assertions.nim(20) raiseAssert
lib/system/fatal.nim(55) sysFatal
lib/system/excpt.nim(482) raiseExceptionEx
lib/system/excpt.nim(463) raiseExceptionAux
lib/system/excpt.nim(408) reportUnhandledError
lib/system/excpt.nim(359) reportUnhandledErrorAux
lib/system/excpt.nim(314) rawWriteStackTrace
lib/system/excpt.nim(163) auxWriteStackTraceWithOverride
/Users/timothee/git_clone/nim/nim-libbacktrace/libbacktrace.nim(39) getBacktrace
/Users/timothee/git_clone/nim/nim-libbacktrace/libbacktrace_wrapper.c(0) get_backtrace_c
Error: unhandled exception: /Users/timothee/git_clone/nim/timn/tests/nim/all/t10308.nim(10, 34) `false`  [AssertionError]
Error: execution of an external program failed: '/Users/timothee/git_clone/nim/timn/tests/nim/all/t10308 '
No stack traceback available
To create a stacktrace, rerun compilation with ./koch temp c <file>

EDIT: seems to work now for some reason

[EDIT]: --excessivestacktrace:off buggy

• --excessivestacktrace seems buggy with -d:nimStackTraceOverride --import:libbacktrace

retried today, now I'm seeing a different behavior:

• with --excessivestacktrace:on works (full paths)
• with --excessivestacktrace:off
  /Users/timothee/git_clone/nim/Nim_prs/t10308.nim is not a real path, it's a concatenation of cwd /Users/timothee/git_clone/nim/Nim_prs and the main project file t10308.nim

/Users/timothee/git_clone/nim/timn/tests/nim/all/t10308.nim(24) fun2
/Users/timothee/git_clone/nim/Nim_prs/t10308.nim(23) fun3
/Users/timothee/git_clone/nim/Nim_devel/lib/system/assertions.nim(29) failedAssertImpl

• and in fact there shouldn't be any absolute paths at all, it should match behavior of nim's standard stacktraces

t10308.nim(24)           fun2
t10308.nim(23)           fun3
assertions.nim(29)       failedAssertImpl

note

to get override stacktrace use:

nimble install libbacktrace
nim c -r --stacktrace:off -d:nimStackTraceOverride --import:libbacktrace -d:case2 -f -g $timn_D/tests/nim/all/t10308.nim

links

• nim-lang/RFCs#198
  D20200304T034657

[stefantalpalaru]

I don't think that combining them is desirable, from the point of view of performance. Let's look at their respective problems:

Default stack tracing:

• requires debug builds
• injects popFrame() calls at the end of each C function, preventing any tail-call optimisation by the C compiler
• relevant overhead by maintaining a function call stack in its own data structure

libbacktrace alternative:

• can't get an exception's stack trace (can we fix this?)
• requires debugging info in the binary

I don't think file path length is something we can blame on libbacktrace. Those paths are injected in the debugging section by #line preprocessor directives, right? Does "--excessivestacktrace" change those into absolute paths?

So I propose we focus on getting the libbacktrace version feature-complete by letting it extract stack traces from exceptions. Maybe with something dumb but cheap like storing the output of stackTraceOverrideGetTraceback() in the Exception object.

As a separate goal, clarify how file paths present in libbacktrace's output are generated and controlled.

[timotheecour]

I don't think that combining them is desirable, from the point of view of performance

I should've been clear, I mean to give an option for combining (off by default);

• the whole point being getting a full stacktrace in this case:

nim_fun1() => nim_fun2() => c_fun3() => c_fun4() => throw c++ exception

even with -d:danger which typically inlines a lot of nim procs making stacktraces show very little with -d:nimStackTraceOverride (unlike with --stacktrace:on which still shows everything, which is desirable or not depending on what user wants).

• Furthermore, --stacktrace:on is also useful for things like nim-lang#13351 which it would be nice to have the C++ end of the stacktrace even in combination with that

requires debug builds

no, just --stacktrace:on, which can be combined with -d:danger for eg

can't get an exception's stack trace (can we fix this?)
feature-complete by letting it extract stack traces from exceptions

do you mean a nim exception? these stacktraces are generated only --stacktrace:on which instruments cgen'd code by adding nimFrame/popFrame; so fixing ting would indeed require the combining I was mentioning

I don't think file path length is something we can blame on libbacktrace [...] Does "--excessivestacktrace" change those into absolute paths?

for some reason re-running my old command shows a different result today, using latest nim devel cb0f7c5

--excessivestacktrace is now honored but --excessivestacktrace:off is now buggy, see --excessivestacktrace:off buggy in top post.

[stefantalpalaru]

--stacktrace:on, which can be combined with -d:danger

Won't it be overridden here? https://github.com/nim-lang/Nim/blob/b6924383df63c91f0ad6baf63d0b1aa84f9329b7/config/nim.cfg#L73

do you mean a nim exception?

Yes, for https://nim-lang.org/docs/system.html#getStackTrace%2Cref.Exception

fixing ting would indeed require the combining I was mentioning

I hope not. I want the feature without that overhead.

[timotheecour]

Won't it be overridden here?

no, see for yourself with:
nim c -r -d:danger main
nim c -r -d:danger --stacktrace:off main # overrides what's done in -d:danger

proc main() = doAssert false
main()

in fact, -d:danger --stacktrace:on is quite useful in some situations

I hope not. I want the feature without that overhead.

but I'm arguing there should be a way for user to have that, as an option:

Nim standard stacktraces enable things that are impossible with libbacktrace stacktraces, such as:

• nim-lang#13351 (super useful, yet impossible with libbacktrace only)
• showing frame info for inlined procs
• [EDIT] showing frames for template calls as if they were proc calls (possible via patching compiler)

for the 2nd point, some debuggers (eg lldb) show the inlined procs as [inlined] but only inside a debugging session, these inlined procs disappear from stacktraces in libbacktrace. The user may want to turn on optimizations (including inlining) yet still have acces to inlined frames in a stacktrace.

[stefantalpalaru]

OK. We'll have to complicate the logic in "system/excpt.nim". How would we avoid any stack tracing duplication? You only want to use libbacktrace in non-Nim code, right? How do we do that?

[timotheecour]

• also added 1 more use case I had in mind (see EDIT in #49 (comment))
• actually, showing C++ stacktraces in nim_fun1() => nim_fun2() => c_fun3() => c_fun4() => throw c++ exception chains (even with -d:danger) was a prime use case I had for your PR

How would we avoid any stack tracing duplication?

the simplest, way, 100% robust is via address comparison:

• first check whether on current architecture stack addresses go up or down (see https://stackoverflow.com/questions/1677415/does-stack-grow-upward-or-downward); let's assume down here;

• then merge standard stacktrace and libbacktrace as follows:

  • generate nim backtrace
  • find address of deepest PFrame (which is stack allocated), call it deepestNimFrame: int
  • generate the C++ backtrace via libbacktrace, but skip the shallowest ones that satisfy:
    addr(currentLibbacktraceFrame) >= deepestNimFrame (assuming addresses go downward)

• however, I'm working on a PR that would replace TFrame (allocated on the stack at beginning of each generated proc, see nimFrame) by something more efficient (using a single threadlocal seq[Tframe] and a slimmer Tframe that is updated inside nimFrame + popFrame via a simple setLen).
  In this setting, deepestNimFrame won't be accessible anymore unless we re-add current stack address to Tframe (maybe there's a way without this extra overhead, however small), ideas welcome.

note

not yet sure how to get addresses of frames via libbacktrace but it should be exposed somewhere; eg with the simpler backtrace (https://linux.die.net/man/3/backtrace) it's available right there; I've asked here ianlancetaylor/libbacktrace#35

alternative based on string comparison

an alternative, but not sure if it's 100% robust, would be doing it from string comparison of generated stacktrace locations, so that:

# standard stacktrace
/pathto/fun1:12
/pathto/fun2:32 # this one won't appear in libbacktrace due to inlining, or bc of a fake nimFrame generated for templates (See note above)
/pathto/fun3:32 # deepest frame in common, used for deduplication

# libbacktrace stacktrace
/pathto/fun1:12
/pathto/fun3:32
/pathto/fun4:44  # this one won't appear in standard backtrace because comes from C++

would deduplicate to:

/pathto/fun1:12
/pathto/fun2:32
/pathto/fun3:32
/pathto/fun4:44

other example, in the case /pathto/fun3:32 gets inlined hence ommitted from libbacktrace:

# libbacktrace stacktrace
/pathto/fun1:12
/pathto/fun4:44  # this one won't appear in standard backtrace because comes from C++

would also deduplicate to:

/pathto/fun1:12
/pathto/fun2:32
/pathto/fun3:32
/pathto/fun4:44

so it seems to work in all cases

but that assumes we can match exactly file/line info bw debug info (libbacktrace) and nimFrame data (standard backtrace); hopefully ok since we are in control of debug info generated