RFR(s): 8065895: Synchronous signals during error reporting may terminate or hang VM process

[Thomas Stüfe]

Hi Dean,

I dont understand. Such a function does not exist, does it? So I would have
to write it:

Do you mean generating and using a StubRoutine which would SIGILL? I did
not do this because I wanted to be able to generate SIGILL also in
initialization code, where StubRoutines may not yet be generated. This
point may may be arguable, but as this function is used to test error
handling, it may be interesting to test it for half-initialized VMs too.

Otherwise I would implement the CPU specific
generate_illegal_instruction_sequence()
probably the same way as I do now the crash_with_sigill() function. That
would mean a bit of more code duplication because:
- Either I use the method I use now (reserve_memory and copy the
instructions to the reserved page)
- Or I use inline assembly - which probably does not work across multiple
OSs, so for CPUs which span various OSs I would have to add one function
per os_cpu combination, not just per cpu.

Kind regards, Thomas

On Wed, Dec 3, 2014 at 5:09 AM, Dean Long <dean.long at oracle.com> wrote:

> Instead of get_illegal_instruction_sequence() that fills in a buffer in
> reserved memory page, how
> about simply generate_illegal_instruction_sequence() that causes the
> SIGILL when executed?
> Then crash_with_sigill() simplifies to something like:
>
>     tty->print_cr("will jump to PC " PTR_FORMAT", which should cause a
> SIGILL.",generate_illegal_instruction_sequence);
>     tty->flush();
>
>     generate_illegal_instruction_sequence(); //  boom
>
> dl
>
>
> On 12/2/2014 8:04 AM, Thomas Stüfe wrote:
>
>> Hi David, you are a hard man to uncringe :)
>>
>> Here is a last modification, which in my opinion would be the best
>> balance.
>> Basically, it is (2) with the CPU dependend code moved away from shared
>> coding and a fallback for CPUs which have no (known) way to cause a
>> SIGILL.
>>
>> http://cr.openjdk.java.net/~stuefe/webrevs/8065895/webrev.03/
>>
>> Kind Regards, Thomas
>>
>>
>> On Tue, Dec 2, 2014 at 10:50 AM, David Holmes <david.holmes at oracle.com>
>> wrote:
>>
>>  On 1/12/2014 11:30 PM, Thomas Stüfe wrote:
>>>
>>>  Hi all,
>>>>
>>>> lets not get this patch bogged down on ARM opcode discussions.
>>>>
>>>> For me, it is just a question of style and which one would be most
>>>> acceptable to the OpenJDK.
>>>>
>>>> As I see it, here are my options:
>>>>
>>>> 1  leave the code as it is and whoever does ARM porting at Oracle will
>>>> provide the SIGILL opcodes inside debug.cpp
>>>> 2  like (1), but provide a fallback for CPUs where we do not know the
>>>> SIGILL opcodes right now, by doing a raise(SIGILL). This would work but
>>>> make the test a tiny bit less valuable on those platforms.
>>>>
>>>> 3  Move the CPU-dependend parts (the big #ifdef) away from debug.cpp
>>>> into debug_<cpu>.cpp. Would mean a bit code duplication because for 3
>>>> out of 5 cpus the SIGILL-generating opcode is 0. This basically would be
>>>> the same as my second webrev
>>>> (http://cr.openjdk.java.net/~stuefe/webrevs/8065895/webrev.01/)
>>>> 4  like (3), but with additional introduction of a debug_<cpu>.hpp, and
>>>> adding a "ZERO_WILL_GENERATE_SIGILL" or somesuch macro to provide a
>>>> common fallback for cpus where 0 generates SIGILL.
>>>>
>>>> I am leaning toward (2) or (3) but I am okay with any of the four.
>>>>
>>>>  I'm really undecided here. #1 makes me cringe because of the cpu
>>> ifdefs in
>>> shared code (including those for non-OpenJDK platforms). #3 and #4 make
>>> me
>>> cringe because it is a lot of overhead to introduce the debug_<cpu>.hpp
>>> files on all platforms.
>>>
>>> That leaves #2 though I'm unclear how we will identify the platforms that
>>> don't have defined bad opcodes. If that's still just a variant of the
>>> ifdefs in #1 then I'm still cringing. :)
>>>
>>> Would appreciate someone else from runtime jumping in with an opinion
>>> here
>>> :)
>>>
>>> David
>>>
>>> (PS. I'm on vacation tomorrow so apologies for delayed responses.)
>>>
>>>
>>>   Kind Regards,
>>>
>>>> Thomas Stuefe
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>> On Thu, Nov 27, 2014 at 12:04 PM, Andrew Haley <aph at redhat.com
>>>> <mailto:aph at redhat.com>> wrote:
>>>>
>>>>      On 11/27/2014 11:00 AM, David Holmes wrote:
>>>>       > On 27/11/2014 8:55 PM, Andrew Haley wrote:
>>>>       >> On 11/27/2014 10:38 AM, Thomas Stüfe wrote:
>>>>       >>> Hi Andrew, thank you! Does endianess matter ?
>>>>       >>
>>>>       >> Yes.  I'd do it symbolically rather than mess with endian
>>>> defines:
>>>>       >>
>>>>       >> #ifdef AARCH64
>>>>       >>    unsigned insn;
>>>>       >>    asm("b 1f; 0: dcps1; 1: ldr %0, 0b" : "=r"(insn));
>>>>       >> #endif
>>>>       >
>>>>       > Does that work for ARMv7?
>>>>
>>>>      Sorry, I don't know what a good choice there would be.  And I must
>>>>      warn you: DCPS1 isn't necessarily guaranteed to do this forever,
>>>> but
>>>>      it works on the kernels I've tried.
>>>>
>>>>      Andrew.
>>>>
>>>>
>>>>
>>>>
>>>>
>