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

[sdf@google.com]

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