RFR (XL): 8152664 - Support non-continuous CodeBlobs in HotSpot

Dean Long dean.long at oracle.com
Fri Apr 8 22:02:52 UTC 2016 

Volker, does this patch fix the problem?

http://cr.openjdk.java.net/~dlong/8151956/8151956.patch

dl

On 4/8/2016 1:15 PM, Dean Long wrote:
> I was able to find this:
>
> https://isocpp.org/wiki/faq/strange-inheritance#calling-virtuals-from-ctor-idiom 
>
>
> dl
>
> On 4/8/2016 1:04 PM, Dean Long wrote:
>> Hi Volker.  I noticed this problem before and filed 8151956.  Making 
>> those member functions
>> non-virtual may solve this particular problem, but as the code 
>> evolves we may hit it
>> again if we ever call a virtual member function by accident.
>>
>> I'm not a C++ expert, but if we declared those functions as virtual 
>> in CodeBlob, then would
>> that work?  It doesn't seem ideal, however.  I would rather not call 
>> out from the CodeBlob
>> constructor at all, but instead do the work in the subclass 
>> constructor.  Let's say we move
>> the call to  cb->copy_code_and_locs_to() to a separate function. Is 
>> there a C++ idiom
>> for making sure all subclasses of CodeBlob call it?  The only think I 
>> can think of is to set
>> an "initialized" flag and to check it in strategic places.
>>
>> dl
>>
>> On 4/8/2016 11:12 AM, Volker Simonis wrote:
>>> Hi Rickard,
>>>
>>> I found the problem why your change crashes the VM on ppc (and I'm 
>>> pretty
>>> sure it will also crash on ARM - @Andrew, maybe you can try it 
>>> out?). It is
>>> caused by the following code in address NativeCall::get_trampoline() 
>>> which
>>> is also present on arm64:
>>>
>>> address NativeCall::get_trampoline() {
>>>    address call_addr = addr_at(0);
>>>    CodeBlob *code = CodeCache::find_blob(call_addr);
>>> ...
>>>    // If the codeBlob is not a nmethod, this is because we get here 
>>> from the
>>>    // CodeBlob constructor, which is called within the nmethod 
>>> constructor.
>>>    return trampoline_stub_Relocation::get_trampoline_for(call_addr,
>>> (nmethod*)code);
>>> }
>>>
>>> The comment explains the situation quite well: we're in the CodeBlob
>>> constructor which was called by the CompiledMethod constructor which 
>>> was
>>> called from the nmethod constructor:
>>>
>>> #3  0x00003fffb741b80c in NativeCall::get_trampoline 
>>> (this=0x3fff607d0fac)
>>> at
>>> /usr/work/d046063/OpenJDK/jdk9-hs-comp/hotspot/src/cpu/ppc/vm/nativeInst_ppc.cpp:146 
>>>
>>> #4  0x00003fffb7596914 in Relocation::pd_call_destination
>>> (this=0x3ffdfe3fcc90, orig_addr=0x3fff603b8a2c "\001") at
>>> /usr/work/d046063/OpenJDK/jdk9-hs-comp/hotspot/src/cpu/ppc/vm/relocInfo_ppc.cpp:87 
>>>
>>> #5  0x00003fffb758f5fc in CallRelocation::fix_relocation_after_move
>>> (this=0x3ffdfe3fcc90, src=0x3ffdfe3fdb40, dest=0x3ffdfe3fcd58) at
>>> /usr/work/d046063/OpenJDK/jdk9-hs-comp/hotspot/src/share/vm/code/relocInfo.cpp:506 
>>>
>>> #6  0x00003fffb6c48898 in CodeBuffer::relocate_code_to
>>> (this=0x3ffdfe3fdb40, dest=0x3ffdfe3fcd58) at
>>> /usr/work/d046063/OpenJDK/jdk9-hs-comp/hotspot/src/share/vm/asm/codeBuffer.cpp:812 
>>>
>>> #7  0x00003fffb6c48404 in CodeBuffer::copy_code_to 
>>> (this=0x3ffdfe3fdb40,
>>> dest_blob=0x3fff607d0c10) at
>>> /usr/work/d046063/OpenJDK/jdk9-hs-comp/hotspot/src/share/vm/asm/codeBuffer.cpp:748 
>>>
>>> #8  0x00003fffb6c42670 in CodeBuffer::copy_code_and_locs_to
>>> (this=0x3ffdfe3fdb40, blob=0x3fff607d0c10) at
>>> /usr/work/d046063/OpenJDK/jdk9-hs-comp/hotspot/src/share/vm/asm/codeBuffer.hpp:607 
>>>
>>> #9  0x00003fffb6c3f834 in CodeBlob::CodeBlob (this=0x3fff607d0c10,
>>> name=0x3fffb7a75fd8 "nmethod", layout=..., cb=0x3ffdfe3fdb40,
>>> frame_complete_offset=20, frame_size=14, oop_maps=0x3ffe00049620,
>>> caller_must_gc_arguments=false, subtype=8) at
>>> /usr/work/d046063/OpenJDK/jdk9-hs-comp/hotspot/src/share/vm/code/codeBlob.cpp:117 
>>>
>>> #10 0x00003fffb6ce52c8 in CompiledMethod::CompiledMethod
>>> (this=0x3fff607d0c10, method=0x3ffe1ddce568, name=0x3fffb7a75fd8 
>>> "nmethod",
>>> size=1768, header_size=392, cb=0x3ffdfe3fdb40, 
>>> frame_complete_offset=20,
>>> frame_size=14, oop_maps=0x3ffe00049620, 
>>> caller_must_gc_arguments=false) at
>>> /usr/work/d046063/OpenJDK/jdk9-hs-comp/hotspot/src/share/vm/code/compiledMethod.cpp:42 
>>>
>>> #11 0x00003fffb7421f58 in nmethod::nmethod (this=0x3fff607d0c10,
>>> method=0x3ffe1ddce568, nmethod_size=1768, compile_id=4, entry_bci=-1,
>>> offsets=0x3ffdfe3fdb18, orig_pc_offset=104, debug_info=0x3fffb03d55f0,
>>> dependencies=0x3ffe00049690, code_buffer=0x3ffdfe3fdb40, frame_size=14,
>>> oop_maps=0x3ffe00049620, handler_table=0x3ffdfe3fdad0,
>>> nul_chk_table=0x3ffdfe3fdaf0, compiler=0x3fffb03bc270, comp_level=3) at
>>> /usr/work/d046063/OpenJDK/jdk9-hs-comp/hotspot/src/share/vm/code/nmethod.cpp:706 
>>>
>>>
>>> Now we cast 'code' to 'nmethod' but at this point in time 'code' is 
>>> still a
>>> CodeBlob from the C++ point of view (i.e. it still has a CodeBlob 
>>> vtable
>>> (see [1] for an explanation)).
>>>
>>> Later on, in RelocIterator::initialize() we call virtual methods on the
>>> nmethod which still has the vtable of a "CodeBlob" and this fails 
>>> badly:
>>>
>>> #0  SingletonBlob::print_on (this=0x3fff607d0c10, st=0x0) at
>>> /usr/work/d046063/OpenJDK/jdk9-hs-comp/hotspot/src/share/vm/code/codeBlob.cpp:584 
>>>
>>> #1  0x00003fffb758d51c in RelocIterator::initialize 
>>> (this=0x3ffdfe3fc928,
>>> nm=0x3fff607d0c10, begin=0x3fff607d0fac "\001", limit=0x0) at
>>> /usr/work/d046063/OpenJDK/jdk9-hs-comp/hotspot/src/share/vm/code/relocInfo.cpp:144 
>>>
>>> #2  0x00003fffb6ace56c in RelocIterator::RelocIterator
>>> (this=0x3ffdfe3fc928, nm=0x3fff607d0c10, begin=0x3fff607d0fac "\001",
>>> limit=0x0) at
>>> /usr/work/d046063/OpenJDK/jdk9-hs-comp/hotspot/src/share/vm/code/relocInfo.hpp:1378 
>>>
>>> #3  0x00003fffb75919dc in 
>>> trampoline_stub_Relocation::get_trampoline_for
>>> (call=0x3fff607d0fac "\001", code=0x3fff607d0c10) at
>>> /usr/work/d046063/OpenJDK/jdk9-hs-comp/hotspot/src/share/vm/code/relocInfo.cpp:849 
>>>
>>> #4  0x00003fffb741b80c in NativeCall::get_trampoline 
>>> (this=0x3fff607d0fac)
>>> at
>>> /usr/work/d046063/OpenJDK/jdk9-hs-comp/hotspot/src/cpu/ppc/vm/nativeInst_ppc.cpp:146 
>>>
>>>
>>> As you can see, we actually want to call nmethod::stub_begin() at
>>> relocInfo.cpp:144
>>>
>>> 142  _section_start[CodeBuffer::SECT_CONSTS] = nm->consts_begin();
>>> 143  _section_start[CodeBuffer::SECT_INSTS ] = nm->insts_begin() ;
>>> 144  _section_start[CodeBuffer::SECT_STUBS ] = nm->stub_begin()  ;
>>>
>>> but we actually end up in SingletonBlob::print_on() which is a 
>>> completely
>>> different method. Notice that the call to nm->consts_begin() before 
>>> also
>>> fails, but it doesn't crash the VM because it happens to call
>>> SingletonBlob::verify() which has no bad side effect. The call to
>>> nm->insts_begin() in line 143 is non-virtual and thus works fine. 
>>> Here are
>>> the corresponding vtable slots in the CodeBlob vtable for 
>>> consts_begin()
>>> and stub_begin()
>>>
>>> (gdb) p &nmethod::consts_begin
>>> $76 = &virtual table offset 42
>>> (gdb) p &nmethod::stub_begin
>>> $77 = &virtual table offset 44
>>> (gdb) p  ((*(void ***)nm) + 1)[42]
>>> $86 = (void *) 0x3fffb6c41df8 <SingletonBlob::verify()>
>>> (gdb) p  ((*(void ***)nm) + 1)[44]
>>> $87 = (void *) 0x3fffb6c41e64 
>>> <SingletonBlob::print_on(outputStream*) const>
>>>
>>> As you can see, 'nm' is indeed a "CodeBlob" at this point:
>>>
>>> (gdb) p *(void ***)nm
>>> $91 = (void **) 0x3fffb7befa00 <vtable for CodeBlob+16>
>>> (gdb) p nm->print()
>>> [CodeBlob (0x00003fff607d1090)]
>>> Framesize: 14
>>>
>>> The offending calls succeeded before your change, because they where 
>>> not
>>> virtual. Any idea how we can fix this with the new class hierarchy?
>>>
>>> Regards,
>>> Volker
>>>
>>> [1]
>>> http://stackoverflow.com/questions/6591859/when-does-the-vptr-pointing-to-vtable-get-initialized-for-a-polymorphic-class 
>>>
>>>
>>>
>>>
>>> On Thu, Apr 7, 2016 at 5:50 PM, Volker Simonis 
>>> <volker.simonis at gmail.com>
>>> wrote:
>>>
>>>> Hi Rickard,
>>>>
>>>> I'd also like to know what's the rational behind this quite large
>>>> change. Do you expect some performance or memory consumption
>>>> improvements or is this a prerequisite for another change which is
>>>> still to come?
>>>>
>>>> The change itself currently doesn't work on ppc64 (neither on Linux
>>>> nor on AIX). I get the following crash during the build when the newly
>>>> built Hotspot is JIT-compiling java.lang.String::charAt on C1 :
>>>>
>>>> #
>>>> # A fatal error has been detected by the Java Runtime Environment:
>>>> #
>>>> #  SIGSEGV (0xb) at pc=0x00001000012a44d0, pid=35331, tid=35404
>>>> #
>>>> # JRE version: OpenJDK Runtime Environment (9.0) (slowdebug build
>>>> 9-internal+0-2016-04-07-162501.d046063.jdk9-hs-comp)
>>>> # Java VM: OpenJDK 64-Bit Server VM (slowdebug
>>>> 9-internal+0-2016-04-07-162501.d046063.jdk9-hs-comp, mixed mode,
>>>> tiered, compressed oo
>>>> ps, serial gc, linux-ppc64le)
>>>> # Problematic frame:
>>>> # V  [libjvm.so+0xf744d0] outputStream::do_vsnprintf_and_write(char
>>>> const*, char*, bool)+0x40
>>>> #
>>>> # No core dump will be written. Core dumps have been disabled. To
>>>> enable core dumping, try "ulimit -c unlimited" before starting Java
>>>>   again
>>>> #
>>>> # If you would like to submit a bug report, please visit:
>>>> #   http://bugreport.java.com/bugreport/crash.jsp
>>>> #
>>>>
>>>> ---------------  S U M M A R Y ------------
>>>>
>>>> Command Line:
>>>> -Dapplication.home=/sapmnt/ld9510/a/d046063/output-jdk9-hs-comp-dbg/jdk 
>>>>
>>>> -Xms8m -XX:+UseSerialGC -Xms32M -Xmx512M -Djdk.
>>>> module.main=jdk.jlink jdk.jlink/jdk.tools.jmod.Main create
>>>> --module-version 9-internal --os-name Linux --os-arch ppc64le
>>>> --os-version
>>>>   2.6 --modulepath /priv/d046063/output-jdk9-hs-comp-dbg/images/jmods
>>>> --hash-dependencies .* --exclude **_the.* --libs
>>>>
>>>> /priv/d046063/output-jdk9-hs-comp-dbg/support/modules_libs-stripped/java.base 
>>>>
>>>> --cmds
>>>> /priv/d046063/output-jdk9-hs-comp-dbg/support/modules_cmds-stripped/java.base 
>>>>
>>>> --config
>>>> /priv/d046063/output-jdk9-hs-comp-dbg/support/modules_conf/java.base
>>>> --class-path 
>>>> /priv/d046063/output-jdk9-hs-comp-dbg/jdk/modules/java.base
>>>> /priv/d046063/output-jdk9-hs-comp-dbg/support/jmods/java.base.jmod
>>>>
>>>> Host: ld9510, POWER8E (raw), altivec supported, 48 cores, 61G, #
>>>> Please check /etc/os-release for details about this release.
>>>> Time: Thu Apr  7 16:28:55 2016 CEST elapsed time: 0 seconds (0d 0h 
>>>> 0m 0s)
>>>>
>>>> ---------------  T H R E A D  ---------------
>>>>
>>>> Current thread (0x000010000429c800):  JavaThread "C1 CompilerThread10"
>>>> daemon [_thread_in_vm, id=35404,
>>>> stack(0x000010006a800000,0x000010006ac00000)]
>>>>
>>>>
>>>> Current CompileTask:
>>>> C1:    761    3       3       java.lang.String::charAt (25 bytes)
>>>>
>>>> Stack: [0x000010006a800000,0x000010006ac00000],
>>>> sp=0x000010006abfc6c0,  free space=4081k
>>>> Native frames: (J=compiled Java code, j=interpreted, Vv=VM code, 
>>>> C=native
>>>> code)
>>>> V  [libjvm.so+0xf744d0] outputStream::do_vsnprintf_and_write(char
>>>> const*, char*, bool)+0x40
>>>> V  [libjvm.so+0xf74668]  outputStream::print_cr(char const*, ...)+0x68
>>>> V  [libjvm.so+0x72189c]  CodeBlob::print_on(outputStream*) const+0x50
>>>> V  [libjvm.so+0x723bdc] RuntimeBlob::print_on(outputStream*) 
>>>> const+0x40
>>>> V  [libjvm.so+0x721eb0] SingletonBlob::print_on(outputStream*) 
>>>> const+0x4c
>>>> V  [libjvm.so+0x106d51c] RelocIterator::initialize(CompiledMethod*,
>>>> unsigned char*, unsigned char*)+0x170
>>>> V  [libjvm.so+0x5ae56c] RelocIterator::RelocIterator(CompiledMethod*,
>>>> unsigned char*, unsigned char*)+0x78
>>>> V  [libjvm.so+0x10719dc]
>>>> trampoline_stub_Relocation::get_trampoline_for(unsigned char*,
>>>> nmethod*)+0x78
>>>> V  [libjvm.so+0xefb80c]  NativeCall::get_trampoline()+0x110
>>>> V  [libjvm.so+0x1076914] Relocation::pd_call_destination(unsigned
>>>> char*)+0x150
>>>> V  [libjvm.so+0x106f5fc]
>>>> CallRelocation::fix_relocation_after_move(CodeBuffer const*,
>>>> CodeBuffer*)+0x74
>>>> V  [libjvm.so+0x728898] CodeBuffer::relocate_code_to(CodeBuffer*)
>>>> const+0x390
>>>> V  [libjvm.so+0x728404] CodeBuffer::copy_code_to(CodeBlob*)+0x134
>>>> V  [libjvm.so+0x722670] 
>>>> CodeBuffer::copy_code_and_locs_to(CodeBlob*)+0x84
>>>> V  [libjvm.so+0x71f834]  CodeBlob::CodeBlob(char const*,
>>>> CodeBlobLayout const&, CodeBuffer*, int, int, OopMapSet*, bool,
>>>> int)+0x320
>>>> V  [libjvm.so+0x7c52c8] CompiledMethod::CompiledMethod(Method*, char
>>>> const*, int, int, CodeBuffer*, int, int, OopMapSet*, bool)+0xd8
>>>> V  [libjvm.so+0xf01f58]  nmethod::nmethod(Method*, int, int, int,
>>>> CodeOffsets*, int, DebugInformationRecorder*, Dependencies*,
>>>> CodeBuffer*, int, OopMapSet*, ExceptionHandlerTable*,
>>>> ImplicitExceptionTable*, AbstractCompiler*, int)+0xe0
>>>> V  [libjvm.so+0xf01610]  nmethod::new_nmethod(methodHandle const&,
>>>> int, int, CodeOffsets*, int, DebugInformationRecorder*, Dependencies*,
>>>> CodeBuffer*, int, OopMapSet*, ExceptionHandlerTable*,
>>>> ImplicitExceptionTable*, AbstractCompiler*, int)+0x2c4
>>>> V  [libjvm.so+0x632970]  ciEnv::register_method(ciMethod*, int,
>>>> CodeOffsets*, int, CodeBuffer*, int, OopMapSet*,
>>>> ExceptionHandlerTable*, ImplicitExceptionTable*, AbstractCompiler*,
>>>> bool, bool, RTMState)+0x560
>>>> V  [libjvm.so+0x48ee00] Compilation::install_code(int)+0x264
>>>> V  [libjvm.so+0x48eff8]  Compilation::compile_method()+0x184
>>>> V  [libjvm.so+0x48f7a8] Compilation::Compilation(AbstractCompiler*,
>>>> ciEnv*, ciMethod*, int, BufferBlob*, DirectiveSet*)+0x288
>>>> V  [libjvm.so+0x4980d0]  Compiler::compile_method(ciEnv*, ciMethod*,
>>>> int, DirectiveSet*)+0xc8
>>>> V  [libjvm.so+0x7b188c]
>>>> CompileBroker::invoke_compiler_on_method(CompileTask*)+0x590
>>>> V  [libjvm.so+0x7b07bc] CompileBroker::compiler_thread_loop()+0x310
>>>> V  [libjvm.so+0x11a614c]  compiler_thread_entry(JavaThread*, 
>>>> Thread*)+0xa0
>>>> V  [libjvm.so+0x119f3a8] JavaThread::thread_main_inner()+0x1b4
>>>> V  [libjvm.so+0x119f1a4]  JavaThread::run()+0x1b8
>>>> V  [libjvm.so+0xf53d90]  java_start(Thread*)+0x204
>>>> C  [libpthread.so.0+0x8a64]  start_thread+0xf4
>>>> C  [libc.so.6+0x1032a0]  clone+0x98
>>>>
>>>> I haven't identified the exact cause (will analyze it tomorrow) but
>>>> the stack trace indicates that it is indeed related to your changes.
>>>>
>>>> Besides that I have some comments:
>>>>
>>>> codeBuffer.hpp:
>>>>
>>>> 472   CodeSection* insts() { return &_insts; }
>>>> 475   const CodeSection* insts() const { return &_insts; }
>>>>
>>>> - do we really need both versions?
>>>>
>>>> codeBlob.hpp:
>>>>
>>>>   135   nmethod* as_nmethod_or_null() const                { return
>>>> is_nmethod() ? (nmethod*) this : NULL; }
>>>>   136   nmethod* as_nmethod() const                        {
>>>> assert(is_nmethod(), "must be nmethod"); return (nmethod*) this; }
>>>>   137   CompiledMethod* as_compiled_method_or_null() const { return
>>>> is_compiled() ? (CompiledMethod*) this : NULL; }
>>>>   138   CompiledMethod* as_compiled_method() const         {
>>>> assert(is_compiled(), "must be compiled"); return (CompiledMethod*)
>>>> this; }
>>>>   139   CodeBlob* as_codeblob_or_null() const              { return
>>>> (CodeBlob*) this; }
>>>>
>>>> - I don't like this code. You make the getters 'const' which
>>>> implicitely makes 'this' a "pointer to const" but then the returned
>>>> pointer is a normal pointer to a non-const object and therefore you
>>>> have to statically cast away the "pointer to const" (that's why you
>>>> need the cast even in the case where you return a CodeBlob*). So
>>>> either remove the const qualifier from the method declarations or make
>>>> them return "pointers to const". And by the way, as_codeblob_or_null()
>>>> doesn't seemed to be used anywhere in the code, why do we need it at
>>>> all?
>>>>
>>>> - Why do we need the non-virtual methods is_nmethod() and
>>>> is_compiled() to manually simulate virtual behavior. Why can't we
>>>> simply make them virtual and implement them accordingly in nmathod and
>>>> CompiledMethod?
>>>>
>>>> Regards,
>>>> Volker
>>>>
>>>> On Thu, Apr 7, 2016 at 2:12 PM, Rickard Bäckman
>>>> <rickard.backman at oracle.com> wrote:
>>>>> Hi,
>>>>>
>>>>> can I please have review for this patch please?
>>>>>
>>>>> So far CodeBlobs have required all the data (metadata, oops, code, 
>>>>> etc)
>>>>> to be in one continuous blob With this patch we are looking to change
>>>>> that. It's been done by changing offsets in CodeBlob to addresses,
>>>>> making some methods virtual to allow different behavior and also
>>>>> creating a couple of new classes. CompiledMethod now sits inbetween
>>>>> CodeBlob and nmethod.
>>>>>
>>>>> CR: https://bugs.openjdk.java.net/browse/JDK-8152664
>>>>> Webrev: http://cr.openjdk.java.net/~rbackman/8152664/
>>>>>
>>>>> Thanks
>>>>> /R
>>
>

More information about the hotspot-dev mailing list