bpftrace icon indicating copy to clipboard operation
bpftrace copied to clipboard

Add for-each loops for iterating over map elements

Open ajor opened this issue 1 year ago • 4 comments

Basic support for #2955. Support for sharing variables will be added in a separate PR: #3014

For loops are implemented using the bpf_for_each_map_elem helper function, which requires them to be rewritten into a callback style.

In this first implementation, we do not allow any variables to be shared between the main probe and the loop's body. Maps are global so are not a problem.

Pseudo code for the transformation we apply:

Before:

PROBE {
  @map[0] = 1;
  for ($kv : @map) {
    [LOOP BODY]
  }
}

After:

PROBE {
  @map[0] = 1;
  bpf_for_each_map_elem(@map, &map_for_each_cb, 0, 0);
}
long map_for_each_cb(bpf_map *map, const void *key, void *value, void *ctx) {
  $kv = ((uint64)key, (uint64)value);
  [LOOP BODY]
}
Checklist
  • [x] Language changes are updated in man/adoc/bpftrace.adoc and if needed in docs/reference_guide.md
  • [x] User-visible and non-trivial changes updated in CHANGELOG.md
  • [x] The new behaviour is covered by tests

ajor avatar Feb 12 '24 19:02 ajor

Overall this is looking good! Few minor nits/questions.

jordalgo avatar Feb 23 '24 19:02 jordalgo

Addressed most of the comments. Still need to add tests for multiple for-loops and fix the nested for-loop bug.

ajor avatar Feb 26 '24 15:02 ajor

Added tests for multiple loops in various scenarios.

Nested loops requires another relocation fix: #3020 Loops used from two different main programs is causing yet another error!

number of funcs in func_info doesn't match number of subprogs

This one appears to have been around for longer and affects the len() builtin as well: BEGIN { @map[0] = 1; print(len(@map)) } END { print(len(@map)) }

ajor avatar Feb 26 '24 17:02 ajor

The loops-in-multiple-probes issue is pretty nasty: #3021

ajor avatar Feb 26 '24 18:02 ajor

I would like to take a look but I'll be busy until ~Tuesday.

danobi avatar Feb 29 '24 19:02 danobi

Also codegen tests need update after #2985 was merged.

viktormalik avatar Mar 01 '24 07:03 viktormalik

Addressed review comments and added extra runtime test for deleting elements while iterating over the map.

ajor avatar Mar 13 '24 11:03 ajor

Fixed a lifetime bug I'd introduced for tuple elements: in CodegenLLVM::visit(Tuple &tuple) I'm now extending the lifetime of all tuple elements until the end of the function with:

scoped_dels.emplace_back(std::move(accept(elem)));

Thanks @arthurshau for the new test case which exposed this! (call.print_map_item_tuple).

Will merge tomorrow if there are no objections.

ajor avatar Mar 14 '24 20:03 ajor

A lot of the error messages I'm testing for in this PR appear to have regressed: #3063

ajor avatar Mar 15 '24 10:03 ajor

Updated the tests to accept the incorrect expected results and left comments referring to #3063

ajor avatar Mar 15 '24 13:03 ajor