Crash with "assert(number_of_codes <= 256) failed: too many bytecodes"

Tobias Hartmann tobias.hartmann at oracle.com
Fri Jul 28 13:21:19 UTC 2017


On 28.07.2017 11:59, Maurizio Cimadamore wrote:
> If you are in search for more inspiration, please take a look at this earlier experiment I did:
> 
> http://cr.openjdk.java.net/~mcimadamore/vgetfield-remove-hotspot/
> 
> This completely removed vgetfield from the picture, so it had workarounds for most of the stuff you found.

Yes but it's not correct:

+  bool is_valuetype = cp->tag_at(idx).is_value_type()
+
+  assert(!(!is_get && is_valuetype), "Invalid configuration: vputfield doesn't exist");

Misses a ";" after the first statement and otherwise breaks with:

#  Internal Error (/oracle/valhalla_cp/hotspot/src/share/vm/oops/array.hpp:118), pid=22366, tid=22383
#  assert(i >= 0 && i< _length) failed: oob: 0 <= 65583 < 562

V  [libjvm.so+0xb344ad]  report_vm_error(char const*, int, char const*, char const*, ...)+0xdd
V  [libjvm.so+0xdc9c44]  GenerateOopMap::do_field(int, int, int, int)+0x94
V  [libjvm.so+0xdcd87c]  GenerateOopMap::interp_bb(BasicBlock*)+0x10c
V  [libjvm.so+0xdce81c]  GenerateOopMap::do_interpretation()+0xdc
[...]

Also, all code that iterates over the java bytecodes, for example GraphKit::compute_stack_effects(), can never find a _vgetfield after your changes. These cases should be removed.

Best regards,
Tobias

