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

Dean Long dean.long at oracle.com
Wed Dec 3 17:51:26 UTC 2014


On 12/3/2014 2:47 AM, Thomas Stüfe wrote:
> 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.
>
Yes, I was thinking inline assembly or assembly in a .S file, but didn't 
consider CPUs which span various OSes.


I don't have a strong opinion about the approach, except I would like to 
avoid ifdefs in shared code, and
using memcpy to new memory, we may need to flush the icache after. And 
on aarch64, we may need to
flush the pipeline of each processor.  I think the aarch64 port does 
this at safepoints.

I'm not sure, but you may be able to generate the code using the 
MacroAssembler early in initialization, even
before StubRoutines are generated.  If so, then that solves the per 
os_cpu combination problem, but not
the icache/pipeline flushing problem.

dl

> Kind regards, Thomas
>
> On Wed, Dec 3, 2014 at 5:09 AM, Dean Long <dean.long at oracle.com 
> <mailto: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/
>         <http://cr.openjdk.java.net/%7Estuefe/webrevs/8065895/webrev.03/>
>
>         Kind Regards, Thomas
>
>
>         On Tue, Dec 2, 2014 at 10:50 AM, David Holmes
>         <david.holmes at oracle.com <mailto: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/
>                 <http://cr.openjdk.java.net/%7Estuefe/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>
>                 <mailto: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.
>
>
>
>
>
>



More information about the hotspot-runtime-dev mailing list