Re: [PATCH v2 dwarves 1/5] dwarves: help dwarf loader spot functions with optimized-out parameters

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

On 30/01/2023 20:23, Arnaldo Carvalho de Melo wrote:
> Em Mon, Jan 30, 2023 at 05:10:51PM -0300, Arnaldo Carvalho de Melo escreveu:
>> Em Mon, Jan 30, 2023 at 03:36:09PM -0300, Arnaldo Carvalho de Melo escreveu:
>>>> +#define NR_REGISTER_PARAMS	8
>>>> +#elif defined(__arc__)
>>>> +#define NR_REGISTER_PARAMS	8
>>>> +#else
>>>> +#define NR_REGISTER_PARAMS      0
>>>> +#endif
>>>
>>> This should be done as a function, something like:
>>>
>>> int cu__nr_register_params(struct cu *cu)
>>> {
>>> 	GElf_Ehdr ehdr;
>>>
>>> 	gelf_getehdr(cu->elf, &ehdr);
>>>
>>> 	switch (ehdr.machine) {
>>> 	...
>>>
>>> }
>>>
>>> I'm coding that now, will send the diff shortly.
>>>
>>> This is to support cross-builds.
>>
>> I made this change to this patch, please check.
> 
> And added this to that cset:
> 
> Committer notes:
> 
> Changed the NR_REGISTER_PARAMS definition from a if/elif/endif for the
> native architecture into a function that uses the ELF header e_machine
> to find the target architecture, to allow for cross builds. 
> 
> ---
> 
> - Arnaldo
> 
>> diff --git a/dwarf_loader.c b/dwarf_loader.c
>> index 752a3c1afc4494f2..81963e71715c8435 100644
>> --- a/dwarf_loader.c
>> +++ b/dwarf_loader.c
>> @@ -994,29 +994,29 @@ static struct class_member *class_member__new(Dwarf_Die *die, struct cu *cu,
>>  
>>  /* How many function parameters are passed via registers?  Used below in
>>   * determining if an argument has been optimized out or if it is simply
>> - * an argument > NR_REGISTER_PARAMS.  Setting NR_REGISTER_PARAMS to 0
>> - * allows unsupported architectures to skip tagging optimized-out
>> + * an argument > cu__nr_register_params().  Making cu__nr_register_params()
>> + * return 0 allows unsupported architectures to skip tagging optimized-out
>>   * values.
>>   */
>> -#if defined(__x86_64__)
>> -#define NR_REGISTER_PARAMS      6
>> -#elif defined(__s390__)
>> -#define NR_REGISTER_PARAMS	5
>> -#elif defined(__aarch64__)
>> -#define NR_REGISTER_PARAMS      8
>> -#elif defined(__mips__)
>> -#define NR_REGISTER_PARAMS	8
>> -#elif defined(__powerpc__)
>> -#define NR_REGISTER_PARAMS	8
>> -#elif defined(__sparc__)
>> -#define NR_REGISTER_PARAMS	6
>> -#elif defined(__riscv) && __riscv_xlen == 64
>> -#define NR_REGISTER_PARAMS	8
>> -#elif defined(__arc__)
>> -#define NR_REGISTER_PARAMS	8
>> -#else
>> -#define NR_REGISTER_PARAMS      0
>> -#endif
>> +static int arch__nr_register_params(const GElf_Ehdr *ehdr)
>> +{
>> +	switch (ehdr->e_machine) {
>> +	case EM_S390:	 return 5;
>> +	case EM_SPARC:
>> +	case EM_SPARCV9:
>> +	case EM_X86_64:	 return 6;
>> +	case EM_AARCH64:
>> +	case EM_ARC:
>> +	case EM_ARM:
>> +	case EM_MIPS:
>> +	case EM_PPC:
>> +	case EM_PPC64:
>> +	case EM_RISCV:	 return 8;
>> +	default:	 break;
>> +	}
>> +
>> +	return 0;
>> +}
>>  
>>  static struct parameter *parameter__new(Dwarf_Die *die, struct cu *cu,
>>  					struct conf_load *conf, int param_idx)
>> @@ -1031,7 +1031,7 @@ static struct parameter *parameter__new(Dwarf_Die *die, struct cu *cu,
>>  		tag__init(&parm->tag, cu, die);
>>  		parm->name = attr_string(die, DW_AT_name, conf);
>>  
>> -		if (param_idx >= NR_REGISTER_PARAMS)
>> +		if (param_idx >= cu->nr_register_params)
>>  			return parm;
>>  		/* Parameters which use DW_AT_abstract_origin to point at
>>  		 * the original parameter definition (with no name in the DIE)
>> @@ -2870,6 +2870,7 @@ static int cu__set_common(struct cu *cu, struct conf_load *conf,
>>  		return DWARF_CB_ABORT;
>>  
>>  	cu->little_endian = ehdr.e_ident[EI_DATA] == ELFDATA2LSB;
>> +	cu->nr_register_params = arch__nr_register_params(&ehdr);
>>  	return 0;
>>  }
>>  
>> diff --git a/dwarves.h b/dwarves.h
>> index fd1ca3ae9f4ab531..ddf56f0124e0ec03 100644
>> --- a/dwarves.h
>> +++ b/dwarves.h
>> @@ -262,6 +262,7 @@ struct cu {
>>  	uint8_t		 has_addr_info:1;
>>  	uint8_t		 uses_global_strings:1;
>>  	uint8_t		 little_endian:1;
>> +	uint8_t		 nr_register_params;
>>  	uint16_t	 language;
>>  	unsigned long	 nr_inline_expansions;
>>  	size_t		 size_inline_expansions;
> 

Thanks for this, never thought of cross-builds to be honest!
Tested just now on x86_64 and aarch64 at my end, just ran
into one small thing on one system; turns out EM_RISCV isn't
defined if using a very old elf.h; below works around this
(dwarves otherwise builds fine on this system).

diff --git a/dwarf_loader.c b/dwarf_loader.c
index dba2d37..47a3bc2 100644
--- a/dwarf_loader.c
+++ b/dwarf_loader.c
@@ -992,6 +992,11 @@ static struct class_member *class_member__new(Dwarf_Die *die, struct cu *c
        return member;
 }
 
+/* for older elf.h */
+#ifndef EM_RISCV
+#define EM_RISCV       243
+#endif
+
 /* How many function parameters are passed via registers?  Used below in
  * determining if an argument has been optimized out or if it is simply
  * an argument > cu__nr_register_params().  Making cu__nr_register_params()