Re: [PATCH bpf-next 04/12] libbpf: Add btf enum64 support

[Yonghong Song <yhs@fb.com>]

On 5/3/22 10:22 AM, kernel test robot wrote:
> Hi Yonghong,
> 
> I love your patch! Perhaps something to improve:
> 
> [auto build test WARNING on bpf-next/master]
> 
> url:    https://github.com/intel-lab-lkp/linux/commits/Yonghong-Song/bpf-Add-64bit-enum-value-support/20220502-030301
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master
> config: i386-randconfig-m021 (https://download.01.org/0day-ci/archive/20220504/202205040133.jd7yTwg5-lkp@intel.com/config )
> compiler: gcc-11 (Debian 11.2.0-20) 11.2.0
> 
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
> 
> New smatch warnings:
> tools/lib/bpf/relo_core.c:348 bpf_core_fields_are_compat() warn: if();
> 
> Old smatch warnings:
> tools/lib/bpf/relo_core.c:349 bpf_core_fields_are_compat() warn: if();

The following change should work:

diff --git a/tools/lib/bpf/relo_core.c b/tools/lib/bpf/relo_core.c
index 1e751400427b..2c8d5292e946 100644
--- a/tools/lib/bpf/relo_core.c
+++ b/tools/lib/bpf/relo_core.c
@@ -345,9 +345,8 @@ static int bpf_core_fields_are_compat(const struct 
btf *local_btf,
         if (btf_is_composite(local_type) && btf_is_composite(targ_type))
                 return 1;
         if (btf_kind(local_type) != btf_kind(targ_type)) {
-               if (btf_is_enum(local_type) && btf_is_enum64(targ_type)) ;
-               else if (btf_is_enum64(local_type) && 
btf_is_enum(targ_type)) ;
-               else return 0;
+               if (!btf_is_enum(local_type) || !btf_is_enum64(targ_type))
+                       return 0;
         }

         switch (btf_kind(local_type)) {

I will wait for more comments before submitting version 2.

> 
> vim +348 tools/lib/bpf/relo_core.c
> 
>     314	
>     315	/* Check two types for compatibility for the purpose of field access
>     316	 * relocation. const/volatile/restrict and typedefs are skipped to ensure we
>     317	 * are relocating semantically compatible entities:
>     318	 *   - any two STRUCTs/UNIONs are compatible and can be mixed;
>     319	 *   - any two FWDs are compatible, if their names match (modulo flavor suffix);
>     320	 *   - any two PTRs are always compatible;
>     321	 *   - for ENUMs, names should be the same (ignoring flavor suffix) or at
>     322	 *     least one of enums should be anonymous;
>     323	 *   - for ENUMs, check sizes, names are ignored;
>     324	 *   - for INT, size and signedness are ignored;
>     325	 *   - any two FLOATs are always compatible;
>     326	 *   - for ARRAY, dimensionality is ignored, element types are checked for
>     327	 *     compatibility recursively;
>     328	 *   - everything else shouldn't be ever a target of relocation.
>     329	 * These rules are not set in stone and probably will be adjusted as we get
>     330	 * more experience with using BPF CO-RE relocations.
>     331	 */
>     332	static int bpf_core_fields_are_compat(const struct btf *local_btf,
>     333					      __u32 local_id,
>     334					      const struct btf *targ_btf,
>     335					      __u32 targ_id)
>     336	{
>     337		const struct btf_type *local_type, *targ_type;
>     338	
>     339	recur:
>     340		local_type = skip_mods_and_typedefs(local_btf, local_id, &local_id);
>     341		targ_type = skip_mods_and_typedefs(targ_btf, targ_id, &targ_id);
>     342		if (!local_type || !targ_type)
>     343			return -EINVAL;
>     344	
>     345		if (btf_is_composite(local_type) && btf_is_composite(targ_type))
>     346			return 1;
>     347		if (btf_kind(local_type) != btf_kind(targ_type)) {
>   > 348			if (btf_is_enum(local_type) && btf_is_enum64(targ_type)) ;
>     349			else if (btf_is_enum64(local_type) && btf_is_enum(targ_type)) ;
>     350			else return 0;
>     351		}
>     352	
>     353		switch (btf_kind(local_type)) {
>     354		case BTF_KIND_PTR:
>     355		case BTF_KIND_FLOAT:
>     356			return 1;
>     357		case BTF_KIND_FWD:
>     358		case BTF_KIND_ENUM:
>     359		case BTF_KIND_ENUM64: {
>     360			const char *local_name, *targ_name;
>     361			size_t local_len, targ_len;
>     362	
>     363			local_name = btf__name_by_offset(local_btf,
>     364							 local_type->name_off);
>     365			targ_name = btf__name_by_offset(targ_btf, targ_type->name_off);
>     366			local_len = bpf_core_essential_name_len(local_name);
>     367			targ_len = bpf_core_essential_name_len(targ_name);
>     368			/* one of them is anonymous or both w/ same flavor-less names */
>     369			return local_len == 0 || targ_len == 0 ||
>     370			       (local_len == targ_len &&
>     371				strncmp(local_name, targ_name, local_len) == 0);
>     372		}
>     373		case BTF_KIND_INT:
>     374			/* just reject deprecated bitfield-like integers; all other
>     375			 * integers are by default compatible between each other
>     376			 */
>     377			return btf_int_offset(local_type) == 0 &&
>     378			       btf_int_offset(targ_type) == 0;
>     379		case BTF_KIND_ARRAY:
>     380			local_id = btf_array(local_type)->type;
>     381			targ_id = btf_array(targ_type)->type;
>     382			goto recur;
>     383		default:
>     384			return 0;
>     385		}
>     386	}
>     387	
>