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 stack mode for stack builtin #3322

Merged
merged 1 commit into from
Jul 17, 2024

Conversation

jordalgo
Copy link
Contributor

@jordalgo jordalgo commented Jul 17, 2024

The stack mode for kstack and ustack builtin
variables (i.e. without ()) were not respecting
the stack mode value set by the config block.

Example:

bpftrace -e 'config = { stack_mode = raw } kprobe:vfs_read { @ = kstack; exit() }'
bpftrace -e 'config = { stack_mode = raw } kprobe:vfs_read { @[kstack] = 0; exit() }'
bpftrace -e 'config = { stack_mode = raw } kprobe:vfs_read { $s = kstack; print($s); exit() }'

All of these would have the default 'bpftrace'
stack mode instead of 'raw'.

Checklist
  • Language changes are updated in man/adoc/bpftrace.adoc
  • User-visible and non-trivial changes updated in CHANGELOG.md
  • The new behaviour is covered by tests

Copy link
Contributor

@viktormalik viktormalik left a comment

Choose a reason for hiding this comment

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

LGTM, just one comment below.

Also, I don't think that this fixes stack mode specifically for maps or map keys. It rather fixes stack mode for kstack and ustack used as a builtin variable (i.e. without ()).

For instance, all of these are currently broken and should be fixed by this PR:

# bpftrace -e 'config = { stack_mode = raw } kprobe:vfs_read { @ = kstack; exit() }'
# bpftrace -e 'config = { stack_mode = raw } kprobe:vfs_read { @[kstack] = 0; exit() }'
# bpftrace -e 'config = { stack_mode = raw } kprobe:vfs_read { $s = kstack; print($s); exit() }'

Perhaps the tests and the commit message should be adjusted to reflect that.

Also this affects released v0.21.0 so a changelog entry would make sense here.

src/ast/passes/resource_analyser.h Show resolved Hide resolved
@jordalgo
Copy link
Contributor Author

Perhaps the tests and the commit message should be adjusted to reflect that. Also this affects released v0.21.0 so a changelog entry would make sense here.

Yes, good call. Will update both.

@jordalgo jordalgo force-pushed the stack_mode_map_printing branch from a9da163 to 2f6b87b Compare July 17, 2024 09:55
@jordalgo jordalgo changed the title Fix: stack mode printing for maps Fix stack mode for stack builtin Jul 17, 2024
tests/runtime/config Outdated Show resolved Hide resolved
@jordalgo jordalgo force-pushed the stack_mode_map_printing branch from 2f6b87b to 1f04426 Compare July 17, 2024 12:38
@jordalgo jordalgo force-pushed the stack_mode_map_printing branch from 1f04426 to 5780653 Compare July 17, 2024 13:05
The stack mode for kstack and ustack builtin
variables (i.e. without ()) were not respecting
the stack mode value set by the config block.

Example:
```
bpftrace -e 'config = { stack_mode = raw } kprobe:vfs_read { @ = kstack; exit() }'
bpftrace -e 'config = { stack_mode = raw } kprobe:vfs_read { @[kstack] = 0; exit() }'
bpftrace -e 'config = { stack_mode = raw } kprobe:vfs_read { $s = kstack; print($s); exit() }'
```

All of these would have the default 'bpftrace'
stack mode instead of 'raw'.
@jordalgo jordalgo force-pushed the stack_mode_map_printing branch from 5780653 to 7947a9d Compare July 17, 2024 13:43
@jordalgo jordalgo merged commit 5ec55a5 into bpftrace:master Jul 17, 2024
17 checks passed
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.

2 participants