'Re: RFR: 8233600: cross-builds fails after JDK-8233285'

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

List:       openjdk-serviceability-dev
Subject:    Re: RFR: 8233600: cross-builds fails after JDK-8233285
From:       Boris Ulasevich <boris.ulasevich () bell-sw ! com>
Date:       2019-11-06 16:27:57
Message-ID: 6a7f785c-d9cb-0658-b4b6-09f126f21303 () bell-sw ! com
[Download RAW message or body]

Thank you!

On 06.11.2019 19:18, Erik Joelsson wrote:
> Looks good! Verified the same patch with all our available cross compile 
> builds.
> 
> /Erik
> 
> On 2019-11-06 06:31, Boris Ulasevich wrote:
>> Hi,
>>
>> Indeed, the fix is quite evident. I checked it works for arm32/aarch 
>> cross-compilation builds.
>>
>> http://bugs.openjdk.java.net/browse/JDK-8233600
>> http://cr.openjdk.java.net/~bulasevich/8233600/webrev.00
>>
>> regards,
>> Boris
>>
>> On 06.11.2019 16:33, Erik Joelsson wrote:
>>> I looked closer at it now and the build change is not good. Any 
>>> toolchain definition with BUILD in the name, like 
>>> TOOLCHAIN_BUILD_LINK_CXX, is only meant to be used for building tools 
>>> that are run during the build. I believe the fix is to just remove 
>>> the "BUILD_".
>>>
>>> /Erik
>>>
>>> On 2019-11-06 05:13, David Holmes wrote:
>>>> On 4/11/2019 8:27 pm, Magnus Ihse Bursie wrote:
>>>>> On 2019-11-02 13:43, Daniel D. Daugherty wrote:
>>>>>> Since this review contains build changes, I've added build-dev@...
>>>>> Thanks Dan for noticing this and cc:ing us.
>>>>>
>>>>> Yasumasa: build changes look fine. Thanks.
>>>>
>>>> This change broke all cross-compilation.
>>>>
>>>> David
>>>>
>>>>> /Magnus
>>>>>>
>>>>>> Dan
>>>>>>
>>>>>>
>>>>>> On 11/1/19 4:56 AM, Yasumasa Suenaga wrote:
>>>>>>> (Changed subject to review request)
>>>>>>>
>>>>>>> Hi,
>>>>>>>
>>>>>>> I converted LinuxDebuggerLocal.c to C++ code, and it works fine 
>>>>>>> on submit repo.
>>>>>>> (mach5-one-ysuenaga-JDK-8233285-1-20191101-0746-6336009)
>>>>>>>
>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8233285/webrev.00/
>>>>>>>
>>>>>>> Could you review it?
>>>>>>>
>>>>>>>
>>>>>>> Thanks,
>>>>>>>
>>>>>>> Yasumasa
>>>>>>>
>>>>>>>
>>>>>>> On 2019/11/01 8:54, Yasumasa Suenaga wrote:
>>>>>>>> Hi David,
>>>>>>>>
>>>>>>>> On 2019/11/01 7:55, David Holmes wrote:
>>>>>>>>> Hi Yasumasa,
>>>>>>>>>
>>>>>>>>> New build dependencies cannot be added lightly. This impacts 
>>>>>>>>> everyone who maintains build/test farms.
>>>>>>>>
>>>>>>>> Ok, thanks for telling it.
>>>>>>>>
>>>>>>>>> We already use the C++ demangling capabilities in the VM. Is 
>>>>>>>>> there some way to export that for use by libsaproc ?
>>>>>>>>>
>>>>>>>>> Otherwise using C++ demangle may still be the better choice 
>>>>>>>>> given we already have it as a dependency.
>>>>>>>>
>>>>>>>> I found abi::__cxa_demangle() is used in ElfDecoder::demangle() 
>>>>>>>> at decoder_linux.cpp .
>>>>>>>> It is similar with my original proposal.
>>>>>>>>
>>>>>>>>>>>>>>> http://cr.openjdk.java.net/~ysuenaga/sa-demangle/
>>>>>>>>
>>>>>>>> I agree with David to use C++ demangle way.
>>>>>>>> However we need to choice the fix from following:
>>>>>>>>
>>>>>>>>      A. Convert LinuxDebuggerLocal.c to C++ code
>>>>>>>>      B. Add C++ code for libsaproc.so to demangle symbols.
>>>>>>>>
>>>>>>>> I've discussed with Chris about it in [1].
>>>>>>>> Option A might be large change.
>>>>>>>>
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>>
>>>>>>>> Yasumasa
>>>>>>>>
>>>>>>>>
>>>>>>>> [1] 
>>>>>>>> https://mail.openjdk.java.net/pipermail/serviceability-dev/2019-October/029716.html 
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>> David
>>>>>>>>>
>>>>>>>>> On 1/11/2019 12:58 am, Chris Plummer wrote:
>>>>>>>>>> Hi Yasumasa,
>>>>>>>>>>
>>>>>>>>>> Here's the failure during configure:
>>>>>>>>>>
>>>>>>>>>> [2019-10-31T06:07:45,131Z] checking demangle.h usability... no
>>>>>>>>>> [2019-10-31T06:07:45,150Z] checking demangle.h presence... no
>>>>>>>>>> [2019-10-31T06:07:45,150Z] checking for demangle.h... no
>>>>>>>>>> [2019-10-31T06:07:45,151Z] configure: error: Could not find 
>>>>>>>>>> demangle.h! You might be able to fix this by running 'sudo yum 
>>>>>>>>>> install binutils-devel'.
>>>>>>>>>>
>>>>>>>>>> Chris
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> On 10/31/19 1:08 AM, Yasumasa Suenaga wrote:
>>>>>>>>>>> Hi,
>>>>>>>>>>>
>>>>>>>>>>> I filed this enhancement to JBS:
>>>>>>>>>>>
>>>>>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8233285
>>>>>>>>>>>
>>>>>>>>>>> Also I pushed the changes to submit repo, but it was failed.
>>>>>>>>>>>
>>>>>>>>>>> http://hg.openjdk.java.net/jdk/submit/rev/bfbc49233c26
>>>>>>>>>>> http://hg.openjdk.java.net/jdk/submit/rev/430e4f65ef25
>>>>>>>>>>>
>>>>>>>>>>> According to the email from Mach 5, dependency errors were 
>>>>>>>>>>> occurred in jib.
>>>>>>>>>>> Can someone share the details?
>>>>>>>>>>> I'm not familiar in jib, so I want help.
>>>>>>>>>>>
>>>>>>>>>>> mach5-one-ysuenaga-JDK-8233285-20191031-0606-6301426
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> Thanks,
>>>>>>>>>>>
>>>>>>>>>>> Yasumasa
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> On 2019/10/31 11:23, Chris Plummer wrote:
>>>>>>>>>>>> You can change the configure script. I don't know if there's 
>>>>>>>>>>>> any concerns with using libiberty.a. That's possibly a legal 
>>>>>>>>>>>> question (GNU GPL). You might want to ask that on jdk-dev 
>>>>>>>>>>>> and/or build-dev.
>>>>>>>>>>>>
>>>>>>>>>>>> Chris
>>>>>>>>>>>>
>>>>>>>>>>>> On 10/30/19 7:14 PM, Yasumasa Suenaga wrote:
>>>>>>>>>>>>> Hi Chris,
>>>>>>>>>>>>>
>>>>>>>>>>>>> Thanks for quick reply!
>>>>>>>>>>>>>
>>>>>>>>>>>>> If we convert LinuxDebuggerLocal.c to C++ code, we have to 
>>>>>>>>>>>>> convert a lot of JNI calls to C++ style.
>>>>>>>>>>>>> For example:
>>>>>>>>>>>>>
>>>>>>>>>>>>>    (*env)->FindClass(env, "java/lang/String")
>>>>>>>>>>>>>            to
>>>>>>>>>>>>>    env->FindClass("java/lang/String")
>>>>>>>>>>>>>
>>>>>>>>>>>>> Can it be accepted?
>>>>>>>>>>>>>
>>>>>>>>>>>>> OTOH I said in my email, to use cplus_demangle(), we need 
>>>>>>>>>>>>> to link libiberty.a which is provided by binutils. Thus I 
>>>>>>>>>>>>> think we need to check libiberty.a in configure script. Is 
>>>>>>>>>>>>> it ok?
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> I prefer to use cplus_demangle() if we can change configure 
>>>>>>>>>>>>> script.
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> Yasumasa
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> On 2019/10/31 11:03, Chris Plummer wrote:
>>>>>>>>>>>>>> Hi Yasumasa,
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> I don't have concerns with adding C++ source to SA, but in 
>>>>>>>>>>>>>> order to do so you've put the new native code in its own 
>>>>>>>>>>>>>> file rather than in LinuxDebuggerLocal.c. I'd like to see 
>>>>>>>>>>>>>> that resolved. So either convert LinuxDebuggerLocal.c to 
>>>>>>>>>>>>>> C++, or use cplus_demangle().
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> thanks,
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Chris
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> On 10/30/19 6:54 PM, Yasumasa Suenaga wrote:
>>>>>>>>>>>>>>> Hi all,
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> I saw C++ frames in `jhsdb jstack --mixed`, and they were 
>>>>>>>>>>>>>>> mangled as below:
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> 0x00007ff255a8fa4c 
>>>>>>>>>>>>>>> _ZN9JavaCalls11call_helperEP9JavaValueRK12methodHandleP17JavaCallArgumentsP6Thread 
>>>>>>>>>>>>>>> + 0x6ac
>>>>>>>>>>>>>>> 0x00007ff255a8cc1d 
>>>>>>>>>>>>>>> _ZN9JavaCalls12call_virtualEP9JavaValueP5KlassP6SymbolS5_P17JavaCallArgumentsP6Thread 
>>>>>>>>>>>>>>> + 0x33d
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> We can demangle them via c++filt, but I think it is more 
>>>>>>>>>>>>>>> convenience if jstack can show demangling symbols.
>>>>>>>>>>>>>>> I think we can demangle in jstack with this patch. If it 
>>>>>>>>>>>>>>> is accepted, I will file it to JBS and send review request.
>>>>>>>>>>>>>>> What do you think?
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> http://cr.openjdk.java.net/~ysuenaga/sa-demangle/
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> We can get the stack as below after applying this patch:
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> 0x00007ff1aba20a4c JavaCalls::call_helper(JavaValue*, 
>>>>>>>>>>>>>>> methodHandle const&, JavaCallArguments*, Thread*) + 0x6ac
>>>>>>>>>>>>>>> 0x00007ff1aba1dc1d JavaCalls::call_virtual(JavaValue*, 
>>>>>>>>>>>>>>> Klass*, Symbol*, Symbol*, JavaCallArguments*, Thread*) + 
>>>>>>>>>>>>>>> 0x33d
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> I use abi::__cxa_demangle() for demangling, so this patch 
>>>>>>>>>>>>>>> adds C++ source to SA.
>>>>>>>>>>>>>>> If it is not comfortable, we can use cplus_demangle().
>>>>>>>>>>>>>>> But this function is provided by libiberty.a, so we need 
>>>>>>>>>>>>>>> to link it to libsaproc and need to check libiberty.a in 
>>>>>>>>>>>>>>> configure script.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Yasumasa
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
[prev in list] [next in list] [prev in thread] [next in thread]
Configure | About | News | Add a list | Sponsored by KoreLogic