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

Handle 64-bit negative addresses when adjusting them. #397

Merged
merged 1 commit into from
Jul 9, 2018

Conversation

aalexand
Copy link
Collaborator

@aalexand aalexand commented Jul 2, 2018

Fixes #280. 64-bit addresses with the high bit set should be adjustable
as long as the offset does not overflow the result. This change
supersedes #396 avoiding math/big dependency.

Fixes google#280. 64-bit addresses with the high bit set should be adjustable
as long as the offset does not overflow the result. This change
supersedes google#396 avoiding `math/big` dependency.
@codecov-io
Copy link

Codecov Report

Merging #397 into master will increase coverage by 0.03%.
The diff coverage is 76.47%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #397      +/-   ##
=========================================
+ Coverage   67.67%   67.7%   +0.03%     
=========================================
  Files          36      36              
  Lines        7535    7544       +9     
=========================================
+ Hits         5099    5108       +9     
  Misses       2031    2031              
  Partials      405     405
Impacted Files Coverage Δ
internal/symbolz/symbolz.go 80% <76.47%> (+2.22%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2b5d435...4d23e37. Read the comment docs.

@nolanmar511 nolanmar511 merged commit 2c569c7 into google:master Jul 9, 2018
gmarin13 pushed a commit to gmarin13/pprof that referenced this pull request Dec 17, 2020
Fixes google#280. 64-bit addresses with the high bit set should be adjustable
as long as the offset does not overflow the result. This change
supersedes google#396 avoiding `math/big` dependency.
aalexand added a commit to aalexand/pprof that referenced this pull request Oct 18, 2024
We added skipping unsymbolizable mappings in symbolz long time ago in
PR google#368 to address google#339 where we saw error like "unexpected negative
adjusted address".

But that error was fixed in a more proper way in subsequent google#397 to fix
another reported issue google#280 (and internal b/32020573). So skipping
unsymbolized mappings shouldn't be needed anymore.

I tested this by verifying that the test case from google#339 still works fine
with the proposed change. And that it does fail if I roll back google#397
locally.

This change is useful as we experiment with using symbolz to symbolize
JIT locations from //anon (which is unsymbolizable per current terminology).
aalexand added a commit that referenced this pull request Oct 23, 2024
We added skipping unsymbolizable mappings in symbolz long time ago in
PR #368 to address #339 where we saw error like "unexpected negative
adjusted address".

But that error was fixed in a more proper way in subsequent #397 to fix
another reported issue #280 (and internal b/32020573). So skipping
unsymbolized mappings shouldn't be needed anymore.

I tested this by verifying that the test case from #339 still works fine
with the proposed change. And that it does fail if I roll back #397
locally.

This change is useful as we experiment with using symbolz to symbolize
JIT locations from //anon (which is unsymbolizable per current terminology).

Co-authored-by: Maggie Nolan Edmonds <nolanmar@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants