RFR(S): 8187091: ReturnBlobToWrongHeapTest fails because of problems in CodeHeap::contains_blob()

[Volker Simonis]

Hi,

can I please have a review and sponsor for the following small fix:

http://cr.openjdk.java.net/~simonis/webrevs/2017/8187091/
https://bugs.openjdk.java.net/browse/JDK-8187091

We see failures in
test/compiler/codecache/stress/ReturnBlobToWrongHeapTest.java which
are cause by problems in CodeHeap::contains_blob() for corner cases
with CodeBlobs of zero size:

# A fatal error has been detected by the Java Runtime Environment:
#
# Internal Error (heap.cpp:248), pid=27586, tid=27587
# guarantee((char*) b >= _memory.low_boundary() && (char*) b <
_memory.high()) failed: The block to be deallocated 0x00007fffe6666f80
is not within the heap starting with 0x00007fffe6667000 and ending
with 0x00007fffe6ba000

The problem is that JDK-8183573 replaced

  virtual bool contains_blob(const CodeBlob* blob) const { return
low_boundary() <= (char*) blob && (char*) blob < high(); }

by:

  bool contains_blob(const CodeBlob* blob) const { return
contains(blob->code_begin()); }

But that my be wrong in the corner case where the size of the
CodeBlob's payload is zero (i.e. the CodeBlob consists only of the
'header' - i.e. the C++ object itself) because in that case
CodeBlob::code_begin() points right behind the CodeBlob's header which
is a memory location which doesn't belong to the CodeBlob anymore.

This exact corner case is exercised by ReturnBlobToWrongHeapTest which
allocates CodeBlobs of size zero (i.e. zero 'payload') with the help
of sun.hotspot.WhiteBox.allocateCodeBlob() until the CodeCache fills
up. The test first fills the 'non-profiled nmethods' CodeHeap. If the
'non-profiled nmethods' CodeHeap is full, the VM automatically tries
to allocate from the 'profiled nmethods' CodeHeap until that fills up
as well. But in the CodeCache the 'profiled nmethods' CodeHeap is
located right before the non-profiled nmethods' CodeHeap. So if the
last CodeBlob allocated from the 'profiled nmethods' CodeHeap has a
payload size of zero and uses all the CodeHeaps remaining size, we
will end up with a CodeBlob whose code_begin() address will point
right behind the actual CodeHeap (i.e. it will point right at the
beginning of the adjacent, 'non-profiled nmethods' CodeHeap). This
will result in the above guarantee to fire, when we will try to free
the last allocated CodeBlob (with
sun.hotspot.WhiteBox.freeCodeBlob()).

In a previous mail thread
(http://mail.openjdk.java.net/pipermail/hotspot-dev/2017-August/028175.html)
Vladimir explained why JDK-8183573 was done:

> About contains_blob(). The problem is that AOTCompiledMethod allocated in CHeap and not in aot code section (which is RO):
>
> http://hg.openjdk.java.net/jdk10/hs/hotspot/file/8acd232fb52a/src/share/vm/aot/aotCompiledMethod.hpp#l124
>
> It is allocated in CHeap after AOT library is loaded. Its code_begin() points to AOT code section but AOTCompiledMethod*
> points outside it (to normal malloced space) so you can't use (char*)blob address.

and proposed these two fixes:

> There are 2 ways to fix it, I think.
> One is to add new field to CodeBlobLayout and set it to blob* address for normal CodeCache blobs and to code_begin for
> AOT code.
> Second is to use contains(blob->code_end() - 1) assuming that AOT code is never zero.

I came up with a slightly different solution - just use
'CodeHeap::code_blob_type()' whether to use 'blob->code_begin()' (for
the AOT case) or '(void*)blob' (for all other blobs) as input for the
call to 'CodeHeap::contain()'. It's simple and still much cheaper than
a virtual call. What do you think?

I've also updated the documentation of the CodeBlob class hierarchy in
codeBlob.hpp. Please let me know if I've missed something.

Thank you and best regards,
Volker