RFR: 8251152: ARM32: jtreg c2 Test8202414 test crash

[Igor Ignatyev]

Hi Filipp,

> On Sep 4, 2020, at 12:46 AM, Filipp Zhinkin <filipp.zhinkin at gmail.com> wrote:
> 
> Hi,
> 
> updated webrev: http://cr.openjdk.java.net/~bulasevich/fzhinkin/8251152/webrev.2/ <http://cr.openjdk.java.net/~bulasevich/fzhinkin/8251152/webrev.2/> (I've updated years in the copyright).

I don't know the format used in SAP copyright notices, so I can't really tell if you should have added ", 2020" there, this I'd suggest you to seek comments from SAP folks.

> IIRC it's enough to have only one reviewer for such a small change, right?
right, one reviewer is sufficient for "trivial" changes, but as I said I can't really review changes in SAP copyright line.

Cheers,
-- Igor

> 
> Thanks,
> Filipp.
> 
> 
> On Wed, 2 Sep 2020 at 21:53, Igor Ignatyev <igor.ignatyev at oracle.com <mailto:igor.ignatyev at oracle.com>> wrote:
> 
> 
>> On Sep 2, 2020, at 1:06 AM, Filipp Zhinkin <filipp.zhinkin at gmail.com <mailto:filipp.zhinkin at gmail.com>> wrote:
>> 
>> Hi,
>> 
>> updated webrev: http://cr.openjdk.java.net/~bulasevich/fzhinkin/8251152/webrev.1/ <http://cr.openjdk.java.net/~bulasevich/fzhinkin/8251152/webrev.1/>
>> 
>> Tests are throwing SkippedException now, as Igor suggested.
> thanks, LGTM.
> 
>> Seems like I should also update a year in JdkInternalMiscUnsafeUnalignedAccess' copyright, but I'm not sure if there should be a line designating Oracle as intellectual property owner along with SAP. Should I add it too?
> IANAL, but unless the file was modified by Oracle, it doesn't have to have Oracle copyright notice.
> 
> Thanks,
> -- Igor
>> 
>> Thanks,
>> Filipp.
>> 
>> On Tue, 1 Sep 2020 at 22:48, Filipp Zhinkin <filipp.zhinkin at gmail.com <mailto:filipp.zhinkin at gmail.com>> wrote:
>> Hi Igor,
>> 
>> On Tue, 1 Sep 2020 at 19:46, Igor Ignatyev <igor.ignatyev at oracle.com <mailto:igor.ignatyev at oracle.com>> wrote:
>> Hi Filipp,
>> 
>> 1st of all, welcome back!
>> 
>> thanks!
>>  
>> 
>> it would be better to throw jtreg.SkippedException at L#46 so jtreg will reported the test as skipped (as opposed to just passed).
>> Thanks, I'll update Test8202414 as well as compiler/unsafe/JdkInternalMiscUnsafeUnalignedAccess (which also skips execution the same way).
>>  
>> alternative, you could use '@requires vm.simpleArch != "arm"' to exclude the test from arm32 execution.
>> 
>> I was thinking about adding something like vm.unalignedAccess.enabled, but it seems to be too complicated solution for two tests (Test8202414 and compiler/unsafe/JdkInternalMiscUnsafeUnalignedAccess).
>> I don't want to use 'simpleArch', because if some new platform missing unaligned access support will be added in the future then someone will have to spend time to find out why a test crashes.
>> 
>> Thanks,
>> Filipp.
>>  
>> 
>> Thanks,
>> -- Igor
>> 
>> 
>> > On Sep 1, 2020, at 8:29 AM, Filipp Zhinkin <filipp.zhinkin at gmail.com <mailto:filipp.zhinkin at gmail.com>> wrote:
>> > 
>> > Hi,
>> > 
>> > Test8202414 crashes on ARM32 while writing to memory using an unaligned
>> > address.
>> > ARM32 supports unaligned memory accesses for some load/store instructions
>> > under certain conditions, but LDRD (which is used when we're calling
>> > Unsafe::putLong) is always causing alignment fault when called with an
>> > unaligned address [1].
>> > 
>> > The fix is simply skipping the test execution if a platform does not
>> > support unaligned memory accesses.
>> > 
>> > Bug: https://bugs.openjdk.java.net/browse/JDK-8251152 <https://bugs.openjdk.java.net/browse/JDK-8251152>
>> > Webrev: http://cr.openjdk.java.net/~bulasevich/fzhinkin/8251152/webrev.0/ <http://cr.openjdk.java.net/~bulasevich/fzhinkin/8251152/webrev.0/>
>> > 
>> > [1] ARM Architecture Reference Manual ARMv7-A and ARMv7-R edition, §A3.2.1
>> > Unaligned data access https://developer.arm.com/documentation/ddi0406/cd <https://urldefense.com/v3/__https://developer.arm.com/documentation/ddi0406/cd__;!!GqivPVa7Brio!LmT9wbXnxT5H1XrOZaO8lKj3bT1acuEZPfqS39n-zC99pnrihbCQokSh2J1jqf-MSEE$>
>> > 
>> > Thanks,
>> > Filipp.
>> 
>