RE: [PATCH 7/7] [gdb/testsuite] add jit-elf-util.h and run jit function

["Strasuns, Mihails" <mihails.strasuns@intel.com>]

> -----Original Message-----
> From: Simon Marchi <simark@simark.ca>
> Sent: Monday, March 23, 2020 4:13 AM
> To: Strasuns, Mihails <mihails.strasuns@intel.com>; gdb-
> patches@sourceware.org
> Subject: Re: [PATCH 7/7] [gdb/testsuite] add jit-elf-util.h and run jit function
> 
> On 2020-02-18 7:43 a.m., Mihails Strasuns wrote:
> > +/* Looks into string tables for entry
> > +   "jit_function_XXXX" and updates it to use `rename_num`.  */ static
> > +void update_name (ElfW (Addr) addr, int rename_num) {
> > +  const ElfW (Ehdr) *const ehdr = (ElfW (Ehdr) *) addr;
> > +  ElfW (Shdr) *const shdr = (ElfW (Shdr) *) ((char *) addr +
> > +ehdr->e_shoff);
> > +  ElfW (Phdr) *const phdr = (ElfW (Phdr) *) ((char *) addr +
> > +ehdr->e_phoff);
> > +
> > +  for (int i = 0; i < ehdr->e_shnum; ++i)
> > +    {
> > +      if (shdr[i].sh_type == SHT_STRTAB)
> > +	{
> > +	  /* Note: we update both .strtab and .dynstr.  The latter would
> > +	     not be correct if this were a regular shared library (.hash
> > +	     would be wrong), but this is a simulation -- the library is
> > +	     never exposed to the dynamic loader, so it all ends up ok.  */
> > +	  char *const strtab = (char *) (addr + shdr[i].sh_offset);
> > +	  char *const strtab_end = strtab + shdr[i].sh_size;
> > +	  char *p;
> > +
> > +	  for (p = strtab; p < strtab_end; p += strlen (p) + 1)
> > +	    if (strcmp (p, "jit_function_XXXX") == 0)
> > +	      sprintf (p, "jit_function_%04d", rename_num);
> > +	}
> > +    }
> > +}
> 
> I was wondering about this function.  It updates the function name in the ELF
> string table, but not in the DWARF debug info, so what is GDB going to display
> if the debug info says that the function is called `jit_function_XXXX`?
> 
> In fact, since we generate multiple shared objects, could we just generate
> them with the right function names directly?  The .exp would compile the
> first one with
> 
>   -DJIT_FUNCTION_NAME=jit_function_0001
> 
> the second one with
> 
>   -DJIT_FUNCTION_NAME=jit_function_0002
> 
> and so forth.  We wouldn't need any of this name munging.

Hello,

Here I was trying to preserve the old test flow as much as possible - I did seem a bit strange to me though.
Original version relies on _not_ having DWARF debug info present (there is a comment about it in .exp file), the
same applies for the refactored one. If you think simply emitting different function names in the first place is
an equivalent change, I am more than happy to get rid of this :)

Will address rest of comments and submit a new patchset soon, thanks for review!

BR,
Mihails
Intel Deutschland GmbH
Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Christin Eisenschmid, Gary Kershaw
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928