* [PATCH bpf-next] libbpf: hashmap.h update to fix build issues using LLVM14 @ 2022-11-10 22:32 Eduard Zingerman 2022-11-11 1:25 ` sdf 2022-11-11 18:40 ` patchwork-bot+netdevbpf 0 siblings, 2 replies; 8+ messages in thread From: Eduard Zingerman @ 2022-11-10 22:32 UTC (permalink / raw) To: bpf, ast Cc: andrii, daniel, kernel-team, yhs, Eduard Zingerman, kernel test robot 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) || ...) 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@intel.com> Signed-off-by: Eduard Zingerman <eddyz87@gmail.com> [1] https://lore.kernel.org/bpf/20221109142611.879983-1-eddyz87@gmail.com/ [2] https://lore.kernel.org/all/202211110355.BcGcbZxP-lkp@intel.com/ --- 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 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH bpf-next] libbpf: hashmap.h update to fix build issues using LLVM14 2022-11-10 22:32 [PATCH bpf-next] libbpf: hashmap.h update to fix build issues using LLVM14 Eduard Zingerman @ 2022-11-11 1:25 ` sdf 2022-11-11 1:34 ` sdf 2022-11-11 18:40 ` patchwork-bot+netdevbpf 1 sibling, 1 reply; 8+ messages in thread From: sdf @ 2022-11-11 1:25 UTC (permalink / raw) To: Eduard Zingerman Cc: bpf, ast, andrii, daniel, kernel-team, yhs, kernel test robot 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/ ? > 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@intel.com> > Signed-off-by: Eduard Zingerman <eddyz87@gmail.com> > [1] https://lore.kernel.org/bpf/20221109142611.879983-1-eddyz87@gmail.com/ > [2] https://lore.kernel.org/all/202211110355.BcGcbZxP-lkp@intel.com/ > --- > 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH bpf-next] libbpf: hashmap.h update to fix build issues using LLVM14 2022-11-11 1:25 ` sdf @ 2022-11-11 1:34 ` sdf 2022-11-11 1:44 ` Eduard Zingerman 2022-11-11 1:51 ` Eduard Zingerman 0 siblings, 2 replies; 8+ messages in thread From: sdf @ 2022-11-11 1:34 UTC (permalink / raw) To: Eduard Zingerman Cc: bpf, ast, andrii, daniel, kernel-team, yhs, kernel test robot 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@intel.com> > > Signed-off-by: Eduard Zingerman <eddyz87@gmail.com> > > > > [1] > https://lore.kernel.org/bpf/20221109142611.879983-1-eddyz87@gmail.com/ > > [2] https://lore.kernel.org/all/202211110355.BcGcbZxP-lkp@intel.com/ > > --- > > 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 > > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH bpf-next] libbpf: hashmap.h update to fix build issues using LLVM14 2022-11-11 1:34 ` sdf @ 2022-11-11 1:44 ` Eduard Zingerman 2022-11-11 17:19 ` sdf 2022-11-11 1:51 ` Eduard Zingerman 1 sibling, 1 reply; 8+ messages in thread From: Eduard Zingerman @ 2022-11-11 1:44 UTC (permalink / raw) To: sdf; +Cc: bpf, ast, andrii, daniel, kernel-team, yhs, kernel test robot On Thu, 2022-11-10 at 17:34 -0800, sdf@google.com 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? > > Are we simply guarding against the user calling hashmap_cast_ptr with > explicit NULL argument? In case if (p) is not a constant I want the second part of the || to kick-in. The complete condition looks as follows: _Static_assert((__builtin_constant_p((p)) ? (p) == NULL : 0) || \ sizeof(*(p)) == sizeof(long), "...error...") The intent is to check that (p) is either NULL or a pointer to something of size long. So, if (p) is not a constant the expression would be equivalent to: _Static_assert(0 || sizeof(*(p)) == sizeof(long), "...error...") I just tried the following: struct hashmap *name_map; char x; // not a constant, wrong pointer size ... hashmap__find(name_map, orig_name, &x); And it fails with an error message as intended: btf_dump.c:1548:2: error: static_assert failed due to requirement '(__builtin_constant_p((&x)) ? (&x) == ((void *)0) : 0) || sizeof (*(&x)) == sizeof(long)' "&x pointee should be a long-sized integer or a pointer" hashmap__find(name_map, orig_name, &x); ./hashmap.h:170:35: note: expanded from macro 'hashmap__find' hashmap_find((map), (long)(key), hashmap_cast_ptr(value)) ./hashmap.h:126:2: note: expanded from macro 'hashmap_cast_ptr' _Static_assert((__builtin_constant_p((p)) ? (p) == NULL : 0) || > > > > 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@intel.com> > > > Signed-off-by: Eduard Zingerman <eddyz87@gmail.com> > > > > > > [1] > > https://lore.kernel.org/bpf/20221109142611.879983-1-eddyz87@gmail.com/ > > > [2] https://lore.kernel.org/all/202211110355.BcGcbZxP-lkp@intel.com/ > > > --- > > > 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 > > > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH bpf-next] libbpf: hashmap.h update to fix build issues using LLVM14 2022-11-11 1:44 ` Eduard Zingerman @ 2022-11-11 17:19 ` sdf 2022-11-11 18:30 ` Andrii Nakryiko 0 siblings, 1 reply; 8+ messages in thread From: sdf @ 2022-11-11 17:19 UTC (permalink / raw) To: Eduard Zingerman Cc: bpf, ast, andrii, daniel, kernel-team, yhs, kernel test robot On 11/11, Eduard Zingerman wrote: > On Thu, 2022-11-10 at 17:34 -0800, sdf@google.com 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? > > > > Are we simply guarding against the user calling hashmap_cast_ptr with > > explicit NULL argument? > In case if (p) is not a constant I want the second part of the || to > kick-in. > The complete condition looks as follows: > _Static_assert((__builtin_constant_p((p)) ? (p) == NULL : 0) || \ > sizeof(*(p)) == sizeof(long), "...error...") > The intent is to check that (p) is either NULL or a pointer to > something of size long. So, if (p) is not a constant the expression > would be equivalent to: > _Static_assert(0 || sizeof(*(p)) == sizeof(long), "...error...") > I just tried the following: > struct hashmap *name_map; > char x; // not a constant, wrong pointer size > ... > hashmap__find(name_map, orig_name, &x); > And it fails with an error message as intended: > btf_dump.c:1548:2: error: static_assert failed due to > requirement '(__builtin_constant_p((&x)) ? (&x) == ((void *)0) : 0) || > sizeof (*(&x)) == sizeof(long)' "&x pointee should be a long-sized > integer or a pointer" > hashmap__find(name_map, orig_name, &x); > ./hashmap.h:170:35: note: expanded from macro 'hashmap__find' > hashmap_find((map), (long)(key), hashmap_cast_ptr(value)) > ./hashmap.h:126:2: note: expanded from macro 'hashmap_cast_ptr' > _Static_assert((__builtin_constant_p((p)) ? (p) == NULL : 0) || Awesome, thanks for clarifying! Acked-by: Stanislav Fomichev <sdf@google.com> > > > > > > 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@intel.com> > > > > Signed-off-by: Eduard Zingerman <eddyz87@gmail.com> > > > > > > > > [1] > > > https://lore.kernel.org/bpf/20221109142611.879983-1-eddyz87@gmail.com/ > > > > [2] https://lore.kernel.org/all/202211110355.BcGcbZxP-lkp@intel.com/ > > > > --- > > > > 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 > > > > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH bpf-next] libbpf: hashmap.h update to fix build issues using LLVM14 2022-11-11 17:19 ` sdf @ 2022-11-11 18:30 ` Andrii Nakryiko 0 siblings, 0 replies; 8+ messages in thread From: Andrii Nakryiko @ 2022-11-11 18:30 UTC (permalink / raw) To: sdf Cc: Eduard Zingerman, bpf, ast, andrii, daniel, kernel-team, yhs, kernel test robot On Fri, Nov 11, 2022 at 9:19 AM <sdf@google.com> wrote: > > On 11/11, Eduard Zingerman wrote: > > On Thu, 2022-11-10 at 17:34 -0800, sdf@google.com 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? > > > > > > Are we simply guarding against the user calling hashmap_cast_ptr with > > > explicit NULL argument? > > > In case if (p) is not a constant I want the second part of the || to > > kick-in. > > The complete condition looks as follows: > > > _Static_assert((__builtin_constant_p((p)) ? (p) == NULL : 0) || \ > > sizeof(*(p)) == sizeof(long), "...error...") > > > The intent is to check that (p) is either NULL or a pointer to > > something of size long. So, if (p) is not a constant the expression > > would be equivalent to: > > > _Static_assert(0 || sizeof(*(p)) == sizeof(long), "...error...") > > > I just tried the following: > > > struct hashmap *name_map; > > char x; // not a constant, wrong pointer size > > ... > > hashmap__find(name_map, orig_name, &x); > > > And it fails with an error message as intended: > > > btf_dump.c:1548:2: error: static_assert failed due to > > requirement '(__builtin_constant_p((&x)) ? (&x) == ((void *)0) : 0) || > > sizeof (*(&x)) == sizeof(long)' "&x pointee should be a long-sized > > integer or a pointer" > > hashmap__find(name_map, orig_name, &x); > > ./hashmap.h:170:35: note: expanded from macro 'hashmap__find' > > hashmap_find((map), (long)(key), hashmap_cast_ptr(value)) > > ./hashmap.h:126:2: note: expanded from macro 'hashmap_cast_ptr' > > _Static_assert((__builtin_constant_p((p)) ? (p) == NULL : 0) || > > Awesome, thanks for clarifying! > > Acked-by: Stanislav Fomichev <sdf@google.com> > Added Fixes: c302378bc157 ("libbpf: Hashmap interface update to allow both long and void* keys/values") and moved links to be before Signed-off-by (they should be last). Pushed to bpf-next, thanks. > > > > > > > > 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@intel.com> > > > > > Signed-off-by: Eduard Zingerman <eddyz87@gmail.com> > > > > > > > > > > [1] > > > > https://lore.kernel.org/bpf/20221109142611.879983-1-eddyz87@gmail.com/ > > > > > [2] https://lore.kernel.org/all/202211110355.BcGcbZxP-lkp@intel.com/ > > > > > --- > > > > > 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 > > > > > > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH bpf-next] libbpf: hashmap.h update to fix build issues using LLVM14 2022-11-11 1:34 ` sdf 2022-11-11 1:44 ` Eduard Zingerman @ 2022-11-11 1:51 ` Eduard Zingerman 1 sibling, 0 replies; 8+ messages in thread From: Eduard Zingerman @ 2022-11-11 1:51 UTC (permalink / raw) To: sdf; +Cc: bpf, ast, andrii, daniel, kernel-team, yhs, kernel test robot On Thu, 2022-11-10 at 17:34 -0800, sdf@google.com 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@intel.com> > > > Signed-off-by: Eduard Zingerman <eddyz87@gmail.com> > > > > > > [1] > > https://lore.kernel.org/bpf/20221109142611.879983-1-eddyz87@gmail.com/ > > > [2] https://lore.kernel.org/all/202211110355.BcGcbZxP-lkp@intel.com/ > > > --- > > > 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 > > > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH bpf-next] libbpf: hashmap.h update to fix build issues using LLVM14 2022-11-10 22:32 [PATCH bpf-next] libbpf: hashmap.h update to fix build issues using LLVM14 Eduard Zingerman 2022-11-11 1:25 ` sdf @ 2022-11-11 18:40 ` patchwork-bot+netdevbpf 1 sibling, 0 replies; 8+ messages in thread From: patchwork-bot+netdevbpf @ 2022-11-11 18:40 UTC (permalink / raw) To: Eduard Zingerman; +Cc: bpf, ast, andrii, daniel, kernel-team, yhs, lkp Hello: This patch was applied to bpf/bpf-next.git (master) by Andrii Nakryiko <andrii@kernel.org>: On Fri, 11 Nov 2022 00:32:40 +0200 you wrote: > A fix for the LLVM compilation error while building bpftool. > Replaces the expression: > > _Static_assert((p) == NULL || ...) > > by expression: > > [...] Here is the summary with links: - [bpf-next] libbpf: hashmap.h update to fix build issues using LLVM14 https://git.kernel.org/bpf/bpf-next/c/42597aa372f5 You are awesome, thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/patchwork/pwbot.html ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2022-11-11 18:42 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2022-11-10 22:32 [PATCH bpf-next] libbpf: hashmap.h update to fix build issues using LLVM14 Eduard Zingerman 2022-11-11 1:25 ` sdf 2022-11-11 1:34 ` sdf 2022-11-11 1:44 ` Eduard Zingerman 2022-11-11 17:19 ` sdf 2022-11-11 18:30 ` Andrii Nakryiko 2022-11-11 1:51 ` Eduard Zingerman 2022-11-11 18:40 ` patchwork-bot+netdevbpf
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox