[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-serviceability-dev
Subject:    Re: PING: RFR: 8234624: jstack mixed mode should refer DWARF
From:       Yasumasa Suenaga <suenaga () oss ! nttdata ! com>
Date:       2019-12-12 1:07:37
Message-ID: 635516c4-8f89-c9e6-75c4-9debd8a315be () oss ! nttdata ! com
[Download RAW message or body]

Hi Dmitry,

Thanks for your comment!

On 2019/12/12 0:34, Dmitry Samersoff wrote:
> Hello Yasumasa,
> 
> Please,
> 
> 1. Consider to use mmap for reading elf sections.

Did you pointed `read_section_data()`?

   lib->eh_frame.data = read_section_data(lib->fd, &ehdr, sh);

I do not change implementation of `read_section_data()`.
If you want to change to use mmap, I think it should be fixed as another issue.


> 2. Please move all platfrom-specific parts of native code to a separate
> file/directory. Current patch will brake AARCH64 build.

Unfortunately JDK libraries (shared libraries excepts HotSpot) seem not to care CPU type in makefiles.

   http://hg.openjdk.java.net/jdk/jdk/file/f22d91b2d072/make/common/JdkNativeCompilation.gmk#l38

I believe my patch do not call platform-specific function(s).
Can you share your concern?


> 3. I didn't find any tests here. How did your test the changes?

It can be tested in TestJhsdbJstackMixed and ClhsdbPstack whether mixed jstack can work without error.

We can add the test whether native frames exist in the result, but I found same issue on Windows.
So I do not want to add it now.


> libproc_impl.c
> 
> 131: If is not necessary, free handles NULLPTR gracefully.

Thanks, I will fix it.


Yasumasa


> -Dmitry
> 
> 
> On 04.12.19 03:54, Yasumasa Suenaga wrote:
>> PING: Could you review it?
>>
>>     JBS: https://bugs.openjdk.java.net/browse/JDK-8234624
>>     webrev: http://cr.openjdk.java.net/~ysuenaga/JDK-8234624/webrev.01/
>>
>> This bug is targeted to JDK 14.
>>
>>
>> Thanks,
>>
>> Yasumasa
>>
>>
>> On 2019/11/28 21:39, Yasumasa Suenaga wrote:
>>> Hi,
>>>
>>> I refactored LinuxAMD64CFrame.java . It works fine in
>>> serviceability/sa tests and
>>> all tests on submit repo
>>> (mach5-one-ysuenaga-JDK-8234624-2-20191128-0928-7059923).
>>> Could you review new webrev?
>>>
>>>       http://cr.openjdk.java.net/~ysuenaga/JDK-8234624/webrev.01/
>>>
>>> The diff from previous webrev is here:
>>>       http://hg.openjdk.java.net/jdk/submit/rev/4bc47efbc90b
>>>
>>>
>>> Thanks,
>>>
>>> Yasumasa
>>>
>>>
>>> On 2019/11/25 14:08, Yasumasa Suenaga wrote:
>>>> Hi all,
>>>>
>>>> Please review this change:
>>>>
>>>>       JBS: https://bugs.openjdk.java.net/browse/JDK-8234624
>>>>       webrev: http://cr.openjdk.java.net/~ysuenaga/JDK-8234624/webrev.00/
>>>>
>>>>
>>>> According to 2.7 Stack Unwind Algorithm in System V Application
>>>> Binary Interface AMD64
>>>> Architecture Processor Supplement [1], we need to use DWARF in
>>>> .eh_frame or .debug_frame
>>>> for stack unwinding.
>>>>
>>>> As JDK-8022183 said, omit-frame-pointer is enabled by default since
>>>> GCC 4.6, so system
>>>> library (e.g. libc) might be compiled with this feature.
>>>>
>>>> However `jhsdb jstack --mixed` does not do so, it uses base pointer
>>>> register (RBP).
>>>> So it might be lack of stack frames.
>>>>
>>>> I guess JDK-8219201 is caused by same issue.
>>>>
>>>>
>>>> Thanks,
>>>>
>>>> Yasumasa
>>>>
>>>>
>>>> [1]
>>>> https://software.intel.com/sites/default/files/article/402129/mpx-linux64-abi.pdf
>>>>
[prev in list] [next in list] [prev in thread] [next in thread]