RFR(S) 8193318: ELF decoder should be able to use external debug info file

[Zhengyu Gu]

Hi All,

I updated webrev to add comment to reference the document that this 
implementation is based on.

I would like to stress the importance of this patch to OpenJDK RPM 
distributors (include RedHat).

Due to the limitation of RPM, it is no possible to ship minimal debug 
symbols in OpenJDK RPM production bundle at this time. Which means, we 
won't get meaningful call stack in hs_err report without this patch, it 
makes supporting OpenJDK significantly harder.

 From maintenance point of view, I would like to point out, that it will 
largely fall to RPM distributors (likely, us at RedHat). As I mentioned 
in early email, it has minimal impact to Oracle distribution, as long as 
it contains debug symbols (as it always does), external debug info code 
path should *never* be executed. Therefore, any bugs in the 
implementation, are likely reported back to us (RPM distributors), and 
we will fix them.


Updated webrev: http://cr.openjdk.java.net/~zgu/8193318/werev.01/

Thanks,

-Zhengyu


On 03/14/2018 05:19 PM, Mikael Vidstedt wrote:
> 
>> On Mar 14, 2018, at 10:12 AM, Zhengyu Gu <zgu at redhat.com 
>> <mailto:zgu at redhat.com>> wrote:
>>
>> Hi Mikael,
>>
>> On 03/14/2018 12:42 PM, Mikael Vidstedt wrote:
>>>> On Mar 14, 2018, at 4:52 AM, coleen.phillimore at oracle.com 
>>>> <mailto:coleen.phillimore at oracle.com> wrote:
>>>>
>>>>
>>>> Hi, I looked at this and I really wasn't happy with all this extra 
>>>> code going into the JVM.  And I don't know this code.  There's a 
>>>> large hex table in this file:
>>>>
>>>> http://cr.openjdk.java.net/~zgu/8193318/webrev.00/src/hotspot/os/linux/decoder_linux.cpp.udiff.html 
>>>> <http://cr.openjdk.java.net/~zgu/8193318/webrev.00/src/hotspot/os/linux/decoder_linux.cpp.udiff.html>
>>> I’m also curious - what is that code actually doing? Why is that hex 
>>> table needed?
>>
>> I should have added comment here, as Andrew did in SA implementation.
>>
>> https://sourceware.org/gdb/current/onlinedocs/gdb/Separate-Debug-Files.html#Separate-Debug-Files
> 
> Ah, that certainly helps, thanks!
> 
> Cheers,
> Mikael
> 
>>
>> Thanks,
>>
>> -Zhengyu
>>
>>> Cheers,
>>> Mikael
>>>>
>>>> This seems like something that will be painful to maintain.  Can the 
>>>> SA be used for the cases where the binary is stripped?  I believe 
>>>> that Oracle ships with minimal stripping so that we have these 
>>>> symbols, why don't all suppliers?   I don't really like this change.
>>>>
>>>> Thanks,
>>>> Coleen
>>>>
>>>> On 3/8/18 11:12 AM, Zhengyu Gu wrote:
>>>>> Ping! I can get formal review?
>>>>>
>>>>> Thanks,
>>>>>
>>>>> -Zhengyu
>>>>>
>>>>>
>>>>> On 02/23/2018 12:26 PM, Zhengyu Gu wrote:
>>>>>> Thanks, Aleksey.
>>>>>>
>>>>>> On 02/23/2018 12:01 PM, Aleksey Shipilev wrote:
>>>>>>> On 02/15/2018 06:59 PM, Zhengyu Gu wrote:
>>>>>>>> This Linux only patch allows OpenJDK Linux distributions, which 
>>>>>>>> strip out debug symbols in
>>>>>>>> production bundles, to use external debuginfo bundles, if 
>>>>>>>> present, to decode stacks.
>>>>>>>>
>>>>>>>> The implementation mirrors SA implementation [1][2]. It has 
>>>>>>>> minimal impact to Oracle's JDK, which
>>>>>>>> always contains symbol tables.
>>>>>>>>
>>>>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8193318
>>>>>>>> Webrev: http://cr.openjdk.java.net/~zgu/8193318/webrev.00/
>>>>>>>
>>>>>>> Looks OK on my untrained eye.
>>>>>>>
>>>>>>> *) I am wondering if we can merge the existing implementation in
>>>>>>> src/jdk.hotspot.agent/linux/native/libsaproc/symtab.c and this 
>>>>>>> mirror implementation in
>>>>>>> src/hotspot/os/linux/decoder_linux.cpp? Notably, moving 
>>>>>>> gnu_debuglink_crc32 to some shared file
>>>>>>> would trim down the change.
>>>>>>
>>>>>> Yes, but I don't know if there is a way.
>>>>>>
>>>>>>>
>>>>>>> Andrew Haley knows this better, as he is the author of the 
>>>>>>> related change.
>>>>>>
>>>>>> Cc'ed Andrew Haley.
>>>>>>
>>>>>> -Zhengyu
>>>>>>
>>>>>>
>>>>>>
>>>>>>>
>>>>>>> Thanks,
>>>>>>> -Aleksey
>>>>>>>
>>>>
>