> On 28/07/17 09:57, Tobias Hartmann wrote:
>> Hi,
>>
>> I found more problems:
>> (1) C2 emits non value type specific code for _vgetfield because it looks at the Java bytecode (not the raw bytecode) which is now _getfield.
>> -> Fixed by checking the field holder in parse3.cpp
>>
>> (2) Oop map generation is broken because there is no way to determine if the _getfield bytecode accesses a value type field.
>> -> I had to temporarily disable verification. See changes in generateOopMap.cpp/hpp. Checking itr->raw_code() == Bytecodes::_vgetfield is not sufficient and also a constant pool lookup does not work in all cases.
>> Any ideas on how to fix this?
>>
>> Here are the updated webrevs:
>> http://cr.openjdk.java.net/~thartmann/valhalla/vt_prototype/webrev.36.jdk/
>> http://cr.openjdk.java.net/~thartmann/valhalla/vt_prototype/webrev.36.langtools/
>> http://cr.openjdk.java.net/~thartmann/valhalla/vt_prototype/webrev.36.hs/
>>
>> I'll run them through JPRT.
>>
>> Best regards,
>> Tobias
>>
>> On 27.07.2017 20:09, Tobias Hartmann wrote:
>>> I updated the webrevs in place and will re-run JPRT.
>>>
>>> I'm also seeing lots of these:
>>>
>>> Error: A JNI error has occurred, please check your installation and try again
>>> Exception in thread "main" java.lang.VerifyError: Illegal type at constant pool entry 57 in class valhalla.shady.MinimalValueTypes_1_0
>>> Exception Details:
>>>    Location:
>>>      valhalla/shady/MinimalValueTypes_1_0.getValueClass()Ljava/lang/Class; @0: ldc
>>>    Reason:
>>>      Constant pool index 57 is invalid
>>>    Bytecode:
>>>      0000000: 1239 c000 3ab0
>>>
>>>     at java.base/java.lang.invoke.MethodTypeForm.canonicalize(MethodTypeForm.java:368)
>>>     at java.base/java.lang.invoke.MethodTypeForm.canonicalizeAll(MethodTypeForm.java:424)
>>>     at java.base/java.lang.invoke.MethodTypeForm.canonicalize(MethodTypeForm.java:348)
>>>     at java.base/java.lang.invoke.MethodTypeForm.findForm(MethodTypeForm.java:320)
>>>     at java.base/java.lang.invoke.MethodType.makeImpl(MethodType.java:325)
>>>     at java.base/java.lang.invoke.MethodHandleNatives.findMethodHandleType(MethodHandleNatives.java:292)
>>>     at java.base/java.util.ResourceBundle.getLoader(ResourceBundle.java:561)
>>>     at java.base/java.util.ResourceBundle.getBundleImpl(ResourceBundle.java:1560)
>>>     at java.base/java.util.ResourceBundle.getBundleImpl(ResourceBundle.java:1535)
>>>     at java.base/java.util.ResourceBundle.getBundle(ResourceBundle.java:836)
>>>     at java.base/sun.launcher.LauncherHelper$ResourceBundleHolder.<clinit>(LauncherHelper.java:128)
>>>     at java.base/sun.launcher.LauncherHelper.getLocalizedMessage(LauncherHelper.java:352)
>>>     at java.base/sun.launcher.LauncherHelper.abort(LauncherHelper.java:519)
>>>     at java.base/sun.launcher.LauncherHelper.loadMainClass(LauncherHelper.java:677)
>>>     at java.base/sun.launcher.LauncherHelper.checkAndLoadMain(LauncherHelper.java:552)
>>>
>>> Best regards,
>>> Tobias
>>>
>>> On 27.07.2017 19:51, Tobias Hartmann wrote:
>>>> On 27.07.2017 19:50, Karen Kinnear wrote:
>>>>> We missed a change in byte codes.cpp:
>>>>>
>>>>> Need to move the def(_vgetfield down to the JVM byte codes and add the additional field which is _getfield as the java_code.
>>>> Okay, I'll fix that.
>>>>
>>>> Best regards,
>>>> Tobias
>>>>
>>>>> thanks,
>>>>> Karen
>>>>>
>>>>>> On Jul 27, 2017, at 1:34 PM, Tobias Hartmann <tobias.hartmann at oracle.com> wrote:
>>>>>>
>>>>>> Hi Karen,
>>>>>>
>>>>>> On 27.07.2017 19:30, Karen Kinnear wrote:
>>>>>>> Do you want to change in ByteCodes.java -
>>>>>>> ByteCodeCount to 210 instead of 216?
>>>>>> Right, that count should be updated as well.
>>>>>>
>>>>>> I just run this through JPRT and I get dozens of failures:
>>>>>>
>>>>>> #  Internal Error (/scratch/opt/jprt/T/P1/170707.tohartma/s/hotspot/src/share/vm/interpreter/bytecodeStream.hpp:210), pid=10773, tid=10777
>>>>>> #  assert(Bytecodes::is_java_code(code)) failed: sanity check
>>>>>>
>>>>>> Stack: [0x00007fc4238db000,0x00007fc4239db000],  sp=0x00007fc4239d72d0,  free space=1008k
>>>>>> Native frames: (J=compiled Java code, A=aot compiled Java code, j=interpreted, Vv=VM code, C=native code)
>>>>>> V  [libjvm.so+0x16aee0f]  VMError::report_and_die(int, char const*, char const*, __va_list_tag*, Thread*, unsigned char*, void*, void*, char const*, int, unsigned long)+0x15f
>>>>>> V  [libjvm.so+0x16afbbf]  VMError::report_and_die(Thread*, char const*, int, char const*, char const*, __va_list_tag*)+0x2f
>>>>>> V  [libjvm.so+0xad92fd]  report_vm_error(char const*, int, char const*, char const*, ...)+0xdd
>>>>>> V  [libjvm.so+0xb0e5d8]  BytecodeStream::next()+0x158
>>>>>> V  [libjvm.so+0xd65088]  RetTable::compute_ret_table(methodHandle const&)+0x48
>>>>>> V  [libjvm.so+0xd67c7d]  GenerateOopMap::compute_map(Thread*)+0x31d
>>>>>> V  [libjvm.so+0x1327c7a]  OopMapForCacheEntry::compute_map(Thread*)+0xba
>>>>>> V  [libjvm.so+0x1329aa6]  OopMapCacheEntry::fill(methodHandle, int)+0x136
>>>>>> V  [libjvm.so+0x132b2ab]  OopMapCache::lookup(methodHandle const&, int, InterpreterOopMap*) const+0x1bb
>>>>>> V  [libjvm.so+0x126587a]  Method::mask_for(int, InterpreterOopMap*)+0x5a
>>>>>> V  [libjvm.so+0xc1e97e]  frame::oops_interpreted_do(OopClosure*, RegisterMap const*, bool)+0xb0e
>>>>>> V  [libjvm.so+0x15dd2ba]  JavaThread::oops_do(OopClosure*, CodeBlobClosure*)+0x21a
>>>>>> V  [libjvm.so+0x15e799e]  Threads::possibly_parallel_oops_do(bool, OopClosure*, CodeBlobClosure*)+0x4e
>>>>>> V  [libjvm.so+0xd36be1]  GenCollectedHeap::process_roots(StrongRootsScope*, GenCollectedHeap::ScanningOption, OopClosure*, OopClosure*, CLDClosure*, CLDClosure*, CodeBlobToOopClosure*)+0x81
>>>>>> V  [libjvm.so+0xd36fdc]  GenCollectedHeap::young_process_roots(StrongRootsScope*, OopsInGenClosure*, OopsInGenClosure*, CLDClosure*)+0x4c
>>>>>> V  [libjvm.so+0xaf22e4]  DefNewGeneration::collect(bool, bool, unsigned long, bool)+0x554
>>>>>> V  [libjvm.so+0xd3a576]  GenCollectedHeap::collect_generation(Generation*, bool, unsigned long, bool, bool, bool, bool)+0x356
>>>>>> V  [libjvm.so+0xd3bb28]  GenCollectedHeap::do_collection(bool, bool, unsigned long, bool, GenCollectedHeap::GenerationType)+0x338
>>>>>> V  [libjvm.so+0xa02ebd]  GenCollectorPolicy::satisfy_failed_allocation(unsigned long, bool)+0xed
>>>>>> V  [libjvm.so+0x16b059d]  VM_GenCollectForAllocation::doit()+0xcd
>>>>>> V  [libjvm.so+0x16ebfc9]  VM_Operation::evaluate()+0xa9
>>>>>> V  [libjvm.so+0x16e8fbf]  VMThread::evaluate_operation(VM_Operation*)+0x34f
>>>>>> V  [libjvm.so+0x16e98a9]  VMThread::loop()+0x269
>>>>>> V  [libjvm.so+0x16e9f50]  VMThread::run()+0xc0
>>>>>> V  [libjvm.so+0x1347982]  thread_native_entry(Thread*)+0x112
>>>>>>
>>>>>> Maybe it's best to back out 8185349 for now.
>>>>>>
>>>>>> Best regards,
>>>>>> Tobias
>>>>>>
>>>>>>> Maurizio’s call - this is compiler sources,
>>>>>>>
>>>>>>> thanks,
>>>>>>> Karen
>>>>>>>
>>>>>>>> On Jul 27, 2017, at 12:18 PM, Tobias Hartmann <tobias.hartmann at oracle.com> wrote:
>>>>>>>>
>>>>>>>> Okay, it's getting complicated. Here are the webrevs:
>>>>>>>>
>>>>>>>> http://cr.openjdk.java.net/~thartmann/valhalla/vt_prototype/webrev.36.jdk/
>>>>>>>> http://cr.openjdk.java.net/~thartmann/valhalla/vt_prototype/webrev.36.langtools/
>>>>>>>> http://cr.openjdk.java.net/~thartmann/valhalla/vt_prototype/webrev.36.hs/
>>>>>>>>
>>>>>>>> What do you think?
>>>>>>>>
>>>>>>>> Best regards,
>>>>>>>> Tobias
>>>>>>>>
>>>>>>>> On 27.07.2017 18:08, Tobias Hartmann wrote:
>>>>>>>>> On 27.07.2017 17:58, Maurizio Cimadamore wrote:
>>>>>>>>>> Wait - if you update these opcodes, then we need to update bytecode library as well - see
>>>>>>>>>>
>>>>>>>>>> jdk.experimental.bytecode.Opcode
>>>>>>>>> Right:
>>>>>>>>>
>>>>>>>>> diff -r 2e4c9a8ae4e3 src/java.base/share/classes/jdk/experimental/bytecode/Opcode.java
>>>>>>>>> --- a/src/java.base/share/classes/jdk/experimental/bytecode/Opcode.java    Thu Jul 27 15:59:25 2017 +0100
>>>>>>>>> +++ b/src/java.base/share/classes/jdk/experimental/bytecode/Opcode.java    Thu Jul 27 18:07:52 2017 +0200
>>>>>>>>> @@ -235,14 +235,11 @@
>>>>>>>>>     VSTORE(204),
>>>>>>>>>     VALOAD(205),
>>>>>>>>>     VASTORE(206),
>>>>>>>>> -    VNEW(207),
>>>>>>>>> -    VRETURN(210),
>>>>>>>>> -    VGETFIELD(211),
>>>>>>>>> -    TYPED(212),
>>>>>>>>> -    VDEFAULT(214),
>>>>>>>>> -    VWITHFIELD(215),
>>>>>>>>> -    VBOX(216),
>>>>>>>>> -    VUNBOX(217);
>>>>>>>>> +    VRETURN(207),
>>>>>>>>> +    VDEFAULT(208),
>>>>>>>>> +    VWITHFIELD(209),
>>>>>>>>> +    VBOX(210),
>>>>>>>>> +    VUNBOX(211);
>>>>>>>>>
>>>>>>>>>     int code;
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> And I guess we have to change this as well:
>>>>>>>>>
>>>>>>>>> diff -r 298730c35452 src/jdk.jdeps/share/classes/com/sun/tools/classfile/Opcode.java
>>>>>>>>> --- a/src/jdk.jdeps/share/classes/com/sun/tools/classfile/Opcode.java    Thu Jul 27 16:00:29 2017 +0100
>>>>>>>>> +++ b/src/jdk.jdeps/share/classes/com/sun/tools/classfile/Opcode.java    Thu Jul 27 18:08:20 2017 +0200
>>>>>>>>> @@ -247,13 +247,13 @@
>>>>>>>>>     VSTORE(204, LOCAL),
>>>>>>>>>     VALOAD(205),
>>>>>>>>>     VASTORE(206),
>>>>>>>>> -    VRETURN(210),
>>>>>>>>> +    VRETURN(207),
>>>>>>>>>
>>>>>>>>> -    VDEFAULT(214, CPREF_W),
>>>>>>>>> -    VWITHFIELD(215, CPREF_W),
>>>>>>>>> +    VDEFAULT(208, CPREF_W),
>>>>>>>>> +    VWITHFIELD(209, CPREF_W),
>>>>>>>>>
>>>>>>>>> -    VBOX(216, CPREF_W),
>>>>>>>>> -    VUNBOX(217, CPREF_W),
>>>>>>>>> +    VBOX(210, CPREF_W),
>>>>>>>>> +    VUNBOX(211, CPREF_W),
>>>>>>>>>
>>>>>>>>>     // impdep 0xfe: PicoJava nonpriv
>>>>>>>>>     // impdep 0xff: Picojava priv
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>> Tobias
>>>>>>>>>
>>>>>>>>>> On 27/07/17 16:31, Tobias Hartmann wrote:
>>>>>>>>>>> Hi,
>>>>>>>>>>>
>>>>>>>>>>> seems like this is due to:
>>>>>>>>>>> http://hg.openjdk.java.net/valhalla/valhalla/hotspot/rev/dbb31857c4d4
>>>>>>>>>>>
>>>>>>>>>>> If I count correctly, adding _vgetfield at the end 'causes number_of_codes' to be 257 and thus we fail.
>>>>>>>>>>>
>>>>>>>>>>> We should reset the value type bytecode numbers:
>>>>>>>>>>>
>>>>>>>>>>> diff -r 5cd8d5559192 src/share/vm/interpreter/bytecodes.hpp
>>>>>>>>>>> --- a/src/share/vm/interpreter/bytecodes.hpp    Thu Jul 27 16:00:00 2017 +0100
>>>>>>>>>>> +++ b/src/share/vm/interpreter/bytecodes.hpp    Thu Jul 27 17:23:40 2017 +0200
>>>>>>>>>>> @@ -248,11 +248,11 @@
>>>>>>>>>>>      _vstore               = 204, // 0xcc
>>>>>>>>>>>      _vaload               = 205, // 0xcd
>>>>>>>>>>>      _vastore              = 206, // 0xce
>>>>>>>>>>> -    _vreturn              = 210, // 0xd2
>>>>>>>>>>> -    _vdefault             = 214, // 0xd6
>>>>>>>>>>> -    _vwithfield           = 215, // 0xd7
>>>>>>>>>>> -    _vbox                 = 216, // 0xd6
>>>>>>>>>>> -    _vunbox               = 217, // 0xd7
>>>>>>>>>>> +    _vreturn              = 207, // 0xcf
>>>>>>>>>>> +    _vdefault             = 208, // 0xd0
>>>>>>>>>>> +    _vwithfield           = 209, // 0xd1
>>>>>>>>>>> +    _vbox                 = 210, // 0xd2
>>>>>>>>>>> +    _vunbox               = 211, // 0xd3
>>>>>>>>>>>        number_of_java_codes,
>>>>>>>>>>>
>>>>>>>>>>> Thanks,
>>>>>>>>>>> Tobias
> 


More information about the valhalla-dev mailing list