Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
1 of 5 tasks
timotheecour opened this issue Mar 4, 2020 · 6 comments

Comments

@timotheecour
Copy link
Owner

timotheecour commented Mar 4, 2020

/cc @stefantalpalaru what do you think of this?

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

@stefantalpalaru
Copy link

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
Copy link
Owner Author

timotheecour commented Mar 7, 2020

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).

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
Copy link

--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
Copy link
Owner Author

timotheecour commented Mar 7, 2020

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:

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
Copy link

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 timotheecour changed the title combine nim classic backtrace with C++ backtraces to get full backtrace combine nim classic backtrace with C++ backtraces to get full backtrace + other TODO's regarding nimStackTraceOverride Mar 8, 2020
@timotheecour
Copy link
Owner Author

timotheecour commented Mar 8, 2020

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

@timotheecour timotheecour changed the title combine nim classic backtrace with C++ backtraces to get full backtrace + other TODO's regarding nimStackTraceOverride combine nim classic stacktrace with C++ backtraces to get full backtrace + other TODO's regarding nimStackTraceOverride Apr 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants