[PATCH bpf-next] libbpf: hashmap.h update to fix build issues using LLVM14

[PATCH bpf-next] libbpf: hashmap.h update to fix build issues using LLVM14

[Eduard Zingerman]

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

Re: [PATCH bpf-next] libbpf: hashmap.h update to fix build issues using LLVM14

[sdf]

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

Re: [PATCH bpf-next] libbpf: hashmap.h update to fix build issues using LLVM14

[sdf]

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

Re: [PATCH bpf-next] libbpf: hashmap.h update to fix build issues using LLVM14

[Eduard Zingerman]

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

Re: [PATCH bpf-next] libbpf: hashmap.h update to fix build issues using LLVM14

[Eduard Zingerman]

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

Re: [PATCH bpf-next] libbpf: hashmap.h update to fix build issues using LLVM14

[sdf]

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

Re: [PATCH bpf-next] libbpf: hashmap.h update to fix build issues using LLVM14

[Andrii Nakryiko]

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

Re: [PATCH bpf-next] libbpf: hashmap.h update to fix build issues using LLVM14

[patchwork-bot+netdevbpf]

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