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

Add support for inlinee lines #47

Merged
merged 32 commits into from
Sep 5, 2019
Merged

Conversation

mitsuhiko
Copy link
Member

@mitsuhiko mitsuhiko commented Jul 20, 2019

Refactors symbol parsing, introduces support for inlinees and parsing of their binary annotations into line records.

Breaking Changes:

  • Symbol::name has been removed, since not all symbols have a name. Instead, every symbol with a name now exposes a name field. To provide uniform access to the name, SymbolData::name now exposes an Option<RawString> after parsing. (NB: This felt more robust than maintaining a manual list of offsets)
  • SymbolData::Public and SymbolData::Data have been stripped of their *Symbol prefix, as this is redundant and inconsistent.
  • Many symbol kinds, most notablySymbolData::CompileFlags, have been fixed with stronger typing.
  • TypeIndex is now a newtype around u32, and so is IdIndex. This is in preparation of supporting the IPI stream. I'm yet undecided whether to expose the IPI in a separate type or via directly TypeFinder.

Public Changes:

  • Symbol::starts_scope and Symbol::ends_scope now indicate which symbols can contain sub-symbols. This helps to deal with a hierarchy of symbols, such as building a tree.
  • Every Symbol now exposes an index. Also, some symbols may refer to others via that index (e.g. inline sites point to their parent and their end). SymbolTable, ModuleInfo and SymbolIter now allow jumping to symbols by their index.
  • Inline Sites are now supported via SymbolData::InlineSite(InlineSiteSymbol). It exposes an iterator over its binary annotations, which can be used to evaluate the line program of the inline site.
  • LineProgram::inlinees allows iteration over inlinees in a module. Each inlinee has an inlinee_lines function that returns an iterator over line records evaluated from the binary annotations in the inline site symbol.

Internal Changes:

  • Pread is now implemented for more types, including: Variant, CPUType, SourceLanguage.
  • ParseBuffer can now parse any type that implements TryFromCtx, not just the ones that use scroll::Endian as context. There's also a new method parse_with to pass a custom parse context.
  • SectionOffset types now implement AddAssign which has the same semantics as Add.

Fixes #14

@jan-auer
Copy link
Member

This is not yet complete, but will contain a full refactor of Symbols to fix #14. Additionally, this helps to expose binary annotations more neatly.

@jan-auer
Copy link
Member

jan-auer commented Aug 2, 2019

Edit: Updated PR description.

@mitsuhiko mitsuhiko marked this pull request as ready for review September 2, 2019 07:41
* master:
  Fix clippy lints
  Add alignment tests
  Fix an invalid check when aligning buffers
@jan-auer
Copy link
Member

jan-auer commented Sep 3, 2019

@willglynn This is ready for review now. See #47 (comment) for a summary of all changes.

@jan-auer
Copy link
Member

jan-auer commented Sep 3, 2019

When working on the next feature (Cross Module Imports/Exports), I realized that inlinees are probably better placed on the module info directly than the line program.

src/symbol/annotations.rs Outdated Show resolved Hide resolved
src/symbol/annotations.rs Show resolved Hide resolved
src/symbol/mod.rs Outdated Show resolved Hide resolved
Copy link
Collaborator

@willglynn willglynn left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks! 🙌

@jan-auer jan-auer merged commit 1fc32ab into getsentry:master Sep 5, 2019
@jan-auer jan-auer deleted the feature/inlinees branch September 5, 2019 10:59
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.

Not all symbol records have names
3 participants