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

[Vladimir Kozlov]

On 12/4/13 2:44 PM, Volker Simonis wrote:
>
> On Wednesday, December 4, 2013, Vladimir Kozlov wrote:
>
>     I would keep "#ifdef PPC64" in ElfFile::load_tables()) to be safe
>     because someone can create .opd file for different purpose on other
>     platforms.
>
> OK, no problem.
>
>     Few nitpicks. In our coding style we don't put 'else' and 'else if'
>     on separate line:
>
>     } else if () {
>
>     I see you put 'else' on separate line in elfSymbolTable.cpp and
>     elfFile.cpp.
>
> OK, I'll fix that.
>
>     In elfFuncDescTable.cpp missed _APPLE in #endif comment.
>
> That's because I copied it from elfFile.{hpp,cpp}. Do you want me to fix
> that as well?

Yes, please. I think it should be also have '!' negation:

#endif // !_WINDOWS and !_APPLE

Thanks,
Vladimir

>
>     Thanks,
>     Vladimir
>
>     On 12/4/13 11:17 AM, Volker Simonis 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
>         <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/
>         <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/
>                         <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/
>                         <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
>