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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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





[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux