On Thu, 2022-11-10 at 17:34 -0800, sdf@xxxxxxxxxx wrote: > On 11/10, Stanislav Fomichev wrote: > > On 11/11, Eduard Zingerman wrote: > > > A fix for the LLVM compilation error while building bpftool. > > > Replaces the expression: > > > > > > _Static_assert((p) == NULL || ...) > > > > > > by expression: > > > > > > _Static_assert((__builtin_constant_p((p)) ? (p) == NULL : 0) || ...) > > > IIUC, when __builtin_constant_p(p) returns false, we just ignore the NULL > > check? > > Do we have cases like that? If no, maybe it's safer to fail? > > > s/(p) == NULL : 0/(p) == NULL : 1/ ? > > I'm probably missing something, can you pls clarify? So the error is as > follows: > > > > btf_dump.c:1546:2: error: static_assert expression is not an integral > > > constant expression > hashmap__find(name_map, orig_name, &dup_cnt); > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > ./hashmap.h:169:35: note: expanded from macro 'hashmap__find' > hashmap_find((map), (long)(key), hashmap_cast_ptr(value)) > ^~~~~~~~~~~~~~~~~~~~~~~ > ./hashmap.h:126:17: note: expanded from macro 'hashmap_cast_ptr' > _Static_assert((p) == NULL || sizeof(*(p)) == > sizeof(long), > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > btf_dump.c:1546:2: note: cast from 'void *' is not allowed in a constant > expression > ./hashmap.h:169:35: note: expanded from macro 'hashmap__find' > hashmap_find((map), (long)(key), hashmap_cast_ptr(value)) > > This line in particular: > > btf_dump.c:1546:2: note: cast from 'void *' is not allowed in a constant > expression > > And the code does: > > size_t dup_cnt = 0; > hashmap__find(name_map, orig_name, &dup_cnt); > > So where is that cast from 'void *' is happening? Is it the NULL check > itself? The void * comes from the definition of NULL: ((void*)0). And, unfortunately, sizeoof(*((void*)0)) == 1, so two conditions are necessary to allow a typical use-case when some of the pointer parameters are omitted. > > Are we simply guarding against the user calling hashmap_cast_ptr with > explicit NULL argument? > > > > When "p" is not a constant the former is not considered to be a > > > constant expression by LLVM 14. > > > > > > The error was introduced in the following patch-set: [1]. > > > The error was reported here: [2]. > > > > > > Reported-by: kernel test robot <lkp@xxxxxxxxx> > > > Signed-off-by: Eduard Zingerman <eddyz87@xxxxxxxxx> > > > > > > [1] > > https://lore.kernel.org/bpf/20221109142611.879983-1-eddyz87@xxxxxxxxx/ > > > [2] https://lore.kernel.org/all/202211110355.BcGcbZxP-lkp@xxxxxxxxx/ > > > --- > > > tools/lib/bpf/hashmap.h | 3 ++- > > > tools/perf/util/hashmap.h | 3 ++- > > > 2 files changed, 4 insertions(+), 2 deletions(-) > > > > > > diff --git a/tools/lib/bpf/hashmap.h b/tools/lib/bpf/hashmap.h > > > index 3fe647477bad..0a5bf1937a7c 100644 > > > --- a/tools/lib/bpf/hashmap.h > > > +++ b/tools/lib/bpf/hashmap.h > > > @@ -123,7 +123,8 @@ enum hashmap_insert_strategy { > > > }; > > > > > > #define hashmap_cast_ptr(p) ({ \ > > > - _Static_assert((p) == NULL || sizeof(*(p)) == sizeof(long), \ > > > + _Static_assert((__builtin_constant_p((p)) ? (p) == NULL : 0) || \ > > > + sizeof(*(p)) == sizeof(long), \ > > > #p " pointee should be a long-sized integer or a pointer"); \ > > > (long *)(p); \ > > > }) > > > diff --git a/tools/perf/util/hashmap.h b/tools/perf/util/hashmap.h > > > index 3fe647477bad..0a5bf1937a7c 100644 > > > --- a/tools/perf/util/hashmap.h > > > +++ b/tools/perf/util/hashmap.h > > > @@ -123,7 +123,8 @@ enum hashmap_insert_strategy { > > > }; > > > > > > #define hashmap_cast_ptr(p) ({ \ > > > - _Static_assert((p) == NULL || sizeof(*(p)) == sizeof(long), \ > > > + _Static_assert((__builtin_constant_p((p)) ? (p) == NULL : 0) || \ > > > + sizeof(*(p)) == sizeof(long), \ > > > #p " pointee should be a long-sized integer or a pointer"); \ > > > (long *)(p); \ > > > }) > > > -- > > > 2.34.1 > > >
