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

fix(symcache): Compute correct line offsets #319

Merged
merged 2 commits into from
Feb 4, 2021
Merged

Conversation

jan-auer
Copy link
Member

@jan-auer jan-auer commented Feb 2, 2021

SymCaches store line records where record specifies the delta to the previous line record. These deltas are stored as u8 to save space for the common case where only small increments are needed. If two records are more than 0xff apart, we insert filler line records.

This PR solves two bugs in creating these filler line records:

  1. The delta was computed with (diff & 0xff) as u8 instead of diff.min(0xff) as u8. This means that there was an address drift every time we had to insert filler line records.
  2. The filler line records are technically still part of the previous range. However, they already carried information of the next line record, leading to wrong line numbers.

SymCache version is incremented to 6.

The previous implementation originates affe7b9 and 5209ebd and was subsequently amended with split functions in #155.

@jan-auer jan-auer self-assigned this Feb 2, 2021
@jan-auer
Copy link
Member Author

jan-auer commented Feb 2, 2021

The second part of the fix will not work if we are running into should_split_function, since we're exiting the current scope and losing the last_* variables. However in such a case the line records are irrelevant. They are filling a large gap (>65k) where other functions will have better matching line records.

I'm trying to check if we can instead store the size of the line record.

@jan-auer jan-auer marked this pull request as ready for review February 4, 2021 08:24
@jan-auer jan-auer requested a review from a team February 4, 2021 08:24
@jan-auer jan-auer merged commit 86279cb into master Feb 4, 2021
@jan-auer jan-auer deleted the fix/line-offsets branch February 4, 2021 09:35
jan-auer added a commit that referenced this pull request Feb 4, 2021
* master:
  fix(symcache): Compute correct line offsets (#319)
  release: 8.0.3
  meta: Changelog
jan-auer added a commit that referenced this pull request Mar 1, 2021
* master:
  test: add similar-asserts' assert_eq (#333)
  release: 8.0.4
  meta: Changelog for 8.0.4
  build: Drop support for python 2.7 (#328)
  meta(vscode): Fix include paths for C++ (#331)
  doc(debuginfo): Add descriptions of records to breakpad.pest (#329)
  build: Replace virtualenv with venv
  doc(symcache): Symcache documentation
  fix(debuginfo): Support debug_addr indexes in DWARF functions (#326)
  fix(symcache): Fixed bug that caused functions to have len 0 (#324)
  ref(symcache): FuncRecord::len must be nonzero (#323)
  fix: Clippy 1.50 lints (#322)
  fix(symcache): Support lookup for public syms larger than 65k (#320)
  fix(symcache): Compute correct line offsets (#319)
  release: 8.0.3
  meta: Changelog
  build: Update goblin to 0.3.1 (#318)
  fix(elf): Consider sections of type SHT_MIPS_DWARF (#317)
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.

None yet

2 participants