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. >>>>>>>>> >>>>>>>>> >>> >> > -- 日本電信電話株式会社 研究企画部門 OSS センタ 応用技術ユニット Webグループ 末永 恭正（すえなが やすまさ） TEL: 03-5860-5105 (直通 5069) E-mail: [email protected]
