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" . >>>>>>> >>>>>>> ----------------- >>>>>>> [r...@rhel4-4 test]# cat /etc/redhat-release >>>>>>> Red Hat Enterprise Linux ES release 4 (Nahant Update 4) >>>>>>> [r...@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 >>>>>>> [r...@rhel4-4 test]# rpm -q glibc >>>>>>> glibc-2.3.4-2.25 >>>>>>> [r...@rhel4-4 test]# rpm -q gcc >>>>>>> gcc-3.4.6-3 >>>>>>> [r...@rhel4-4 test]# touch test.h >>>>>>> [r...@rhel4-4 test]# cpp -dM test.h | grep _LP64 >>>>>>> [r...@rhel4-4 test]# >>>>>>> ----------------- >>>>>>> >>>>>>> >>>>>>> So, you need to modify the patch as follows: >>>>>>> >>>>>>> ----------------- >>>>>>> [r...@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: >>>>>>>>> >>>>>>>>> [ke...@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. >>>>>>>>>> >>>>>>>>>> >
