RFR: 8144885: agent/src/os/linux/libproc.h needs to support Linux/SPARC builds

[Mikael Vidstedt]

On 2015-12-09 08:22, Mikael Vidstedt wrote:
>
>
> On 2015-12-08 21:58, Mikael Vidstedt wrote:
>>> On Dec 8, 2015, at 20:08, David Holmes <david.holmes at oracle.com> wrote:
>>>
>>>> On 9/12/2015 4:05 AM, Mikael Vidstedt wrote:
>>>>
>>>> The problem is that this needs to be backporting to jdk8u where 
>>>> there is no devkit, and more importantly, even if there were I 
>>>> believe we would see the same issue: the types needed are not made 
>>>> available by including sys/ptrace.h on this platform. I believe we 
>>>> can choose between linux/ptrace.h or asm/ptrace.h. The former feels 
>>>> slightly better.
>>> Sorry I'm not convinced. What is the issue with using sys/ptrace.h 
>>> (which should exist on your build platforms and your devkit)?
>> It simply doesn't export the type we need on SPARC (struct pt_regs or 
>> whatever it's called, I don't have the code in front of me right 
>> now). I don't know why, it simply doesn't :)
>
> Also note that there's a similar #ifdef+#include for aarch64 a few 
> lines further down:
>
> #if defined(sparc) || defined(sparcv9) || defined(ppc64)
> #define user_regs_struct pt_regs
> #endif
> #if defined(aarch64)
> #include <asm/ptrace.h>
> #define user_regs_struct user_pt_regs
> #endif So the problem doesn't seem to be unique to SPARC. Given that 
> asm/ptrace.h is used here it's probably better to use asm/ptrace.h on 
> SPARC as well for consistency. This whole logic in the SA seems to be 
> a mess at best, but I'm not sure cleaning it up is in scope for this 
> change. How about filing an enhancement for that? Cheers, Mikael

Ok, so here's the story:

On linux/x86 we use the "struct user_regs_struct" system type in the SA 
code. The user_regs_struct is what is being used throughout the SA code, 
so that's all nice and dandy.

For some reason (good or not) on SPARC the "struct pt_regs" type is used 
instead, but since the SA code is referring to "user_regs_struct" 
there's that ugly #define user_regs_struct pt_regs in libproc.h. On 
aarch64 the story is similar - the actual type being used is 
user_pt_regs, so there's helpful #define to make any code in the SA 
mentioning user_regs_struct actually use user_pt_regs instead.

Now, depending on which type is *actually* used, one will have to 
include different headers. It turns out that on linux_x64 we get the 
user_regs_struct (the actual, non-redefined thing) through this include 
chain:

"libproc.h" -> "proc_service.h" -> <thread_db.h> -> <sys/procfs.h> -> 
<sys/user.h>

But since on SPARC we're really interested in pt_regs that doesn't help; 
the pt_regs type is defined in asm/ptrace.h (which is also included from 
linux/ptrace.h). I assume the same is true for user_pt_regs on aarch64.

So I believe including sys/ptrace.h here is actually harmless, and could 
be done on SPARC too. The fix should probably instead be to #include 
<asm/ptrace.h> in the #if defined(sparcv9) block much like on aarch64.

Cheers,
Mikael

PS. This code so needs to go.


>
>
>>
>> Cheers,
>> Mikael
>>
>>> Thanks,
>>> David
>>>
>>>> Cheers,
>>>> Mikael
>>>>
>>>>> On Dec 8, 2015, at 03:15, Dmitry Samersoff 
>>>>> <dmitry.samersoff at oracle.com> wrote:
>>>>>
>>>>> Jerry,
>>>>>
>>>>>> In which case the devkit needs fixing I think, not the code.
>>>>> I'm second to David.
>>>>>
>>>>> It's better to not include linux/* headers explicitly.
>>>>>
>>>>> -Dmitry
>>>>>
>>>>>
>>>>>>> On 2015-12-08 08:23, David Holmes wrote:
>>>>>>> On 8/12/2015 2:59 PM, Gerald Thornbrugh wrote:
>>>>>>> Hi David,
>>>>>>>
>>>>>>> Yes, I believe this is a devkit issue.  Mikael and I have been 
>>>>>>> working
>>>>>>> to get JDK9 building
>>>>>>> in JPRT with the lab Linux/SPARC machines that use a devkit and 
>>>>>>> this
>>>>>>> change was needed.
>>>>>>> I will update the bug detailing this as a devkit issue.
>>>>>> In which case the devkit needs fixing I think, not the code.
>>>>>>
>>>>>> Cheers,
>>>>>> David
>>>>>>
>>>>>>> Thanks!
>>>>>>>
>>>>>>> Jerry
>>>>>>>> Hi Jerry,
>>>>>>>>
>>>>>>>>> On 8/12/2015 8:58 AM, Gerald Thornbrugh wrote:
>>>>>>>>> Hi Everyone,
>>>>>>>>>
>>>>>>>>> Please review this small change which fixes a JDK9 Linux/SPARC 
>>>>>>>>> build
>>>>>>>>> problem in "hotspot/agent/src/os/linux/libproc.h”
>>>>>>>>> so the correct “ptrace.h” include file will be used during the 
>>>>>>>>> build
>>>>>>>>> process.
>>>>>>>>>
>>>>>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8144885
>>>>>>>>> <https://bugs.openjdk.java.net/browse/JDK-8144885>
>>>>>>>>> Webrev: http://cr.openjdk.java.net/~gthornbr/8144885/webrev/
>>>>>>>>> <http://cr.openjdk.java.net/~gthornbr/8144885/webrev/>
>>>>>>>> As far as I can see both versions of ptrace.h exist on linux 
>>>>>>>> x86 and
>>>>>>>> linux sparc distributions, so it is not at all clear why this 
>>>>>>>> change
>>>>>>>> is needed. Is it perhaps an issue with a devkit?
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> David
>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>>
>>>>>>>>> Jerry
>>>>>
>>>>> -- 
>>>>> Dmitry Samersoff
>>>>> Oracle Java development team, Saint Petersburg, Russia
>>>>> * I would love to change the world, but they won't give me the 
>>>>> source code.
>