Re: [PATCH dwarves 1/3] dwarves: auto-detect maximum kind supported by vmlinux

[Alan Maguire <alan.maguire@oracle.com>]

On 13/09/2023 17:58, Eduard Zingerman wrote:
> On Wed, 2023-09-13 at 15:26 +0100, Alan Maguire wrote:
>> When a newer pahole is run on an older kernel, it often knows about BTF
>> kinds that the kernel does not support.  This is a problem because the BTF
>> generated is then embedded in the kernel image and read, and if unknown
>> kinds are found, BTF handling fails and core BPF functionality is
>> unavailable.
>>
>> The scripts/pahole-flags.sh script enumerates the various pahole options
>> available associated with various versions of pahole, but the problem is
>> what matters in the case of an older kernel is the set of kinds the kernel
>> understands.  Because recent features such as BTF_KIND_ENUM64 are added
>> by default (and only skipped if --skip_encoding_btf_* is set), BTF will
>> be created with these newer kinds that the older kernel cannot read.
>> This can be fixed by stable-backporting --skip options, but this is
>> cumbersome and would have to be done every time a new BTF kind is
>> introduced.
>>
>> Here instead we pre-process the DWARF information associated with the
>> target for BTF generation; if we find an enum with a BTF_KIND_MAX
>> value in the DWARF associated with the object, we use that to
>> determine the maximum BTF kind supported.  Note that the enum
>> representation of BTF kinds starts for the 5.16 kernel; prior to this
>> The benefit of auto-detection is that no work is required for older
>> kernels when new kinds are added, and --skip_encoding options are
>> less needed.
>>
>> Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
>> ---
>>  btf_encoder.c  | 12 ++++++++++++
>>  dwarf_loader.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++
>>  dwarves.h      |  2 ++
>>  3 files changed, 66 insertions(+)
>>
>> diff --git a/btf_encoder.c b/btf_encoder.c
>> index 65f6e71..98c7529 100644
>> --- a/btf_encoder.c
>> +++ b/btf_encoder.c
>> @@ -1889,3 +1889,15 @@ struct btf *btf_encoder__btf(struct btf_encoder *encoder)
>>  {
>>  	return encoder->btf;
>>  }
>> +
>> +void dwarves__set_btf_kind_max(struct conf_load *conf_load, int btf_kind_max)
>> +{
>> +	if (btf_kind_max < 0 || btf_kind_max >= BTF_KIND_MAX)
>> +		return;
>> +	if (btf_kind_max < BTF_KIND_DECL_TAG)
>> +		conf_load->skip_encoding_btf_decl_tag = true;
>> +	if (btf_kind_max < BTF_KIND_TYPE_TAG)
>> +		conf_load->skip_encoding_btf_type_tag = true;
>> +	if (btf_kind_max < BTF_KIND_ENUM64)
>> +		conf_load->skip_encoding_btf_enum64 = true;
>> +}
>> diff --git a/dwarf_loader.c b/dwarf_loader.c
>> index ccf3194..8984043 100644
>> --- a/dwarf_loader.c
>> +++ b/dwarf_loader.c
>> @@ -3358,8 +3358,60 @@ static int __dwarf_cus__process_cus(struct dwarf_cus *dcus)
>>  	return 0;
>>  }
>>  
>> +/* Find enumeration value for BTF_KIND_MAX; replace conf_load->btf_kind_max with
>> + * this value if found since it indicates that the target object does not know
>> + * about kinds > its BTF_KIND_MAX value.  This is valuable for kernel/module
>> + * BTF where a newer pahole/libbpf operate on an older kernel which cannot
>> + * parse some of the newer kinds pahole can generate.
>> + */
>> +static void dwarf__find_btf_kind_max(struct dwarf_cus *dcus)
>> +{
>> +	struct conf_load *conf = dcus->conf;
>> +	uint8_t pointer_size, offset_size;
>> +	Dwarf_Off off = 0, noff;
>> +	size_t cuhl;
>> +
>> +	while (dwarf_nextcu(dcus->dw, off, &noff, &cuhl, NULL, &pointer_size, &offset_size) == 0) {
>> +		Dwarf_Die die_mem;
>> +		Dwarf_Die *cu_die = dwarf_offdie(dcus->dw, off + cuhl, &die_mem);
>> +		Dwarf_Die child;
>> +
>> +		if (cu_die == NULL)
>> +			break;
>> +		if (dwarf_child(cu_die, &child) == 0) {
>> +			Dwarf_Die *die = &child;
>> +
>> +			do {
>> +				Dwarf_Die echild, *edie;
>> +
>> +				if (dwarf_tag(die) != DW_TAG_enumeration_type ||
>> +				    !dwarf_haschildren(die) ||
>> +				    dwarf_child(die, &echild) != 0)
>> +					continue;
>> +				edie = &echild;
>> +				do {
>> +					const char *ename;
>> +					int btf_kind_max;
>> +
>> +					if (dwarf_tag(edie) != DW_TAG_enumerator)
>> +						continue;
>> +					ename = attr_string(edie, DW_AT_name, conf);
>> +					if (!ename || strcmp(ename, "BTF_KIND_MAX") != 0)
>> +						continue;
>> +					btf_kind_max = attr_numeric(edie, DW_AT_const_value);
> 
> Nitpick: attr_numeric() returns 0 in case of an error, when 0 is passed to
>          dwarves__set_btf_kind_max() it would turn off all optional kinds.
>          Probably should bail out on 0 instead.
>

not a nitpick at all, great catch! will fix this and make the naming
consistent in patch 3 (using btf__find_btf_kind_max() as you suggest).
And many thanks for testing this at your end!

Alan

>> +					dwarves__set_btf_kind_max(conf, btf_kind_max);
>> +					return;
>> +				} while (dwarf_siblingof(edie, edie) == 0);
>> +			} while (dwarf_siblingof(die, die) == 0);
>> +		}
>> +		off = noff;
>> +	}
>> +}
>> +
>>  static int dwarf_cus__process_cus(struct dwarf_cus *dcus)
>>  {
>> +	dwarf__find_btf_kind_max(dcus);
>> +
>>  	if (dcus->conf->nr_jobs > 1)
>>  		return dwarf_cus__threaded_process_cus(dcus);
>>  
>> diff --git a/dwarves.h b/dwarves.h
>> index eb1a6df..f4d9347 100644
>> --- a/dwarves.h
>> +++ b/dwarves.h
>> @@ -1480,4 +1480,6 @@ extern const char tabs[];
>>  #define DW_TAG_skeleton_unit 0x4a
>>  #endif
>>  
>> +void dwarves__set_btf_kind_max(struct conf_load *conf_load, int btf_kind_max);
>> +
>>  #endif /* _DWARVES_H_ */
> 
>