| Message ID | 20220726220921.2567761-1-irogers@google.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Delegated to: | BPF |
| Headers | show |
| Series | perf bpf: Remove undefined behavior from bpf_perf_object__next | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Not a local patch |
| bpf/vmtest-bpf-next-PR | success | PR summary |
| bpf/vmtest-bpf-next-VM_Test-2 | success | Logs for Kernel LATEST on Array with gcc |
| bpf/vmtest-bpf-next-VM_Test-3 | success | Logs for Kernel LATEST on Array with llvm-15 |
| bpf/vmtest-bpf-next-VM_Test-1 | success | Logs for Kernel LATEST on Array with gcc |
Em Tue, Jul 26, 2022 at 03:09:21PM -0700, Ian Rogers escreveu: > bpf_perf_object__next folded the last element in the list test with the > empty list test. However, this meant that offsets were computed against > null and that a struct list_head was compared against a struct > bpf_perf_object. Working around this with clang's undefined behavior > sanitizer required -fno-sanitize=null and -fno-sanitize=object-size. > in > Remove the undefined behav(ior by using the regular Linux list APIs and > handling the starting case separately from the end testing case. Looking > at uses like bpf_perf_object__for_each, as the constant NULL or non-NULL > argument can be constant propagated the code is no less efficient. Nicely spotted! In some places people solve this with list_first_entry_or_null(), like in cs_etm__queue_aux_records(). Applied. - Arnado > Signed-off-by: Ian Rogers <irogers@google.com> > --- > tools/perf/util/bpf-loader.c | 18 +++++++----------- > 1 file changed, 7 insertions(+), 11 deletions(-) > > diff --git a/tools/perf/util/bpf-loader.c b/tools/perf/util/bpf-loader.c > indelx f8ad581ea247..cdd6463a5b68 100644 > --- a/tools/perf/util/bpf-loader.c > +++ b/tools/perf/util/bpf-loader.c > @@ -63,20 +63,16 @@ static struct hashmap *bpf_map_hash; > static struct bpf_perf_object * > bpf_perf_object__next(struct bpf_perf_object *prev) > { > - struct bpf_perf_object *next; > - > - if (!prev) > - next = list_first_entry(&bpf_objects_list, > - struct bpf_perf_object, > - list); > - else > - next = list_next_entry(prev, list); > + if (!prev) { > + if (list_empty(&bpf_objects_list)) > + return NULL; > > - /* Empty list is noticed here so don't need checking on entry. */ > - if (&next->list == &bpf_objects_list) > + return list_first_entry(&bpf_objects_list, struct bpf_perf_object, list); > + } > + if (list_is_last(&prev->list, &bpf_objects_list)) > return NULL; > > - return next; > + return list_next_entry(prev, list); > } > > #define bpf_perf_object__for_each(perf_obj, tmp) \ > -- > 2.37.1.359.gd136c6c3e2-goog
diff --git a/tools/perf/util/bpf-loader.c b/tools/perf/util/bpf-loader.c index f8ad581ea247..cdd6463a5b68 100644 --- a/tools/perf/util/bpf-loader.c +++ b/tools/perf/util/bpf-loader.c @@ -63,20 +63,16 @@ static struct hashmap *bpf_map_hash; static struct bpf_perf_object * bpf_perf_object__next(struct bpf_perf_object *prev) { - struct bpf_perf_object *next; - - if (!prev) - next = list_first_entry(&bpf_objects_list, - struct bpf_perf_object, - list); - else - next = list_next_entry(prev, list); + if (!prev) { + if (list_empty(&bpf_objects_list)) + return NULL; - /* Empty list is noticed here so don't need checking on entry. */ - if (&next->list == &bpf_objects_list) + return list_first_entry(&bpf_objects_list, struct bpf_perf_object, list); + } + if (list_is_last(&prev->list, &bpf_objects_list)) return NULL; - return next; + return list_next_entry(prev, list); } #define bpf_perf_object__for_each(perf_obj, tmp) \
bpf_perf_object__next folded the last element in the list test with the empty list test. However, this meant that offsets were computed against null and that a struct list_head was compared against a struct bpf_perf_object. Working around this with clang's undefined behavior sanitizer required -fno-sanitize=null and -fno-sanitize=object-size. Remove the undefined behavior by using the regular Linux list APIs and handling the starting case separately from the end testing case. Looking at uses like bpf_perf_object__for_each, as the constant NULL or non-NULL argument can be constant propagated the code is no less efficient. Signed-off-by: Ian Rogers <irogers@google.com> --- tools/perf/util/bpf-loader.c | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-)