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

[Arnaldo Carvalho de Melo <acme@kernel.org>]

Em Mon, Jan 30, 2023 at 03:36:09PM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Mon, Jan 30, 2023 at 02:29:41PM +0000, Alan Maguire escreveu:
> > Compilation generates DWARF at several stages, and often the
> > later DWARF representations more accurately represent optimizations
> > that have occurred during compilation.
> > 
> > In particular, parameter representations can be spotted by their
> > abstract origin references to the original parameter, but they
> > often have more accurate location information.  In most cases,
> > the parameter locations will match calling conventions, and be
> > registers for the first 6 parameters on x86_64, first 8 on ARM64
> > etc.  If the parameter is not a register when it should be however,
> > it is likely passed via the stack or the compiler has used a
> > constant representation instead.  The latter can often be
> > spotted by checking for a DW_AT_const_value attribute,
> > as noted by Eduard.
> > 
> > In addition, absence of a location tag (either across
> > the abstract origin reference and the original parameter,
> > or in the standalone parameter description) is evidence of
> > an optimized-out parameter.  Presence of a location tag
> > is stored in the parameter description and shared between
> > abstract tags and their original referents.
> > 
> > This change adds a field to parameters and their associated
> > ftype to note if a parameter has been optimized out.  Having
> > this information allows us to skip such functions, as their
> > presence in CUs makes BTF encoding impossible.
> > 
> > Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
> > ---
> >  dwarf_loader.c | 125 +++++++++++++++++++++++++++++++++++++++++++++++++++++----
> >  dwarves.h      |   5 ++-
> >  2 files changed, 122 insertions(+), 8 deletions(-)
> > 
> > diff --git a/dwarf_loader.c b/dwarf_loader.c
> > index 5a74035..93c2307 100644
> > --- a/dwarf_loader.c
> > +++ b/dwarf_loader.c
> > @@ -992,13 +992,98 @@ static struct class_member *class_member__new(Dwarf_Die *die, struct cu *cu,
> >  	return member;
> >  }
> >  
> > -static struct parameter *parameter__new(Dwarf_Die *die, struct cu *cu, struct conf_load *conf)
> > +/* 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
> > + * 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
> 
> 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.

Thanks,

- 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;