RFR(L): 8049716: PPC64: Implement SA on Linux/PPC64

[Alexander Smundak]

You are correct, but there no need to have this code for LE at all.

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"));


On Thu, Dec 4, 2014 at 3:43 PM, Maynard Johnson <maynardj at 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.simonis at 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
>>
>