Re: [PATCH bpf-next v1 1/3] tools/resolve_btfids: Simplify handling cross-endian compilation

[Jiri Olsa <olsajiri@gmail.com>]

On Fri, Sep 20, 2024 at 12:49:11AM -0700, Tony Ambardar wrote:

SNIP

> +static int btfids_endian_fix(struct object *obj)
> +{
> +	Elf_Data *btfids = obj->efile.idlist;
> +	Elf *elf = obj->efile.elf;
> +	int file_byteorder;
> +
> +	/* This should always succeed due to prior ELF checks */
> +	file_byteorder = elf_getident(elf, NULL)[EI_DATA];
> +
> +	/* Set type to ensure endian translation occurs, and manually invoke
> +	 * translation on input since .BTF_ids section as created disables it.
> +	 */
> +	btfids->d_type = ELF_T_WORD;
> +	if (gelf_xlatetom(elf, btfids, btfids, file_byteorder) == NULL) {
> +		pr_err("FAILED xlatetom .BTF_ids data: %s\n", elf_errmsg(-1));
> +		return -1;
> +	}
> +	return 0;
> +}
> +
>  static int elf_collect(struct object *obj)
>  {
>  	Elf_Scn *scn = NULL;
>  	size_t shdrstrndx;
> -	GElf_Ehdr ehdr;
>  	int idx = 0;
>  	Elf *elf;
>  	int fd;
> @@ -361,13 +371,6 @@ static int elf_collect(struct object *obj)
>  		return -1;
>  	}
>  
> -	if (gelf_getehdr(obj->efile.elf, &ehdr) == NULL) {
> -		pr_err("FAILED cannot get ELF header: %s\n",
> -			elf_errmsg(-1));
> -		return -1;
> -	}
> -	obj->efile.encoding = ehdr.e_ident[EI_DATA];
> -
>  	/*
>  	 * Scan all the elf sections and look for save data
>  	 * from .BTF_ids section and symbols.
> @@ -409,6 +412,8 @@ static int elf_collect(struct object *obj)
>  			obj->efile.idlist       = data;
>  			obj->efile.idlist_shndx = idx;
>  			obj->efile.idlist_addr  = sh.sh_addr;
> +			if (btfids_endian_fix(obj))
> +				return -1;

nit, it'd be bit more clear to me without using the btfids_endian_fix
function and just unwind it in here.. but anyway lgtm

Acked-by: Jiri Olsa <jolsa@kernel.org>

jirka

>  		} else if (!strcmp(name, BTF_BASE_ELF_SEC)) {
>  			/* If a .BTF.base section is found, do not resolve
>  			 * BTF ids relative to vmlinux; resolve relative
> @@ -706,24 +711,6 @@ static int sets_patch(struct object *obj)
>  			 */
>  			BUILD_BUG_ON((u32 *)set8->pairs != &set8->pairs[0].id);
>  			qsort(set8->pairs, set8->cnt, sizeof(set8->pairs[0]), cmp_id);
> -
> -			/*
> -			 * When ELF endianness does not match endianness of the
> -			 * host, libelf will do the translation when updating
> -			 * the ELF. This, however, corrupts SET8 flags which are
> -			 * already in the target endianness. So, let's bswap
> -			 * them to the host endianness and libelf will then
> -			 * correctly translate everything.
> -			 */
> -			if (obj->efile.encoding != ELFDATANATIVE) {
> -				int i;
> -
> -				set8->flags = bswap_32(set8->flags);
> -				for (i = 0; i < set8->cnt; i++) {
> -					set8->pairs[i].flags =
> -						bswap_32(set8->pairs[i].flags);
> -				}
> -			}
>  		}
>  
>  		pr_debug("sorting  addr %5lu: cnt %6d [%s]\n",
> @@ -748,9 +735,6 @@ static int symbols_patch(struct object *obj)
>  	if (sets_patch(obj))
>  		return -1;
>  
> -	/* Set type to ensure endian translation occurs. */
> -	obj->efile.idlist->d_type = ELF_T_WORD;
> -
>  	elf_flagdata(obj->efile.idlist, ELF_C_SET, ELF_F_DIRTY);
>  
>  	err = elf_update(obj->efile.elf, ELF_C_WRITE);
> -- 
> 2.34.1
>