RFR(s): 8065895: Synchronous signals during error reporting may terminate or hang VM process
David Holmes
david.holmes at oracle.com
Fri Dec 5 07:05:40 UTC 2014
On 3/12/2014 8:47 PM, 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.
I don't think there is any OS dependency with inline assembly - only
compiler. And I am also concerned that writing code to an executable
page will also enter the realm of "self-modifying code" and all the
jumping through hoops that entails. That aspect hadn't occurred to me
till Dean raised it. I'm forming the view that triggering a SIGILL is
more effort than it is worth for a secondary testing function.
I should also add that some of my cringing was unfounded as I was
mistakenly thinking that you were adding the cpu specific debug files
when you were not.
Side nit: raise() should be os::raise() (though I don't see any
implementation that does anything but raise() ).
David
> 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/~stuefe/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/~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>
> <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