On 12/04/2014 07:50 PM, Alexander Smundak wrote:
> You are correct, but there no need to have this code for LE at all.
Agreed. I'm fine with adding the "&& !defined(ABI_ELFv2)" throughout that file
along with the "#if defined(ppc64)".
>
> BTW, a bit on nitpicking in the same file:
> + String endian = System.getProperty("sun.cpu.endian");
> + if (endian.equals("big"))
> + return true;
> + else
> + return false;
> can be rewirtten as
> return "big".equals(System.getProperty("sun.cpu.endian"));
Right. A silly piece of coding there. :-/
-Maynard
>
>
> On Thu, Dec 4, 2014 at 3:43 PM, Maynard Johnson <mayna...@us.ibm.com> wrote:
>> On 12/04/2014 01:15 PM, Alexander Smundak wrote:
>>> The changes for agent/src/os/linux/symtab.c
>>> b/agent/src/os/linux/symtab.c in
>>> http://cr.openjdk.java.net/~simonis/webrevs/8049716 will break
>>> Linux/PPC64 little-endian:
>>> it uses ABIv2, which dropped function descriptors. So the preprocessor
>>> brackets in it should
>>> read
>>> #if defined(ppc64) && !defined(ABI_ELFv2)
>>> instead of just
>>> #if defined(ppc64)
>> Hi, Alexander,
>> I think this is actually fine everywhere except one place. The 'opd'
>> variable will be
>> set to something other than NULL at line 379 only if running on ppc64 BE. So
>> in
>> the rest of that function, opd is checked for non-null before using it. The
>> only
>> place where I think there may be a problem is at line 455:
>>
>> --------------------------
>> #if defined(ppc64)
>> // On Linux/PPC64 the debuginfo files contain an empty file descriptor
>> // section (i.e. '.opd' section) which makes the resolution of symbols
>> // with the above algorithm impossible (we would need the have both, the
>> // .opd section from the library and the symbol table from the debuginfo
>> // file which doesn't match with the current workflow.)
>> if (false) {
>> #else
>> // Look for a separate debuginfo file.
>> if (try_debuginfo) {
>> #endif
>> --------------------------
>>
>> Here I think we should do as you suggest:
>> #if defined(ppc64) && !defined(ABI_ELFv2)
>>
>> -Maynard
>>>
>>> Sorry for the late notice.
>>> Sasha
>>>
>>> On Thu, Dec 4, 2014 at 9:50 AM, Volker Simonis <volker.simo...@gmail.com>
>>> wrote:
>>>> Hi,
>>>>
>>>> I'd like to submit this webrev which adds support for the SA agent on
>>>> Linux/PPC64 on behalf of Maynard Johnson who is the main author of the
>>>> change:
>>>>
>>>> http://cr.openjdk.java.net/~simonis/webrevs/8049716
>>>> https://bugs.openjdk.java.net/browse/JDK-8049716
>>>>
>>>> I have already reviewed and tested the change and from my side
>>>> everything looks fine.
>>>>
>>>> The change touches quite some shared code but all of these changes are
>>>> trivial and straight-forward (i.e. they just add Linux/PPC64 support
>>>> with the help of '#ifdef's in C or yet another 'elseif' clause in
>>>> Java).
>>>>
>>>> We need a second reviewer and a sponsor who can push this to the
>>>> hotspot repository once the review is completed.
>>>>
>>>> Thank you and best regards,
>>>> Volker
>>>
>>
>
