[aarch64-port-dev ] RFR: 8234794: AArch64: runtime/memory/ReadFromNoaccessArea.java crashes

[Ioi Lam]

My test run of tiers 1 and 2 is complete. There were no new failures 
caused by webrev.02.

Thumbs up.

- Ioi

On 12/17/19 7:34 AM, Ioi Lam wrote:
> Hi Nick, sorry for the delay. The changes look good to me.
>
> I am running test tiers 1/2 with our CI pipeline and will report the 
> results back.
>
> Thanks
> - Ioi
>
> On 12/16/19 10:25 PM, Nick Gasson wrote:
>> Hi,
>>
>> Any comments on the most recent webrev below?
>>
>> Thanks,
>> Nick
>>
>> On 12/12/2019 16:31, Nick Gasson wrote:
>>> On 12/12/2019 04:16, Andrew Haley wrote:
>>>> On 12/11/19 7:08 AM, Nick Gasson wrote:
>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8234794
>>>>> Webrev: http://cr.openjdk.java.net/~ngasson/8234794/webrev.01/
>>>>
>>>> Thanks. A few nits:
>>>
>>> Please see the updated webrev:
>>>
>>> http://cr.openjdk.java.net/~ngasson/8234794/webrev.02/
>>>
>>>>
>>>> This recalculates klass_decode_mode() a thousand times on only a small
>>>> program. Be aware that C2 creates a new Assembler instance for most 
>>>> patterns.
>>>>
>>>> _klass_decode_mode could be calculated only once. Make it static 
>>>> and it
>>>> will be.
>>>
>>> Yes. Also added an assert that Metaspace::initialized() in case we 
>>> call it too early.
>>>
>>>>
>>>> This inverted logic:
>>>>
>>>> #if !(defined(AARCH64) || defined(AIX))
>>>>    return ReservedSpace(size, alignment, large_pages, requested_addr);
>>>> #else // AARCH64 || AIX
>>>>
>>>> looks a bit cumbersome. It might be nice to do it the other way up, so
>>>> that reserve_preferred_space is called from a static generic allocate
>>>> method.
>>>>
>>>> Rather than explicitly defined(AARCH64) || defined(AIX) you might 
>>>> like to
>>>> consider a named macro
>>>>
>>>> #if OS_CPU_USES_PREFERRED_SPACE
>>>>    return reserve_preferred_space( ...
>>>> endif
>>>>
>>>
>>> OK. Metaspace::reserve_preferred_space() is now only defined if 
>>> PREFERRED_METASPACE_ALIGNMENT is set. The generic method is 
>>> Metaspace::reserve_space(). MetaspaceShared::reserve_shared_space() 
>>> is now just a trivial wrapper for this so maybe it can be removed.
>>>
>>> I put the definition in the CPU's globalDefinitions.hpp because we 
>>> don't have one for os_cpu, and we need it for every AArch64 system 
>>> anyway.
>>>
>>> Also fixed the break/return inconsistency suggested by Jiangli.
>>>
>>>
>>> Thanks,
>>> Nick
>