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?
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
>