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

symbolz: skip un-symbolizable mappings #340

Closed
wants to merge 2 commits into from

Conversation

toddlipcon
Copy link

This fixes #339 by skipping
attempts to symbolize anything from system mappings like [vdso],
[vsyscall], etc.

@googlebot
Copy link
Collaborator

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here (e.g. I signed it!) and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers
  • Your company has a Point of Contact who decides which employees are authorized to participate. Ask your POC to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the project maintainer to go/cla#troubleshoot.
  • The email used to register you as an authorized contributor must be the email used for the Git commit. Check your existing CLA data and verify that your email is set on your git commits.
  • The email used to register you as an authorized contributor must also be attached to your GitHub account.

@codecov-io
Copy link

codecov-io commented Mar 21, 2018

Codecov Report

Merging #340 into master will decrease coverage by 0.01%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #340      +/-   ##
==========================================
- Coverage   66.54%   66.52%   -0.02%     
==========================================
  Files          36       36              
  Lines        7446     7448       +2     
==========================================
  Hits         4955     4955              
- Misses       2089     2090       +1     
- Partials      402      403       +1
Impacted Files Coverage Δ
internal/symbolz/symbolz.go 77.77% <0%> (-1.97%) ⬇️

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 2c64079...042067c. Read the comment docs.

@@ -45,6 +45,10 @@ func Symbolize(p *profile.Profile, force bool, sources plugin.MappingSources, sy
// Only check for HasFunctions as symbolz only populates function names.
continue
}
// Skip well-known system mappings
Copy link
Collaborator

Choose a reason for hiding this comment

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

Doc for the function should be updated.

@aalexand
Copy link
Collaborator

You also have to sign the CLA.

This fixes google#339 by skipping
attempts to symbolize anything from system mappings like [vdso],
[vsyscall], etc.
@toddlipcon
Copy link
Author

I've been through the CLA process a couple years ago and somehow it got lost. Unfortunately going through it again may take some days of clearance with my company's legal team etc. I was under the understanding that for "trivial bug fix" patches like this, CLAs aren't necessary?

@aalexand
Copy link
Collaborator

I don't know if there is an exemption from CLA based on the patch size, it's not in my domain of expertise so I have to merely follow the automated CLA pre-submit check and can't merge PRs with that check failed.

@aalexand
Copy link
Collaborator

aalexand commented Apr 7, 2018

@toddlipcon Do you plan to proceed with signing the CLA? I guess alternatively I can just make the fix myself if you are fine with that.

@toddlipcon
Copy link
Author

toddlipcon commented Apr 7, 2018 via email

@aalexand
Copy link
Collaborator

aalexand commented May 1, 2018

Superseded by #368.

@aalexand aalexand closed this May 1, 2018
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.

vsyscall mapping with remote gperftools profile causes error
4 participants