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

Support usdt args with index register and scale #1684

Merged
merged 3 commits into from
Feb 9, 2021

Conversation

rsanger
Copy link
Contributor

@rsanger rsanger commented Feb 3, 2021

Adds support usdt args with index register and scale.
Adds tests for all support usdt argument types including register+scale
arguments. The tests also verify the correct sign extension of
signed arguments.
Additionally, this patch corrects sign extension of usdt arguments
in the case of constant and addressed arguments.

Currently, I have disabled 64-bit constant tests as these are waiting on iovisor/bcc#3258

Checklist
  • N/A Language changes are updated in docs/reference_guide.md
  • User-visible and non-trivial changes updated in CHANGELOG.md
  • The new behaviour is covered by tests

Adds tests for all support usdt argument types including register+scale
arguments. The test also verifies the correct sign extension of
signed arguments.
Additionally, corrects sign extension of usdt arguments in the case
of constant and addressed arguments.
@danobi
Copy link
Member

danobi commented Feb 3, 2021

Looks reasonable but didn't look very closely yet. Curious: what kind of code generates usdt probes like this (other than hand crafted test cases)?

@rsanger
Copy link
Contributor Author

rsanger commented Feb 3, 2021

Only GCC (with optimisation -O1 or higher) seems to generate code with index register and scale components.

On occasion, GCC will generate a probe with an index register and scale component when the argument is an element in an array. It seems rare to encounter as you need a case where 1) both the address of the array and index cannot be inferred, and 2) the element is not already loaded into a register.

void update_item(int *array, int index) {
    DTRACE_PROBE1(test, probe1, array[index]);
}

As written will trigger it. However, to trigger it in actual code you need the right circumstance where array[index] is not loaded into a register.

Copy link
Member

@danobi danobi left a comment

Choose a reason for hiding this comment

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

LGTM

Could you fix up the clang-format error? Thanks

Remove blank line at end of file
@rsanger
Copy link
Contributor Author

rsanger commented Feb 9, 2021

No problem, done, I missed that one.

@danobi danobi merged commit dcc3952 into bpftrace:master Feb 9, 2021
casparant added a commit to casparant/bpftrace that referenced this pull request Feb 10, 2021
* 'master' of github.com:iovisor/bpftrace:
  Support usdt args with index register and scale (bpftrace#1684)
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