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 Position Independent Code #252

Merged
merged 8 commits into from Mar 3, 2022

Conversation

kakkoyun
Copy link
Member

@kakkoyun kakkoyun commented Feb 22, 2022

Signed-off-by: Kemal Akkoyun kakkoyun@gmail.com

Fixes parca-dev/parca#326

also fixes #256 and #264

  • Add support for PIC
  • Refactor debuginfo package
  • Fix a couple of linter errors

@kakkoyun kakkoyun marked this pull request as ready for review February 24, 2022 14:56
@kakkoyun kakkoyun changed the title WIP: Add support for Position Independent Code Add support for Position Independent Code Feb 24, 2022
pkg/debuginfo/cache.go Outdated Show resolved Hide resolved
pkg/debuginfo/cache.go Outdated Show resolved Hide resolved
pkg/objectfile/cache.go Outdated Show resolved Hide resolved
}

func (f *ObjectFile) ObjAddr(addr uint64) (uint64, error) {
f.baseOnce.Do(func() { f.baseErr = f.computeBase(addr) })
Copy link
Member

Choose a reason for hiding this comment

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

we already have the NewObjectFile function that we use to instantiate the struct, let's just do the computation there once

Copy link
Member Author

Choose a reason for hiding this comment

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

In order to compute the base properly we need an address, that's why we need to defer the calculation until we received an address from the profiler.

pkg/maps/mapping.go Outdated Show resolved Hide resolved
pkg/profiler/profiler.go Outdated Show resolved Hide resolved
@@ -257,6 +318,7 @@ func (p *CgroupProfiler) profileLoop(ctx context.Context, now time.Time, counts,

mapping := maps.NewMapping(p.pidMappingFileCache)
kernelMapping := &profile.Mapping{
// TODO(kakkoyun): Check if this conflicts with https://github.com/google/pprof/pull/675/files
Copy link
Member

Choose a reason for hiding this comment

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

nice find!

@kakkoyun
Copy link
Member Author

kakkoyun commented Mar 1, 2022

There is a regression, in the branch. I'm trying to find it. Thus, I've marked it as draft again.

Signed-off-by: Kemal Akkoyun <kakkoyun@gmail.com>
Signed-off-by: Kemal Akkoyun <kakkoyun@gmail.com>
Signed-off-by: Kemal Akkoyun <kakkoyun@gmail.com>
Signed-off-by: Kemal Akkoyun <kakkoyun@gmail.com>
Signed-off-by: Kemal Akkoyun <kakkoyun@gmail.com>
Signed-off-by: Kemal Akkoyun <kakkoyun@gmail.com>
Signed-off-by: Kemal Akkoyun <kakkoyun@gmail.com>
Signed-off-by: Kemal Akkoyun <kakkoyun@gmail.com>
@kakkoyun kakkoyun marked this pull request as ready for review March 3, 2022 08:14
@kakkoyun kakkoyun merged commit a9cc56c into parca-dev:main Mar 3, 2022
@kakkoyun kakkoyun deleted the pic_support branch March 3, 2022 09:22
@Sylfrena Sylfrena linked an issue Mar 7, 2022 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants