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.

Sign up for GitHub

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

Jump to bottom

stack-trace symbolizer using llvm-symbolizer subprocess #297

Merged
merged 4 commits into from
Jan 23, 2023
Merged

stack-trace symbolizer using llvm-symbolizer subprocess #297

merged 4 commits into from
Jan 23, 2023

Conversation

mikea
Copy link
Contributor

@mikea mikea commented Jan 19, 2023 •
edited
Loading

When linked it, looks for $LLVM_SYMBOLIZER environment variable and uses it for stacktrace symbolication.

To use it with bazel you should add --config=fastdbg --test_env=LLVM_SYMBOLIZER=llvm-symbolizer-15
to your test command.

@mikea mikea requested a review from kentonv January 19, 2023 22:19
@mikea mikea force-pushed the maizatskyi/2023-01-19-symbolizer branch 2 times, most recently from 6003453 to 8531033 Compare January 19, 2023 22:21
@mikea
stack-trace symbolizer using llvm-symbolizer subprocess
3fa2faa 
When linked it, looks for $LLVM_SYMBOLIZER environment variable
and uses it for stacktrace symbolication.

To effectively use it with bazel you should add:
  -c dbg --test_env=LLVM_SYMBOLIZER=llvm-symbolizer-15
to your test command.
@mikea mikea force-pushed the maizatskyi/2023-01-19-symbolizer branch from 8531033 to 3fa2faa Compare January 19, 2023 22:22
@mikea mikea marked this pull request as ready for review January 19, 2023 22:22
kentonv
kentonv reviewed Jan 20, 2023
View reviewed changes
src/workerd/util/symbolizer.c++
};

String stringifyStackTrace(ArrayPtr<void* const> trace) {
const char* llvmSymbolizer = getenv("LLVM_SYMBOLIZER");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried using this but it appears bazel hides all environment variables from test programs, so it doesn't work unless I run the test directly. What's the right way to make it work when the test runs under bazel?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(wrong chain)

it is in the top comment: -c dbg --test_env=LLVM_SYMBOLIZER=llvm-symbolizer-15

we can always make things default later

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Duh, sorry, I can't read.

@kentonv
Copy link
Member

kentonv commented Jan 20, 2023

Can we also update .bazelrc so that this produces useful results? Right now I get no line numbers because by default we compile without line number info. Should we add -gline-tables-only to the default config?

@kentonv
Copy link
Member

kentonv commented Jan 20, 2023

Hmm, this didn't seem to have any effect in my bazelrc... build --cxxopt='-gline-tables-only' --host_cxxopt='-gline-tables-only'

@mikea
Copy link
Contributor Author

mikea commented Jan 20, 2023

bazel's stripping is weird. It worked for me only with -c dbg.

https://bazel.build/docs/user-manual#strip

@kentonv
Copy link
Member

kentonv commented Jan 20, 2023

-c dbg produces prohibitively large binaries for me. -gline-tables-only combined with --strip=never seems to do the trick.

@kentonv
Copy link
Member

kentonv commented Jan 20, 2023 •
edited
Loading

Here's what I added to .bazelrc:

# Define a config mode which is fastbuild but with basig debug info.
#
# Switching to full debug mode for debugging doesn't really work because debug mode produces
# insanely large binaries.
build:fastdbg -c fastbuild
build:fastdbg --cxxopt='-gline-tables-only' --host_cxxopt='-gline-tables-only'
build:fastdbg --linkopt='-gline-tables-only' --host_linkopt='-gline-tables-only'
build:fastdbg --strip=never

I'd prefer to actually override the behavior of fastbuild mode but couldn't figure out how to do that with bazelrc.

@mikea mikea requested a review from kentonv January 20, 2023 22:48
kentonv
kentonv approved these changes Jan 21, 2023
View reviewed changes
Copy link
Member

@kentonv kentonv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I already used this today, it was quite helpful.

jasnell
jasnell reviewed Jan 21, 2023
View reviewed changes
.bazelrc Outdated Show resolved Hide resolved
@mikea mikea merged commit fd7d179 into main Jan 23, 2023
@mikea mikea deleted the maizatskyi/2023-01-19-symbolizer branch January 23, 2023 16:40
ObsidianMinor added a commit to ObsidianMinor/workerd that referenced this pull request Jan 24, 2023
@ObsidianMinor
Revert "stack-trace symbolizer using llvm-symbolizer subprocess (clou…
69e285b 
…dflare#297)"

This reverts commit fd7d179.
@ObsidianMinor ObsidianMinor mentioned this pull request Jan 24, 2023
Merged
mikea added a commit that referenced this pull request Jan 25, 2023
@mikea
Revert "Revert "stack-trace symbolizer using llvm-symbolizer subproce…
fafcb11 
…ss (#297)""

This reverts commit 69e285b.
mikea added a commit that referenced this pull request Jan 25, 2023
@mikea
reland stack-trace symbolizer with ekam protection (#318)
a00c6e0 
* Revert "Revert "stack-trace symbolizer using llvm-symbolizer subprocess (#297)""

This reverts commit 69e285b.

* disabling symbolizer in ekam
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jasnell jasnell jasnell approved these changes

@kentonv kentonv kentonv approved these changes

@harrishancock harrishancock Awaiting requested review from harrishancock

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@mikea @kentonv @jasnell