assert for polling page reference / bug in reachable() ?

[Salter, Thomas A]

Thanks for the response.

It's good to know I was looking at the wrong code rather than not being able to read the code I was looking at.

Tom


-----Original Message-----
From: Tom Rodriguez [mailto:tom.rodriguez at oracle.com]
Sent: Monday, June 28, 2010 2:40 PM
To: Salter, Thomas A
Cc: hotspot-compiler-dev at openjdk.java.net
Subject: Re: assert for polling page reference / bug in reachable() ?


On Jun 24, 2010, at 7:27 AM, Salter, Thomas A wrote:

> I recently managed to get hotspot to generate a RIP-relative address for a location that was more than 2**32 bytes away.  The displacement simply got truncated and stored as a 32-bit offset.
>
> This was the offending machine code (taken from a Windows dump):
>
> 00000000706A7734 85 05 C6 88 31 8F test        dword ptr [0FFFFFFFFFF9C0000h],eax
> 00000000706A773A C3                ret
>
> After much searching I determined the code originated in c1_LIRAssembler_x86.cpp
>
>  // NOTE: the requires that the polling page be reachable else the reloc
>  // goes to the movq that loads the address and not the faulting instruction
>  // which breaks the signal handler code
>
>  __ test32(rax, polling_page);
>
>  __ ret(0);

That code is only used when running client and it actually uses reachable so I don't think that's the source of your crash.  I think it's actually this code from x86_64.ad:

  enc_class enc_safepoint_poll
  %{
    // testl %rax, off(%rip) // Opcode + ModRM + Disp32 == 6 bytes
    // XXX reg_mem doesn't support RIP-relative addressing yet
    cbuf.set_inst_mark();
    cbuf.relocate(cbuf.inst_mark(), relocInfo::poll_type, 0); // XXX
    emit_opcode(cbuf, 0x85); // testl
    emit_rm(cbuf, 0x0, RAX_enc, 0x5); // 00 rax 101 == 0x5
    // cbuf.inst_mark() is beginning of instruction
    emit_d32_reloc(cbuf, os::get_polling_page());
//                    relocInfo::poll_type,
  %}

This version doesn't check reachable and it should.  Actually for c2 we should just have a poll variant that forces the value into a register if it's not reachable.  I filed 6964776 for this.

tom

>
>
> It seems to me that the requirement in the NOTE could be captured in an assert so that hotspot would fail predictably, rather than with a memory reference exception.
>
> This occurred on Windows 7, 64-bit OS.
>
> The root cause was an "optimization" I had made to the calls to VirtualAlloc in os_windows.cpp in os::reserve_memory and os::init_2.  In order to reduce fragmentation of the address space in the 32-bit JVM, I implemented a simple best-fit alogorithm so that small page allocations could use the gaps between the Windows DLLs in the higher address ranges.  This helped with the 32-bit JVM, but is completely unnecessary for the 64-bit.
>
> Unfortunately, on the 64-bit JVM, this change caused the _polling_page address to be 0x7FEFF9C0000.  The RIP used in the test instruction was 0x706A773A.  I could not convince myself how the reachable() procedure thought this address was reachable, but it seems there may be a bug there also.
>
> Even without my change, hotspot appears to depend on the internal algorithms of VirtualAlloc.  If that were to change, this hotspot code could fail in the same way that my modified JVM did.
>
> Tom Salter
> Unisys.
>