[10] RFR(M): 8176506: C2: loop unswitching and unsafe accesses cause crash

[Volker Simonis]

Sorry, I somehow missed the fact that you're still waiting for my OK.

I think it's good that you now use SIGILL on all platforms. The change
looks good except the following minor nits I've already mentioned in
my first review. I'll leave it up to you if you want to fix them and
in the case you do there's no need for a new webrev.

src/share/vm/opto/graphKit.cpp

// add a check for null for which one branch can't be taken. It uses
// and Opaque4 node that will cause the check to be removed after loop

- should probably read "// an Opaque4 node.." in the second line

src/share/vm/opto/opaquenode.hpp

// know implicitly is always true of false but the compiler has no way
// to prove. If during optimizations, that check becomes true of
// false, the Opaque4 node is replaced by that constant true or

- s/of/or/

test/compiler/unsafe/TestMaybeNullUnsafeAccess.java

- wouldn't it be safer to run the test with -Xbatch and
-XX:-UseOnStackReplacement for any case and to make it evident that
the test relays on the fact that test1() and test2() are both
compiled but not inlined ?

- you should also update the copyright on most files you've touched.

Thank you and best regards,
Volker


On Tue, May 16, 2017 at 6:45 AM, Vladimir Kozlov
<vladimir.kozlov at oracle.com> wrote:
> Looks good to me.
>
> Volker should look too.
>
> Thanks,
> Vladimir
>
>
> On 5/12/17 12:55 AM, Roland Westrelin wrote:
>>
>>
>>> X86 Manual says:
>>>
>>> "Use the 0F0B opcode (UD2 instruction) or the 0FB9H opcode when
>>> deliberately trying to generate an invalid opcode exception (#UD)."
>>
>>
>> Thanks, Vladimir.
>>
>> Here is a new webrev. Halt now should trigger a SIGILL on all
>> platforms. I tested x86, aarch64, arm64. The code for arm32 needs to be
>> tested (I can't even build on arm32).
>>
>> http://cr.openjdk.java.net/~roland/8176506/webrev.04/
>>
>> Roland.
>>
>