Implement strstr

[liutingjieni]

Add a new built-in func like strstr()

Add the strstr () function to make fuzzy comparisons when there is no way to confirm the absolute path of the file or the full name of the process or

usage:
sudo bpftrace -e 't:syscalls:sys_enter_execve /strstr(comm,"sh") == 0/ { printf("%s called %s\n", comm, str(args->filename));}'

Problem background: #2306

Checklist

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

[liutingjieni]

Pr was here before ( add strstr() by liutingjieni · Pull Request #2344 · iovisor/bpftrace ). Because of the conflict with master, so based on the latest master commit changes

[liutingjieni]

I see a check error, but I don't quite understand。what do I need to do now?

[viktormalik]

Pr was here before ( add strstr() by liutingjieni · Pull Request #2344 · iovisor/bpftrace ). Because of the conflict with master, so based on the latest master commit changes

In future, please keep updating the same PR (by force-push into the branch), opening a new one after each rebase is not a good idea.

I see a check error, but I don't quite understand。what do I need to do now?

Your new test is failing with segmentation fault, so you'll have to debug. If you want to reproduce the CI environment, you can build and use the prepared Docker images. Check .github/workflows/ci.yml for exact Docker commands.

[liutingjieni]

I found that the error is build_test (LLVM 15 Release). I tried to remove the commit about it( Latest Commit) and I found that it works.

[viktormalik]

I found that the error is build_test (LLVM 15 Release). I tried to remove the commit about it( Latest Commit) and I found that it works.

Which could mean that your code doesn't work with LLVM 15 (very likely it doesn't work with opaque pointers). Please check and fix that.

[liutingjieni]

all checks have passed. what else do I need to do now?

[viktormalik]

This is a terrible solution. You can't just arbitrarily revert a commit from master if it doesn't suit you.

You have to make your code work with opaque pointers. Once you do that, the checks will pass.

[liutingjieni]

This is a terrible solution. You can't just arbitrarily revert a commit from master if it doesn't suit you.

You have to make your code work with opaque pointers. Once you do that, the checks will pass.

I have changed my code with opaque pointers. The check for llvm15 has passed. The last two commit hashes from master have changed due to cherry-pick, but the code content has not changed.

[liutingjieni]

This branch is 1 commit ahead of iovisor:master.

[liutingjieni]

I want the code to be reviewed

[viktormalik]

I have changed my code with opaque pointers. The check for llvm15 has passed. The last two commit hashes from master have changed due to cherry-pick, but the code content has not changed.

Ah, right, sorry. I'll review when I have time.

[BurntBrunch]

This looks fine for what it is but I have a meta question - why are we unrolling the loop?

For a char[32] vs char[32] call, this emits ~500 instructions.

> src/bpftrace -vv -e 't:mac80211:wake_queue {if (strstr(ar
gs->wiphy_name, args->wiphy_name)) {exit()}}'
INFO: node count: 13
Attaching 1 probe...

Program ID: 1422

The verifier log: 
processed 522 insns (limit 1000000) max_states_per_insn 0 total_states 32 peak_states 32 mark_read 24

Given that bpf has had backward branches for ages, why don't we just emit this as an actual loop with known bounds and let the verifier do the hard work? It would dramatically improve complexity while potentially also allowing probed strings to work (by emitting the loop against the probe size limit or even against the runtime string length, assuming we can satisfy the verifier).

[fbs]

This looks fine for what it is but I have a meta question - why are we unrolling the loop?

For a char[32] vs char[32] call, this emits ~500 instructions.

> src/bpftrace -vv -e 't:mac80211:wake_queue {if (strstr(ar
gs->wiphy_name, args->wiphy_name)) {exit()}}'
INFO: node count: 13
Attaching 1 probe...

Program ID: 1422

The verifier log: 
processed 522 insns (limit 1000000) max_states_per_insn 0 total_states 32 peak_states 32 mark_read 24

Given that bpf has had backward branches for ages, why don't we just emit this as an actual loop with known bounds and let the verifier do the hard work? It would dramatically improve complexity while potentially also allowing probed strings to work (by emitting the loop against the probe size limit or even against the runtime string length, assuming we can satisfy the verifier).

Agree but maybe we shouldn't block on it.

For a future improvement it would also be nice if we compare more bytes at once

[viktormalik]

Could you also merge the two commits and cleanup the commit message?
Thanks!

[viktormalik]

Since there has been a lot of work, I'd like to have one more review before merging this.

[viktormalik]

cc'ing @danobi and @ajor explicitly since this was opened before we added the CODEOWNERS file.

[xh4n3]

Since this is similar to #2387, I guess you may need to add the codegen tests and change the CHANELOG.md.
And maybe a little change to the empty lines, see #2387 (comment)

[fbs]

looks good! Just a few suggestions on the testing side