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

stacktraces can now show custom runtime msgs per frame #13351

Merged
merged 11 commits into from Mar 30, 2020

Conversation

timotheecour
Copy link
Member

@timotheecour timotheecour commented Feb 7, 2020

stacktraces can now show custom runtime msgs per frame.

  • useful in many cases where the stacktrace alone isn't sufficient to debug an issue; in particular for any parser / compiler (such as nim compiler), showing additional runtime info helps showing tracing program along with traced source code (+ arbitrary computed values)

  • efficient implementation using a fixed string, so there is no allocation cost (amortized cost = O(1)); the only allocations happen if an exception is thrown; for stackframe entries with a nonempty custom msg, a string will be allocated before unwinding the stack so that the custom msg isn't invalidated by further calls to setFrameMsg

example:

say you want to debug an internal compiler error such as #12263 which normally gives a regular stacktrace error (with --stacktrace:on);

if we simply add the following line inside proc semExpr(...) (obviously setFrameMsg can be called in other places as needed)

when defined(nimCompilerStackraceHints):
  setFrameMsg c.config$n.info & " " & $n.kind

it'll show that context in each stackframe corresponding to that (re-entrant) function, which helps showing the progress in the file being compiled:

stacktrace with custom runtime msgs for nim c /pathto/t1015.nim
shows both tracing and traced programs along side:

compiler/nim.nim(118) nim
compiler/nim.nim(95) handleCmdLine
compiler/cmdlinehelper.nim(105) loadConfigsAndRunMainCommand
compiler/main.nim(190) mainCommand
compiler/main.nim(92) commandCompileToC
compiler/modules.nim(143) compileProject
compiler/modules.nim(84) compileModule
compiler/passes.nim(210) processModule
compiler/passes.nim(86) processTopLevelStmt
compiler/sem.nim(600) myProcess
compiler/sem.nim(568) semStmtAndGenerateGenerics
compiler/semstmts.nim(2240) semStmt
compiler/semexprs.nim(986) semExprNoType
compiler/semexprs.nim(2743) semExpr /pathto/t1015.nim(5, 1) nkStmtList
compiler/semstmts.nim(2180) semStmtList
compiler/semexprs.nim(2670) semExpr /pathto/t1015.nim(35, 1) nkWhenStmt
compiler/semexprs.nim(2256) semWhen
compiler/semexprs.nim(2743) semExpr /pathto/t1015.nim(36, 3) nkStmtList
compiler/semstmts.nim(2180) semStmtList
compiler/semexprs.nim(2746) semExpr /pathto/t1015.nim(50, 3) nkLetSection
compiler/semstmts.nim(450) semVarOrLet
compiler/semexprs.nim(64) semExprWithType
compiler/semexprs.nim(2708) semExpr /pathto/t1015.nim(50, 33) nkBracket
compiler/semexprs.nim(557) semArrayConstr
compiler/semexprs.nim(64) semExprWithType
compiler/semexprs.nim(2654) semExpr /pathto/t1015.nim(51, 10) nkPrefix
compiler/semexprs.nim(2248) semMagic
compiler/semexprs.nim(967) semDirectOp
compiler/semexprs.nim(816) semOverloadedCallAnalyseEffects
compiler/semcall.nim(532) semOverloadedCall
compiler/semcall.nim(335) resolveOverloads
compiler/semcall.nim(93) pickBestCandidate
compiler/sigmatch.nim(2522) matches
compiler/sigmatch.nim(2459) matchesAux
compiler/sigmatch.nim(2261) prepareOperand
compiler/semexprs.nim(49) semOperand
compiler/semexprs.nim(2708) semExpr /pathto/t1015.nim(51, 11) nkBracket
compiler/semexprs.nim(557) semArrayConstr
compiler/semexprs.nim(64) semExprWithType
compiler/semexprs.nim(2704) semExpr /pathto/t1015.nim(52, 7) nkPar
compiler/semexprs.nim(2513) semTupleConstr
compiler/semexprs.nim(2435) semTuplePositionsConstr
compiler/semexprs.nim(64) semExprWithType
compiler/semexprs.nim(2615) semExpr /pathto/t1015.nim(52, 54) nkDotExpr
compiler/semexprs.nim(2645) semExpr /pathto/t1015.nim(52, 54) nkCall
compiler/semexprs.nim(272) semConv
compiler/semcall.nim(444) inferWithMetatype
compiler/sigmatch.nim(2171) paramTypesMatch
compiler/sigmatch.nim(2015) paramTypesMatchAux
compiler/sigmatch.nim(1654) typeRel
compiler/sigmatch.nim(1210) typeRel
compiler/types.nim(815) lengthOrd
compiler/types.nim(776) lastOrd
compiler/msgs.nim(565) internalError
compiler/msgs.nim(443) rawMessage
compiler/msgs.nim(440) rawMessage
compiler/msgs.nim(356) handleError
compiler/msgs.nim(346) quit

