Re: [PATCH v8.1 14/27] xsplice, symbols: Implement symbol name resolution on address.

From: Konrad Rzeszutek Wilk <konrad@kernel.org>
To: Jan Beulich <JBeulich@suse.com>
Cc: keir@xen.org, andrew.cooper3@citrix.com, mpohlack@amazon.com,
	Ross Lagerwall <ross.lagerwall@citrix.com>,
	sasha.levin@oracle.com, xen-devel@lists.xenproject.org
Subject: Re: [PATCH v8.1 14/27] xsplice, symbols: Implement symbol name resolution on address.
Date: Sun, 24 Apr 2016 17:41:10 -0400	[thread overview]
Message-ID: <20160424214108.GA9784@localhost.localdomain> (raw)
In-Reply-To: <571A249802000078000E4B64@prv-mh.provo.novell.com>

On Fri, Apr 22, 2016 at 05:18:16AM -0600, Jan Beulich wrote:
> >>> On 22.04.16 at 13:08, <konrad.wilk@oracle.com> wrote:
> > On Fri, Apr 22, 2016 at 04:50:34AM -0600, Jan Beulich wrote:
> >> >>> On 22.04.16 at 12:28, <konrad.wilk@oracle.com> wrote:
> >> > On Fri, Apr 22, 2016 at 04:08:10AM -0600, Jan Beulich wrote:
> >> >> >>> On 22.04.16 at 10:45, <ross.lagerwall@citrix.com> wrote:
> >> >> > Rather than ignoring STT_NOTYPE, an alternative would be to ignore 
> >> >> > symbols starting with ".L".
> >> >> 
> >> >> That's an option, but as said before, the rules for which symbols to
> >> >> enter into the symbol table should be consistent for core and modules.
> >> > 
> >> > And they seem to - see above on the .o file.
> >> 
> >> Above .o file was a core one, and doesn't tell anyway whether in the
> >> runtime symbol table the local symbols would be properly prefixed by
> >> file name. Or did I misunderstand you?
> > 
> > I had 'built_in.o' or 'prelink.o' as the final one in mind - in which the
> > .LCx are present. But I think you are thinking about 'xen-syms' output.
> > 
> > Looking at the Makefile runes we hit this:
> > 127 $(TARGET)-syms: prelink.o xen.lds $(BASEDIR)/common/symbols-dummy.o
> > 128         $(LD) $(LDFLAGS) -T xen.lds -N prelink.o $(build_id_linker) \
> > 129             $(BASEDIR)/common/symbols-dummy.o -o $(@D)/.$(@F).0
> > 
> > And that ends up causing the .xen-syms.0 file to drop all the .LC0.
> 
> That's odd: I don't know of us telling the linker to do so, and imo
> it shouldn't do this by default. But yes, linkers often do strange
> things...
> 
> > However we can't do that. We _have_ to produce an relocatable object
> > and not do the final linking (so we append -r to the linker invocation).
> 
> Of course.
> 
> > I tried for fun to use strip:
> > xen/xen/arch/x86/test> strip --strip-symbol=".LC0" xen_replace_world.xsplice
> > strip: not stripping symbol `.LC0' because it is named in a relocation
> 
> Which it is rightfully saying.
> 
> So with the linker stripping .LC* (and why knows what else), you
> stripping them when building the runtime symbol table would be
> fine with me, provided we can somehow identify in the linker why
> this stripping happens, and which precise set of symbols get
> stripped (which you should then match in our code).

I spoke over the weekend with Dan Jacobowitz (who in the past worked 
on binutils) who mentioned that ELF visibility is what one should
pay attention to. And that we shouldn't need to look at LOCAL symbols
at all if we are resolving between objects.

Also he mentions that the  handling of .L* symbols is special since
they're intermediate artifacts of assembly.  They're ... "even more local".

Digging in the binutils over the weekend I found:

/* Return whether a symbol name implies a local symbol.  Most targets
   use this function for the is_local_label_name entry point, but some
   override it.  */

_bfd_elf_is_local_label_name (bfd *abfd ATTRIBUTE_UNUSED,
                              const char *name)
{
  /* Normal local symbols start with ``.L''.  */
  if (name[0] == '.' && name[1] == 'L')
    return TRUE; 
..

The function prototype is suppose to adhere to:

DESCRIPTION
        Return TRUE if a symbol with the name @var{name} in the BFD
        @var{abfd} is a compiler generated local label, 

Looking at the callchain I saw:

/* Link an input file into the linker output file.  This function
   handles all the sections and relocations of the input file at once.
   This is so that we only have to read the local symbols once, and
   don't have to keep them in memory.  */

static bfd_boolean
elf_link_input_bfd (..

And inside there is a big loop in which:

   /* See if we are discarding symbols with this name.  */
      if ((flinfo->info->strip == strip_some
           && (bfd_hash_lookup (flinfo->info->keep_hash, name, FALSE, FALSE)
               == NULL))
          || (((flinfo->info->discard == discard_sec_merge
                && (isec->flags & SEC_MERGE) && !flinfo->info->relocatable)
               || flinfo->info->discard == discard_l)
              && bfd_is_local_label_name (input_bfd, name)))
        continue;

Which really matches with what I see - that is the .LCx symbols are in
mergable sections:

[konrad@x230 x86]$ readelf --symbols prelink.o |grep LC | awk '{print $7}' | sort | uniq
22
23
33
34

 [22] .rodata.str1.1    PROGBITS         0000000000000000  0019d500
       000000000000a088  0000000000000001 AMS       0     0     1
  [23] .rodata.str1.8    PROGBITS         0000000000000000  001a7588
       0000000000020a31  0000000000000001 AMS       0     0     8

  [33] .init.rodata.str1 PROGBITS         0000000000000000  001d5a78
       0000000000000a1e  0000000000000001 AMS       0     0     1
  [34] .init.rodata.str1 PROGBITS         0000000000000000  001d6498
       0000000000002331  0000000000000001 AMS       0     0     8

 [22] .rodata.str1.1    PROGBITS         0000000000000000  0019d500
       000000000000a088  0000000000000001 AMS       0     0     1
  [23] .rodata.str1.8    PROGBITS         0000000000000000  001a7588
       0000000000020a31  0000000000000001 AMS       0     0     8

  [33] .init.rodata.str1 PROGBITS         0000000000000000  001d5a78
       0000000000000a1e  0000000000000001 AMS       0     0     1
  [34] .init.rodata.str1 PROGBITS         0000000000000000  001d6498
       0000000000002331  0000000000000001 AMS       0     0     8

And when we do the final linking:

ld    -melf_x86_64  -T xen.lds -N prelink.o --build-id=sha1 \                   
    /tmp/xen/xen/common/symbols-dummy.o -o /tmp/xen/xen/.xen-syms.0             

The xen.lds.S linker has:

.rodata : {
	..
	*(.rodata.*)
} :text

which collapses these.

(For fun I wrote a seperate entry:
	.rodata.str : {
		__lc_start = .;
		*(.rodata.str1.8)
		__lc_end = .;
	    } :text

and sure enough - that section (.rodata.str) is there and looks. There
are no symbols for it - except the __lc_start and __lc_end.

So coming back to this discussion - the code has this:

static bool_t is_payload_symbol(const struct xsplice_elf *elf,
                                const struct xsplice_elf_sym *sym)
{
    if ( sym->sym->st_shndx == SHN_UNDEF ||
         sym->sym->st_shndx >= elf->hdr->e_shnum )
        return 0;

    return (elf->sec[sym->sym->st_shndx].sec->sh_flags & SHF_ALLOC) &&
            (ELF64_ST_TYPE(sym->sym->st_info) == STT_OBJECT ||
             ELF64_ST_TYPE(sym->sym->st_info) == STT_FUNC);
}

You OK with it being the way it is? I would naturally include a 
comment about these local symbols like .L*

?

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel