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

[Nick Gasson]

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