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

List:       openjdk-hotspot-dev
Subject:    Re: RFR(S): 8229254: solaris_x64 build fails
From:       Boris Ulasevich <boris.ulasevich () bell-sw ! com>
Date:       2019-08-19 12:50:46
Message-ID: 796cbaa2-51ca-b899-9ea4-d8d82ba784ac () bell-sw ! com
[Download RAW message or body]

Hi David,
   thank you!

On 17.08.2019 1:37, David Holmes wrote:
> Hi Boris,
> 
> On 17/08/2019 12:40 am, Boris Ulasevich wrote:
>> Hi David,
>>
>>      Do you think the change is Ok or should I rework it somehow?
> 
> The change is fine.
> 
> Thanks,
> David
> 
>> thanks,
>> Boris
>>
>> On 12.08.2019 11:53, Boris Ulasevich wrote:
>>> Hi David,
>>>
>>> On 12.08.2019 5:47, David Holmes wrote:
>>>> Hi Boris,
>>>>
>>>> On 9/08/2019 11:14 pm, Boris Ulasevich wrote:
>>>>> Hi,
>>>>>
>>>>> Please review the following simple change to make solaris_x64 build 
>>>>> compilable.
>>>>>
>>>>> Syntax error was brought by JDK-8191278 [1]. The fix is evident.
>>>>
>>>> Yes looks good.
>>>>
>>>>> Missing constant issue occurred after JDK-8226238 [2]. I copied 
>>>>> workaround for the case of missing EM_AARCH64 constant in system 
>>>>> elf.h header from os_linux.cpp implementation.
>>>>
>>>> I'm really not sure why we keep expanding a Solaris specific 
>>>> structure with entries for architectures the OS does not run on ...
>>>
>>> Yes. I had the same concern. The first impulse was to remove the 
>>> AARCH64 data line or to surround it #ifdef AARCH directive.
>>>
>>> The sense of this line on os_solaris.cpp implementation might be a 
>>> good question on JDK-8226238 review. I think the answer is that it 
>>> makes no overhead, and potentially improves .so library compatibility 
>>> error message.
>>>
>>>> That aside the fix seems fine - the value is somewhat arbitrary but 
>>>> doesn't conflict with anything AFAICS.
>>>
>>> My explanation was not quite intelligible, sorry. As Kim have already 
>>> answered, this is as constant that should normally come from elf.h 
>>> header. But as Solaris system header elf.h does not have that, this 
>>> is just a workaround for the system header on build machine, very 
>>> similar to that in os_linux.cpp implementation.
>>>
>>>> Thanks,
>>>> David
>>>> -----
>>>>
>>>>> http://cr.openjdk.java.net/~bulasevich/8229254/webrev.00
>>>>> http://bugs.openjdk.java.net/browse/JDK-8229254
>>>>>
>>>>> Testing done: tier1,tier2.
>>>>>
>>>>> thanks,
>>>>> Boris
>>>>>
>>>>> [1] MappedByteBuffer bulk access memory failures are not handled 
>>>>> gracefully
>>>>>
>>>>> http://mail.openjdk.java.net/pipermail/hotspot-dev/2019-June/038352.html 
>>>>>
>>>>> [2] Improve error output and fix elf issues in os::dll_load
>>>>>
>>>>> http://mail.openjdk.java.net/pipermail/hotspot-dev/2019-June/038597.html 
>>>>>
[prev in list] [next in list] [prev in thread] [next in thread]