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

honor --excessivestacktrace, refs https://github.com/status-im/nim-libbacktrace/issues/11 #14

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

timotheecour
Copy link
Contributor

@timotheecour timotheecour commented Apr 6, 2021

/cc @stefantalpalaru

refs #11 (most likely an upstream bug, refs ianlancetaylor/libbacktrace#72)

--excessivestacktrace:on|off is now honored, but for now --excessivestacktrace:on requires -d:libbacktraceWorkaroundRelativePaths --experimental:vmopsDanger

when ianlancetaylor/libbacktrace#72, this code will simplify

links

@timotheecour timotheecour changed the title honor --excessivestacktrace, refs #72 honor --excessivestacktrace, refs https://github.com/status-im/nim-libbacktrace/issues/11 Apr 12, 2021
@timotheecour
Copy link
Contributor Author

ping @stefantalpalaru @arnetheduck

@stefantalpalaru
Copy link
Contributor

Requiring -d:libbacktraceWorkaroundRelativePaths --experimental:vmopsDanger makes this more expensive than the benefit it brings (and very unlikely to be used in practice).

Isn't it easier to unconditionally convert relative paths to absolute ones in the C wrapper?

@timotheecour
Copy link
Contributor Author

timotheecour commented Apr 12, 2021

more expensive

I'm not sure what you mean by expensive. Users can just put -d: d:libbacktraceWorkaroundRelativePaths in their user config so they don't have to deal with this on cmdline. There's no cost apart from knowing what those flags are, and in future work addressing #12 can simplify this.

Isn't it easier to unconditionally convert relative paths to absolute ones in the C wrapper?

this wouldn't honor --excessivestacktrace:on|off. Both on|off are useful

  • off: avoiding generating stacktraces containing absolute compilation dir when shipping a binary with debug symbols
  • on: helps debugging during development

unconditionally convert relative paths to absolute ones in C wrapper would mean that absolute paths are generated even if they're not needed which adds bloat/increases binary size (and can expose private paths that binary author may not want exposed)

@arnetheduck
Copy link
Member

most of the time, global -d switches are a significant anti-feature given how it creates dialects of a library rendering it uncomposable in anything beyond toy examples..

@timotheecour
Copy link
Contributor Author

that's already the case with -d:nimStackTraceOverride, and that's the simplest way to solve this problem. The define symbol is prefixed with the library name so will not clash.

I'm all ears for your proposed superior alternative, until then, this PR fixes a usability problem with nim-libbacktrace, allowing it to be more of a drop-in replacement to nim's --stacktrace.

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.

3 participants