| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAEf4BzYCOpwnTVGKs8rHE4CdcQHDU0ButEKHnJE5cqcWpdZwWw@mail.gmail.com>
Date: Fri, 11 Oct 2024 11:00:56 -0700
From: Andrii Nakryiko <andrii.nakryiko@...il.com>
To: Namhyung Kim <namhyung@...nel.org>
Cc: Arnaldo Carvalho de Melo <acme@...nel.org>, Ian Rogers <irogers@...gle.com>,
Kan Liang <kan.liang@...ux.intel.com>, Jiri Olsa <jolsa@...nel.org>,
Adrian Hunter <adrian.hunter@...el.com>, Peter Zijlstra <peterz@...radead.org>,
Ingo Molnar <mingo@...nel.org>, LKML <linux-kernel@...r.kernel.org>,
linux-perf-users@...r.kernel.org, bpf@...r.kernel.org
Subject: Re: [PATCH 1/2] perf tools: Fix possible compiler warnings in hashmap
On Fri, Oct 11, 2024 at 9:44 AM Namhyung Kim <namhyung@...nel.org> wrote:
>
> On Thu, Oct 10, 2024 at 06:48:26PM -0700, Andrii Nakryiko wrote:
> > On Wed, Oct 9, 2024 at 1:20 PM Namhyung Kim <namhyung@...nel.org> wrote:
> > >
> > > The hashmap__for_each_entry[_safe] is accessing 'map' as if it's a
> > > pointer. But it does without parentheses so passing a static hash map
> > > with an ampersand (like &slab_hash below) caused compiler warnings due
> > > to unmatched types.
> > >
> > > In file included from util/bpf_lock_contention.c:5:
> > > util/bpf_lock_contention.c: In function ‘exit_slab_cache_iter’:
> > > linux/tools/perf/util/hashmap.h:169:32: error: invalid type argument of ‘->’ (have ‘struct hashmap’)
> > > 169 | for (bkt = 0; bkt < map->cap; bkt++) \
> > > | ^~
> > > util/bpf_lock_contention.c:105:9: note: in expansion of macro ‘hashmap__for_each_entry’
> > > 105 | hashmap__for_each_entry(&slab_hash, cur, bkt)
> > > | ^~~~~~~~~~~~~~~~~~~~~~~
> > > /home/namhyung/project/linux/tools/perf/util/hashmap.h:170:31: error: invalid type argument of ‘->’ (have ‘struct hashmap’)
> > > 170 | for (cur = map->buckets[bkt]; cur; cur = cur->next)
> > > | ^~
> > > util/bpf_lock_contention.c:105:9: note: in expansion of macro ‘hashmap__for_each_entry’
> > > 105 | hashmap__for_each_entry(&slab_hash, cur, bkt)
> > > | ^~~~~~~~~~~~~~~~~~~~~~~
> > >
> > > Cc: bpf@...r.kernel.org
> > > Signed-off-by: Namhyung Kim <namhyung@...nel.org>
> > > ---
> > > I've discovered this while prototyping the slab symbolization for perf
> > > lock contention. So this code is not available yet but I'd like to fix
> > > the problem first.
> > >
> > > Also noticed bpf has the same code and the same problem. I'll send a
> > > separate patch for them.
> > >
> >
> > Yep, please do. Fixes look good, thanks.
>
> Sure will do, can I get your Acked-by for this patch?
>
Sure:
Acked-by: Andrii Nakryiko <andrii@...nel.org>
> Thanks,
> Namhyung
>
> >
> > > tools/perf/util/hashmap.h | 20 ++++++++++----------
> > > 1 file changed, 10 insertions(+), 10 deletions(-)
> > >
> > > diff --git a/tools/perf/util/hashmap.h b/tools/perf/util/hashmap.h
> > > index c12f8320e6682d50..0c4f155e8eb745d9 100644
> > > --- a/tools/perf/util/hashmap.h
> > > +++ b/tools/perf/util/hashmap.h
> > > @@ -166,8 +166,8 @@ bool hashmap_find(const struct hashmap *map, long key, long *value);
> > > * @bkt: integer used as a bucket loop cursor
> > > */
> > > #define hashmap__for_each_entry(map, cur, bkt) \
> > > - for (bkt = 0; bkt < map->cap; bkt++) \
> > > - for (cur = map->buckets[bkt]; cur; cur = cur->next)
> > > + for (bkt = 0; bkt < (map)->cap; bkt++) \
> > > + for (cur = (map)->buckets[bkt]; cur; cur = cur->next)
> > >
> > > /*
> > > * hashmap__for_each_entry_safe - iterate over all entries in hashmap, safe
> > > @@ -178,8 +178,8 @@ bool hashmap_find(const struct hashmap *map, long key, long *value);
> > > * @bkt: integer used as a bucket loop cursor
> > > */
> > > #define hashmap__for_each_entry_safe(map, cur, tmp, bkt) \
> > > - for (bkt = 0; bkt < map->cap; bkt++) \
> > > - for (cur = map->buckets[bkt]; \
> > > + for (bkt = 0; bkt < (map)->cap; bkt++) \
> > > + for (cur = (map)->buckets[bkt]; \
> > > cur && ({tmp = cur->next; true; }); \
> > > cur = tmp)
> > >
> > > @@ -190,19 +190,19 @@ bool hashmap_find(const struct hashmap *map, long key, long *value);
> > > * @key: key to iterate entries for
> > > */
> > > #define hashmap__for_each_key_entry(map, cur, _key) \
> > > - for (cur = map->buckets \
> > > - ? map->buckets[hash_bits(map->hash_fn((_key), map->ctx), map->cap_bits)] \
> > > + for (cur = (map)->buckets \
> > > + ? (map)->buckets[hash_bits((map)->hash_fn((_key), (map)->ctx), (map)->cap_bits)] \
> > > : NULL; \
> > > cur; \
> > > cur = cur->next) \
> > > - if (map->equal_fn(cur->key, (_key), map->ctx))
> > > + if ((map)->equal_fn(cur->key, (_key), (map)->ctx))
> > >
> > > #define hashmap__for_each_key_entry_safe(map, cur, tmp, _key) \
> > > - for (cur = map->buckets \
> > > - ? map->buckets[hash_bits(map->hash_fn((_key), map->ctx), map->cap_bits)] \
> > > + for (cur = (map)->buckets \
> > > + ? (map)->buckets[hash_bits((map)->hash_fn((_key), (map)->ctx), (map)->cap_bits)] \
> > > : NULL; \
> > > cur && ({ tmp = cur->next; true; }); \
> > > cur = tmp) \
> > > - if (map->equal_fn(cur->key, (_key), map->ctx))
> > > + if ((map)->equal_fn(cur->key, (_key), (map)->ctx))
> > >
> > > #endif /* __LIBBPF_HASHMAP_H */
> > > --
> > > 2.47.0.rc0.187.ge670bccf7e-goog
> > >
> > >
Powered by blists - more mailing lists