Skip to content

[BOLT] Exit early when prepareToParse fails on buildid #161151

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
Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

[BOLT] Exit early when prepareToParse fails on buildid #161151

wants to merge 2 commits into from
+12 −1

Conversation

paschalis-mpeis
Copy link
Member

No description provided.

@paschalis-mpeis
[BOLT] Exit early when prepareToParse fails on buildid
bfd0874
@paschalis-mpeis
Avoid prepareToParse crash
066ebee 
I stumbled upon a case where perpareToParse return an error message on
'buildid', but then on a subsequent call for 'mmap events' it crashed
with:
> Assertion `!HasError && "Cannot get value when an error exists!"' failed.

This was due to an Error while reading the error message (ErrorMB).
@paschalis-mpeis
Copy link
Member Author

This corcerns and edge case, causing an assertion in 'prepareToParse'.
I was on Ubuntu without perf installed; instead there's a wrapper that prints installation instructions.

perf2bolt assumes perf is present. With one profile type it handled it okay, exiting gradefully with the message.
With another it crashed on the second prepareToParse (for mmaps).

I'd expect that if fetching the build-id failed, the subsequent call would also fail.
Note that when the binary lacks build-id, we have no issues, just a warning:

BOLT-WARNING: build-id will not be checked because we could not read one from input binary

Either way, at prepareToParse we get an error while trying to read the error message.
If we handle that, then the assertion will be gone.

I've sent a second draft patch as an alternative to the first (066ebee).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@aaupov aaupov Awaiting requested review from aaupov aaupov will be requested when the pull request is marked ready for review aaupov is a code owner

@maksfb maksfb Awaiting requested review from maksfb maksfb will be requested when the pull request is marked ready for review maksfb is a code owner

@rafaelauler rafaelauler Awaiting requested review from rafaelauler rafaelauler will be requested when the pull request is marked ready for review rafaelauler is a code owner

@ayermolo ayermolo Awaiting requested review from ayermolo ayermolo will be requested when the pull request is marked ready for review ayermolo is a code owner

@yota9 yota9 Awaiting requested review from yota9 yota9 will be requested when the pull request is marked ready for review yota9 is a code owner

@yozhu yozhu Awaiting requested review from yozhu yozhu will be requested when the pull request is marked ready for review yozhu is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@paschalis-mpeis