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(ebpf): fix issue when a cls arg is a cell #3280

Merged
merged 1 commit into from
May 8, 2024

Conversation

korniltsev
Copy link
Collaborator

@korniltsev korniltsev commented May 7, 2024

Fixes "invalid utf string" issue in our dev cluster.

The issue was caused by ebpf profiler type confusing - trying to use PyCellObject as PyTypeObject

In addition fixes issue when first argument name is incorrectly detected by zeroing the arg name buffer. (previously if could contain garbage from previous frame)

@korniltsev korniltsev requested a review from simonswine May 7, 2024 09:55
Copy link
Contributor

@simonswine simonswine left a comment

Choose a reason for hiding this comment

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

LGTM

Couldn't find a regression, with a quick happy path check. And I also spotted this for the first time 🙂

// Figure out if we want to parse class name, basically checking the name of
// the first argument,
// ((PyTupleObject*)$frame->f_code->co_varnames)->ob_item[0]
// If it's 'self', we get the type and it's name, if it's cls, we just get
// the name. This is not perfect but there is no better way to figure this
// out from the code object.

@korniltsev
Copy link
Collaborator Author

@korniltsev
Copy link
Collaborator Author

And I also spotted this for the first time 🙂

Yesterday I found another issue with the way the code detects class name.

f_code->co_varnames may contain same name multiple times. It may look like ('self', 'foo', 'bar', 'self') and f_localsplus[0] == NULL and f_localsplus[3] == 0x7f....

@korniltsev korniltsev merged commit 6e7a297 into main May 8, 2024
27 checks passed
@korniltsev korniltsev deleted the ebpf-cell-cls-issue-fix branch May 8, 2024 04:28
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