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

bug-1907983: fix sym file header parsing #2991

Merged
merged 1 commit into from
Aug 19, 2024

Conversation

willkg
Copy link
Collaborator

@willkg willkg commented Aug 14, 2024

This fixes sym file header parsing so it parses the entire sym file header rather than stopping after INFO CODE_ID.

While doing this, I also updated the tests for windows, mac, and linux sym file headers with dump_syms 2.3.3 output.

This fixes sym file header parsing so it parses the entire sym file
header rather than stopping after INFO CODE_ID.

While doing this, I also updated the tests for windows, mac, and linux
sym file headers with dump_syms 2.3.3 output.
@willkg willkg marked this pull request as ready for review August 19, 2024 13:16
@willkg willkg requested a review from a team as a code owner August 19, 2024 13:16
@willkg willkg requested a review from relud August 19, 2024 13:17
Copy link
Member

@relud relud left a comment

Choose a reason for hiding this comment

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

r+wc

Comment on lines +41 to +45
if len(parts) == 3:
_, _, code_id = parts
code_file = ""
elif len(parts) == 4:
_, _, code_id, code_file = parts
Copy link
Member

Choose a reason for hiding this comment

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

seems like this should have an else ValueError condition in case the file is corrupted or something

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

When I initially wrote the extraction code, I thought about validation a bit, but decided that's outside of the scope. This code parses the bits it understands so it can fill in some metadata. If the header is malformed or invalid in some way, we don't have any mechanisms for doing anything with that knowledge. Given that, I wrote it such that it just ignores those cases.

Bug 1814430 covers writing a better information view for symbols files which would cover whether they're valid. Adding header validation to that would be helpful.

A while back, I wrote bin/debug-sym-file.py that downloads a file and tells you things about it including whether it parses in symbolic. I used that to debug malformed symbols files for bug 1791785. We could add header validation to that.

@willkg willkg merged commit 1362e1f into main Aug 19, 2024
2 checks passed
@willkg willkg deleted the willkg-bug-1907985-header-parsing branch August 19, 2024 15:57
@willkg
Copy link
Collaborator Author

willkg commented Aug 19, 2024

Thank you!

@willkg willkg changed the title bug-1907985: fix sym file header parsing bug-1907983: fix sym file header parsing Aug 27, 2024
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.

2 participants