Review request: 7003789 PTRACE_GETREGS problems with SA on Linux.

Kevin Walls kevin.walls at oracle.com
Sun Dec 5 23:07:32 PST 2010 

That's great, thanks. I'll get it pushed...


On 06/12/2010 00:14, Yasumasa Suenaga wrote:
> Hi Kevin,
>
> +1
>
> I agree to this modification.
> (Should I do anything? vote?)
>
> (2010/12/03 19:48), Kevin Walls wrote:
>>> If PTRACE_GETREGS is undefined and PT_GETREGS is defined, PTRACE_GETREGS_REQ will be defined
>>> by undefined value.
>> Yes, that was worrying me as well, as it's what I see on my fedora 13 x86_x64 system.
>>
>> And yes to your second message - we need to leave in the sparcv9 parts.
>>
>> I'm convinced PTRACE_GETREGS64 only exists on 64-bit sparcv9, and PowerPC were we don't build. If it became defined on a platform that is not 64-bit we will only use it if we are doing a 64-bit build, so that looks good.  Further searching finds this thread...
>>
>> So here it is as a webrev. Builds and tests OK on x86 and x64.
>>
>> http://cr.openjdk.java.net/~kevinw/7003789/webrev.01/
>>
>> Regards
>> Kevin
>>
>>
>>
>> On 03/12/2010 06:47, Yasumasa Suenaga wrote:
>>> Sorry...
>>>
>>>> BTW, this modification is for "agent/src/os/linux/ps_proc.c".
>>>> I guess that this source affects for Linux ONLY.
>>>> If my guess is correct, we can delete the statement about SPARC from this source code.
>>>>
>>>> -------
>>>> #if defined(sparc) || defined(sparcv9)
>>>> #define ptrace_getregs(request, pid, addr, data) ptrace(request, pid, addr, data)
>>>> #else
>>>> #define ptrace_getregs(request, pid, addr, data) ptrace(request, pid, data, addr)
>>>> #endif
>>>> -------
>>>>
>>>> and
>>>>
>>>> -------
>>>> #ifdef PTRACE_GETREGS64
>>>> #define PTRACE_GETREGS_REQ PTRACE_GETREGS64
>>>> -------
>>> This sentence concerns CPU... not OS.
>>> I don't have Linux on SPARC. So, I can't confirm that Linux (glibc) ptrace() systemcall on SPARC
>>> can handle PTRACE_GETREGS64.
>>> If ptrace() interface is different from x86 and SPARC (Linux on SPARC defines PTRACE_GETREGS64),
>>> my guess is incorrect...
>>>
>>> (2010/12/03 15:21), Yasumasa Suenaga wrote:
>>>> Hi David, Kevin,
>>>>
>>>>>     #if defined(PTRACE_GETREGS) || defined(PT_GETREGS)
>>>>>       #define PTRACE_GETREGS_REQ PTRACE_GETREGS
>>>>>     #endif
>>>> If PTRACE_GETREGS is undefined and PT_GETREGS is defined, PTRACE_GETREGS_REQ will be defined
>>>> by undefined value. (PTRACE_GETREGS: 1? decided by preprocessor... does that case exist?)
>>>>
>>>> In Linux manpage PTRACE(2) in Fedora14, PTRACE_GETREGS is described, PTRACE_GETREGS64 and
>>>> PT_GETREGS is NOT described.
>>>>> My system(Fedora14 x86_64) defines PTRACE_GETREGS and PT_GETREGS, PTRACE_GETREGS64 does NOT define.
>>>> So, how about this?
>>>>
>>>> -------
>>>> #if defined(_LP64)&&   defined(PTRACE_GETREGS64) /* for SPARCV9(and others?) */
>>>> #define PTRACE_GETREGS_REQ PTRACE_GETREGS64
>>>> #elif defined(PTRACE_GETREGS) /* for Linux */
>>>> #define PTRACE_GETREGS_REQ PTRACE_GETREGS
>>>> #elif defined(PT_GETREGS) /* last resort */
>>>> #define PTRACE_GETREGS_REQ PT_GETREGS
>>>> #endif /* _LP64 */
>>>> -------
>>>>
>>>> BTW, this modification is for "agent/src/os/linux/ps_proc.c".
>>>> I guess that this source affects for Linux ONLY.
>>>> If my guess is correct, we can delete the statement about SPARC from this source code.
>>>>
>>>> -------
>>>> #if defined(sparc) || defined(sparcv9)
>>>> #define ptrace_getregs(request, pid, addr, data) ptrace(request, pid, addr, data)
>>>> #else
>>>> #define ptrace_getregs(request, pid, addr, data) ptrace(request, pid, data, addr)
>>>> #endif
>>>> -------
>>>>
>>>> and
>>>>
>>>> -------
>>>> #ifdef PTRACE_GETREGS64
>>>> #define PTRACE_GETREGS_REQ PTRACE_GETREGS64
>>>> -------
>>>>
>>>>
>>>> Thanks.
>>>> Yasumasa
>>>>
>>>>
>>>> (2010/12/03 12:23), David Holmes wrote:
>>>>> Kevin Walls said the following on 12/03/10 06:03:
>>>>>> Thanks both for pointing out that I didn't notice I was smashing the
>>>>>> 32-bit case. 8-)
>>>>>>
>>>>>> I can now build and test 32 and 64-bit OK with...
>>>>>>
>>>>>> #ifdef _LP64
>>>>>>      #ifdef PTRACE_GETREGS64
>>>>>>        #define PTRACE_GETREGS_REQ PTRACE_GETREGS64
>>>>>>      #elif defined(PTRACE_GETREGS)
>>>>>>        #define PTRACE_GETREGS_REQ PTRACE_GETREGS
>>>>>>      #elif defined(PT_GETREGS)
>>>>>>        #define PTRACE_GETREGS_REQ PT_GETREGS
>>>>>>      #endif
>>>>>> #else
>>>>>>      #if defined(PTRACE_GETREGS) || defined(PT_GETREGS)
>>>>>>        #define PTRACE_GETREGS_REQ PTRACE_GETREGS
>>>>>>      #endif
>>>>>> #endif /* _LP64 */
>>>>>>
>>>>>> My 64-bit system is Fedora and really needs to use PT_REGS as that's the
>>>>>> only definition. My 32-bit system is a different RedHat and has
>>>>>> PTRACE_GETREGS defined so the 32-bit build does work. Am tempted to
>>>>>> change the 32-bit side of the ifdef to also use PT_GETREGS as a last
>>>>>> resort...
>>>>> I'd be tempted to do that too. This seems so inconsistent I wouldn't
>>>>> trust what you might find on any particular linux.
>>>>>
>>>>> If we knew that we'd never find PTRACE_GETREGS64 on a 32-bit system we
>>>>> could make it somewhat simpler.
>>>>>
>>>>> David
>>>>>
>>>>>> Thanks
>>>>>> Kevin
>>>>>>
>>>>>>
>>>>>> On 02/12/2010 02:23, Yasumasa Suenaga wrote:
>>>>>>> Thank you for the reply.
>>>>>>>
>>>>>>> I've read your patch on a webrev.
>>>>>>> Your patch will work on AMD64 architecture.
>>>>>>> However, this patch will not work on x86 (32bit) architecture.
>>>>>>> Preprocessor in GCC for i386 does not define "_LP64" .
>>>>>>>
>>>>>>> -----------------
>>>>>>> [root at RHEL4-4 test]# cat /etc/redhat-release
>>>>>>> Red Hat Enterprise Linux ES release 4 (Nahant Update 4)
>>>>>>> [root at RHEL4-4 test]# uname -a
>>>>>>> Linux RHEL4-4 2.6.9-42.EL #1 Wed Jul 12 23:16:43 EDT 2006 i686 i686 i386 GNU/Linux
>>>>>>> [root at RHEL4-4 test]# rpm -q glibc
>>>>>>> glibc-2.3.4-2.25
>>>>>>> [root at RHEL4-4 test]# rpm -q gcc
>>>>>>> gcc-3.4.6-3
>>>>>>> [root at RHEL4-4 test]# touch test.h
>>>>>>> [root at RHEL4-4 test]# cpp -dM test.h | grep _LP64
>>>>>>> [root at RHEL4-4 test]#
>>>>>>> -----------------
>>>>>>>
>>>>>>>
>>>>>>> So, you need to modify the patch as follows:
>>>>>>>
>>>>>>> -----------------
>>>>>>> [root at Fedora13 OpenJDK7]# diff -u b118/openjdk/hotspot/agent/src/os/linux/ps_proc.c trunk/
>>>>>>> openjdk/hotspot/agent/src/os/linux/ps_proc.c
>>>>>>> --- a/agent/src/os/linux/ps_proc.c   2010-11-12 05:43:12.000000000 +0900
>>>>>>> +++ b/agent/src/os/linux/ps_proc.c   2010-12-02 10:45:09.117050388 +0900
>>>>>>> @@ -124,6 +124,8 @@
>>>>>>>     #ifdef _LP64
>>>>>>>     #ifdef PTRACE_GETREGS64
>>>>>>>     #define PTRACE_GETREGS_REQ PTRACE_GETREGS64
>>>>>>> +#elif defined(PTRACE_GETREGS)
>>>>>>> +#define PTRACE_GETREGS_REQ PTRACE_GETREGS
>>>>>>>     #endif
>>>>>>>     #else
>>>>>>>     #if defined(PTRACE_GETREGS) || defined(PT_GETREGS)
>>>>>>> -----------------
>>>>>>>
>>>>>>>
>>>>>>> Thanks.
>>>>>>>
>>>>>>>
>>>>>>> (2010/12/02 1:41), Kevin Walls wrote:
>>>>>>>> Sorry, there was actually a typo in that diff, although it was good
>>>>>>>> enough to be a solution on my system.
>>>>>>>>
>>>>>>>> A webrev also:
>>>>>>>>
>>>>>>>> http://cr.openjdk.java.net/~kevinw/7003789/webrev.00/
>>>>>>>>
>>>>>>>> Thanks
>>>>>>>> Kevin
>>>>>>>>
>>>>>>>>
>>>>>>>> On 01/12/2010 16:01, Kevin Walls wrote:
>>>>>>>>> Hi --
>>>>>>>>>
>>>>>>>>> I've just been hitting that as well.... I think it needs to be dealt
>>>>>>>>> with as a separate bug as there may be a few which are similar - and
>>>>>>>>> 6359295 is marked fixed in 5.0 long ago...
>>>>>>>>>
>>>>>>>>> I just found your message after I'd done some investigating... The patch
>>>>>>>>> I was working with is to split one of the existing "if defined"
>>>>>>>>> statements, as currently the bug is: if we have PT_GETREGS defined, we
>>>>>>>>> use PTRACE_GETREGS to define PTRACE_GETREGS_REQ... We soon realise that
>>>>>>>>> may NOT be defined and give the "unsupported" message. So I can log and
>>>>>>>>> bug and get this done if it sounds good:
>>>>>>>>>
>>>>>>>>> [kevin at oldbox make]$ hg diff  ../src/os/linux/ps_proc.c
>>>>>>>>> diff --git a/agent/src/os/linux/ps_proc.c b/agent/src/os/linux/ps_proc.c
>>>>>>>>> --- a/agent/src/os/linux/ps_proc.c
>>>>>>>>> +++ b/agent/src/os/linux/ps_proc.c
>>>>>>>>> @@ -124,9 +124,9 @@
>>>>>>>>>      #ifdef _LP64
>>>>>>>>>      #ifdef PTRACE_GETREGS64
>>>>>>>>>      #define PTRACE_GETREGS_REQ PTRACE_GETREGS64
>>>>>>>>> -#endif
>>>>>>>>> -#else
>>>>>>>>> -#if defined(PTRACE_GETREGS) || defined(PT_GETREGS)
>>>>>>>>> +#elif defined(PTRACE_GETREGS)
>>>>>>>>> +#define PTRACE_GETREGS_REQ PTRACE_GETREGS
>>>>>>>>> +#elif defined (PT_GETREGS)
>>>>>>>>>      #define PTRACE_GETREGS_REQ PTRACE_GETREGS
>>>>>>>>>      #endif
>>>>>>>>>      #endif /* _LP64 */
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Regards
>>>>>>>>> Kevin
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On 15/11/2010 06:46, Yasumasa Suenaga wrote:
>>>>>>>>>> Hi.
>>>>>>>>>>
>>>>>>>>>> I and co-worker use jstack for various trouble shooting.
>>>>>>>>>>
>>>>>>>>>> We mainly use Java on Linux with AMD64 architecture.
>>>>>>>>>> However, jstack -F option doesn't work our platform.
>>>>>>>>>>
>>>>>>>>>> I ran jstack -F with LIBSAPROC_DEBUG=1 (environment variable),
>>>>>>>>>> I got following messages:
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> /***************/
>>>>>>>>>> Thread 31933: (state = BLOCKED)
>>>>>>>>>> libsaproc DEBUG: ptrace(PTRACE_GETREGS, ...) not supported
>>>>>>>>>> Error occurred during stack walking:
>>>>>>>>>> sun.jvm.hotspot.debugger.DebuggerException: sun.jvm.hotspot.debugger.DebuggerException: get_thread_regs failed for a lwp
>>>>>>>>>> /***************/
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> In order to fix this problem, I made a patch for preprocessor
>>>>>>>>>> macro in "agent/src/os/linux/ps_proc.c" .
>>>>>>>>>> The patch that attached this mail works well on Fedora 13 x86_64.
>>>>>>>>>>
>>>>>>>>>> Please merge this patch if you don't fix this problem yet.
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Best regards.
>>>>>>>>>>
>>>>>>>>>>
>

More information about the serviceability-dev mailing list