unrelated changes

  • added stackframes.procName

@timotheecour timotheecour changed the title [WIP] stacktraces can now show custom runtime msgs [WIP] stacktraces can now show custom runtime msgs per frame Feb 7, 2020
lib/system.nim Outdated Show resolved Hide resolved
@Varriount
Copy link
Contributor

This is really slick! I can see this being used in things like backend servers to annotate exceptions when something unexpected happens in a request.

@timotheecour timotheecour force-pushed the pr_stacktrace_metadata branch 3 times, most recently from 6e29493 to 577e95e Compare February 7, 2020 23:42
@timotheecour timotheecour reopened this Feb 8, 2020
@timotheecour timotheecour reopened this Feb 8, 2020
@timotheecour timotheecour reopened this Feb 8, 2020
@timotheecour timotheecour changed the title [WIP] stacktraces can now show custom runtime msgs per frame stacktraces can now show custom runtime msgs per frame Feb 8, 2020
@timotheecour timotheecour marked this pull request as ready for review February 8, 2020 01:58
@timotheecour
Copy link
Member Author

PTAL @Araq, ready for review

@timotheecour
Copy link
Member Author

friendly ping @Araq this really helps for debugging, whether you're debugging compiler or debugging user code, making stacktraces a lot more powerful

@Varriount
Copy link
Contributor

Do you know if this would be compatible with talk of using libbacktrace?

@timotheecour
Copy link
Member Author

timotheecour commented Feb 28, 2020

Do you know if this would be compatible with talk of using libbacktrace?

this only works with nim's default backtrace which relies on inserting code at begin/end of each function; this can't work with libbacktrace (which you get with -d:nativeStacktrace), unless you added some lowlevel asm code to emulate what I did in this PR, which doesn't sound easy.

@timotheecour
Copy link
Member Author

timotheecour commented Mar 6, 2020

PTAL, all comments addressed, and now test works with or without --gc:arc; I'd really like to have this PR merged as it helps a lot when debugging (whether it's user programs or nim compiler itself);
happy to address any further comments

@dom96
Copy link
Contributor

dom96 commented Mar 7, 2020

Cool. Looks like a nice feature, up to Araq or someone else knowledgeable with these internals to review though. :)

@timotheecour
Copy link
Member Author

timotheecour commented Mar 19, 2020

PTAL @Araq

https://gitter.im/nim-lang/Nim?at=5e69663a47b42479253f82f5]

From IRC (bridge bot) @FromIRC Mar 11 15:40
how is 'var frameMsgBuf* {.threadvar.}: string' entirely opt-in?
anyhow, can you make it a 'cstring'? constants are fine and with a little care you can ensure the string is from a buffer that is not freed to early

done, see latest commit; frameMsgBuf is now a CstringWrapper which wraps a string-like interface on a c_realloc-allocated growable cstring buffer

@timotheecour
Copy link
Member Author

timotheecour commented Mar 20, 2020

this is blocked by #13701 which should be merged first; EDIT: got merged

@timotheecour
Copy link
Member Author

friendly ping @Araq

@Araq
Copy link
Member

Araq commented Mar 24, 2020

Sorry, I was unclear. Can the frameMsg*: string field be a cstring?

@timotheecour
Copy link
Member Author

I don't see how that'd work, please explain; the problem with frameMsg: cstring (that would point to some offset in frameMsgBuf) is when you unwind AND you run some other code that could overwrite what frameMsg points to:

proc fun()=setFrameMsg("foo")
try:
  fun()
except Exception as e:
  printf("%s\n", e.frameMsg) # that should be safe even though stack was unwinded
  bar() # this could overwrite what `e.frameMsg` points to, potentially
  printf("%s\n", e.frameMsg) # now we are reading the wrong thing

unless what you mean is for frameMsg to have its own memory (malloc-managed like it did via CstringWrapper, excpect i now also need to care about deallocation), but I really don't see the advantage over string in that case.

note

how about I hide this behind --stacktrace:annotate flag instead?
so it becomes entirely opt-in.

@Araq
Copy link
Member

Araq commented Mar 24, 2020

how about I hide this behind --stacktrace:annotate flag instead?

Ok.

@timotheecour
Copy link
Member Author

timotheecour commented Mar 25, 2020

done, PTAL

  • It's now off by default; future PR's may change the default after gather more usage
  • added tests with --stacktraceMsgs:on/off

@Araq Araq merged commit 19cab9f into nim-lang:devel Mar 30, 2020
@timotheecour
Copy link
Member Author

note:
you can use this in your user config:

when defined nimHasStacktraceMsgs: switch("stacktraceMsgs", "on")

which is backward compatible

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants