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

Wed Nov 6 14:31:13 UTC 2019

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 at ...
>>> 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
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>
>>>>
>>>