RFR(S): 8019929: PPC64 (part 107): Extend ELF-decoder to support PPC64 function descriptor tables

[Vitaly Davidovich]

Ok.

171         if (string_table->string_at(shdr.sh_name, buf, sizeof(buf)) &&
!strncmp(".opd", buf, 4)) {
172           m_funcDesc_table = new (std::nothrow)
ElfFuncDescTable(m_file, shdr);
173           break;

So if that alloc fails, I see that code handles a null m_funcDesc_table
where it's used.  But is that what you want for PPC64? Won't you get wrong
symbol info? Code reading other tables does this for OOM cases:

m_status = NullDecoder::out_of_memory;
return false;

Sent from my phone
On Dec 4, 2013 2:17 PM, "Volker Simonis" <volker.simonis at gmail.com> wrote:

> Hi,
>
> thanks for the comments.
>
> I think the function descriptor table logically belongs to the
> ELF-file itself and not to symbol table. An ELF file can have several
> symbol tables but just one function descriptor table. Also, the
> function descriptor table is read in when the ELF file is opened (i.e.
> in the ElfFile constructor).
>
> So after Vladimirs suggestion to remove most of the "#ifdef PPC64"
> there was no reason to keep the ElfFuncDescTable class in
> elfSymbolTable.{hpp,cpp} so I created two new files
> elfFuncDescTable.{hpp, cpp} for it. Now the only remaining "#ifdef
> PPC64" is in ElfFile::load_tables() when the function descriptor table
> is loaded (as requested by Vladimir).
>
> But actually, the corresponding '.opd' section is only available on
> PPC64 (see
> http://refspecs.linuxfoundation.org/LSB_3.1.1/LSB-Core-PPC64/LSB-Core-PPC64/specialsections.html
> )
> and I don't think the code will do any harm if it would be executed on
> a non-PPC64 system - the '.opd' section would just not be found. I
> also think the corresponding performance impact would be minimal
> compared to the loading of the symbol and string tables. So I tend to
> remove the last "#ifdef PPC64" as well. So what do you think - I'm OK
> with both solutions?
>
> Below is a webrev with the described changes (and still with the last
> "#ifdef PPC64" in ElfFile::load_tables()):
>
> http://cr.openjdk.java.net/~simonis/webrevs/8019929.v2/
>
> If you agree with it, I would appreciate if you could push it trough JPRT.
>
> Thank you and best regards,
> Volker
>
> PS: the little change in make/aix/makefiles/vm.make was necessarx to
> exlude the new file from the AIX-build because AIX uses XCOFF instead
> of ELF.
>
>
> On Wed, Dec 4, 2013 at 3:39 AM, Vitaly Davidovich <vitalyd at gmail.com>
> wrote:
> > Hi Volker,
> >
> > Would it be cleaner if you were to extend ElfSymbolTable for PPC and
> embed
> > the funcDesc in there, keeping the lookup() signature the same? It seems
> > like the funcDesc should be a hidden indirection as part of lookup()
> rather
> > than a parameter.
> >
> > Just a thought ...
> >
> > Sent from my phone
> >
> > On Dec 3, 2013 5:43 PM, "Volker Simonis" <volker.simonis at gmail.com>
> wrote:
> >>
> >> Hi Vladimir,
> >>
> >> thanks for looking at the change. I initially did it this way to
> >> keep changes to the existing platforms as small as possible but I'll be
> >> happy to change in the way you suggested if nobody objects.
> >>
> >> Regards,
> >> Volker
> >>
> >> On Tuesday, December 3, 2013, Vladimir Kozlov wrote:
> >>
> >> > Volker,
> >> >
> >> > It looks fine to me except #ifdef pollution.
> >> >
> >> > I think ElfSymbolTable::lookup() should always take ElfFuncDescTable
> >> > argument and you need ElfFuncDescTable always defined.
> >> > In ElfSymbolTable::lookup() you can check funcDescTable for null
> instead
> >> > of ifdefs.
> >> > The only place we can keep #ifdef is m_funcDesc_table setting in
> >> > ElfFile::load_tables().
> >> > ElfFuncDescTable class's methods are not so big to ifdef them.
> >> >
> >> > But others may have different opinion.
> >> >
> >> > Thanks,
> >> > Vladimir
> >> >
> >> > On 12/3/13 9:39 AM, Volker Simonis wrote:
> >> >
> >> >> On PowerPC-64 (and other architectures like for example IA64) a
> >> >> pointer to a function is not just a plain code address, but a pointer
> >> >> to a so called function descriptor (see
> >> >> http://refspecs.linuxfoundation.org/ELF/ppc64/
> >> >> PPC-elf64abi-1.9.html#FUNC-DES).
> >> >> This fact is also reflected in the ELF ABI for PowerPC-64. This small
> >> >> changes adds support for ELF function descriptor tables to the
> current
> >> >> ELF decoder:
> >> >>
> >> >> http://cr.openjdk.java.net/~simonis/webrevs/8019929/
> >> >>
> >> >> On architectures like x86 or SPARC, the ELF symbol table contains the
> >> >> start address and size of an object. So for example for a function
> >> >> object (i.e. type FUNC) the symbol table's value field directly
> >> >> represents the starting address and the size field the size of a
> >> >> function. On PPC64 however, the symbol table's value field only
> >> >> contains an index into a PPC64 specific .opd (official procedure
> >> >> descriptors) section, while the size field still holds the size of
> the
> >> >> corresponding function. In order to get the actual start address of a
> >> >> function, it is necessary to read the corresponding function
> >> >> descriptor entry in the .opd section at the corresponding index and
> >> >> extract the start address from there.
> >> >>
> >> >> This change extends the current HotSpot ELF utilities to support the
> >> >> .opd (official procedure descriptors) section on PPC64 platforms. It
> >> >> does this by adding a new field m_funcDesc_table of type
> >> >> ElfFuncDescTable to the ElfFile class. The m_funcDesc_table is
> >> >> initialized in the ElfFile::load_tables() in the same way like the
> >> >> symbol table members by parsing the corresponding .opd section if it
> >> >> is available.
> >> >>
> >> >> The ElfSymbolTable::lookup() method is changed on PPC64 to take an
> >> >> extra ElfFuncDescTable argument. If running on PPC64, this argument
> is
> >> >> used to do the extra level of indirection through the function
> >> >> description table to get the real start address associated with a
> >> >> symbol.
> >> >>
> >> >> This change also slightly improves the implementation of
> >> >> ElfSymbolTable::lookup(). Before, the method always iterated over all
> >> >> symbols in the symbol table and returned the one with the highest
> >> >> address below the requested addr argument. This not only could take a
> >> >> significant amount of time for big libraries, it could also return
> >> >> bogus symbols for addresses which were not really covered by that
> >> >> symbol table at all. The new versions additionally uses the symbol
> >> >> table's st_size field to verify that the requested addr argument is
> >> >> indeed within the range covered by the corresponding symbol table
> >> >> entry. If so, the search is stopped and the symbol is returned
> >> >> immediately.
> >> >>
> >> >> Thank you and best regards,
> >> >> Volker
> >> >>
> >> >>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/ppc-aix-port-dev/attachments/20131204/75178ec7/attachment.html