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

Rawtracepoint support wildcards and list show #2588

Merged

Conversation

zf1575192187
Copy link

1: Rawtracepoint support wildcards and list show
2: Add testcases for rawtracepoint's wildcards and list show

tests/bpftrace.cpp Fixed Show fixed Hide fixed
@zf1575192187 zf1575192187 force-pushed the feature-rt-support-wildcards branch from c95df35 to c0616d7 Compare May 5, 2023 08:32
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.

A runtime test for an actual attachment to a wildcarded raw tracepoint would be useful.

Other than that looks good to me, thanks!

src/ast/attachpoint_parser.cpp Outdated Show resolved Hide resolved
src/probe_matcher.cpp Outdated Show resolved Hide resolved
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.

I just realized that this is not properly handling argX for wildcarded probes.
Example:

# bpftrace -e 'rawtracepoint:mm_* { printf("%d\n", arg1); exit(); }'
Attaching 50 probes...
ERROR: Error attaching probe: rawtracepoint:mm_compaction_kcompactd_sleep

mm_compaction_kcompactd_sleep has only one argument so the verifier rejects to attach the above program to it. We should address this in semantic analyzer and provide a reasonable error message.

@zf1575192187
Copy link
Author

Ok, thanks for helping with the test, I'll check it out.

@zf1575192187
Copy link
Author

Considering that it is difficult to obtain the number of parameters of rawtracepoint,
whether it can be handled simply, and prompt after judging the error

else if (tracing_fd_ == -EINVAL)
      throw std::runtime_error("Error attaching probe: " + probe_.name + ", \
        Maybe access arguments beyond what's available in this tracepoint");

@viktormalik What do you think, or is there a better way

@viktormalik
Copy link
Contributor

Considering that it is difficult to obtain the number of parameters of rawtracepoint, whether it can be handled simply, and prompt after judging the error

else if (tracing_fd_ == -EINVAL)
      throw std::runtime_error("Error attaching probe: " + probe_.name + ", \
        Maybe access arguments beyond what's available in this tracepoint");

@viktormalik What do you think, or is there a better way

Right, I didn't realize that. Checking the error code and printing a hint seems like a reasonable approach to me.

@zf1575192187 zf1575192187 force-pushed the feature-rt-support-wildcards branch 2 times, most recently from 2de7d48 to dc75fc9 Compare May 22, 2023 06:06
@zf1575192187
Copy link
Author

Done

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.

One more nit below, otherwise lgtm, thanks! Also, I'm gonna squash the commits on merging, if you don't mind.

src/attached_probe.cpp Outdated Show resolved Hide resolved
@viktormalik
Copy link
Contributor

Ah, we'll need a CHANGELOG update, too. Thanks!

Feng Zhou added 3 commits May 24, 2023 14:17
Signed-off-by: Feng Zhou <zhoufeng.zf@bytedance.com>
Signed-off-by: Feng Zhou <zhoufeng.zf@bytedance.com>
…e error message

Signed-off-by: Feng Zhou <zhoufeng.zf@bytedance.com>
@zf1575192187
Copy link
Author

Done

@viktormalik viktormalik merged commit fb1e082 into bpftrace:master May 24, 2023
21 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.

None yet

2 participants