OpenJDK / jdk / jdk
changeset 43437:90e15b78684e
8168926: C2: Bytecode escape analyzer crashes due to stack overflow
Summary: Whether current call site needs an appendix is determined only based on the target method and the current bytecode instruction.
Reviewed-by: kvn, thartmann
author | zmajo |
---|---|
date | Wed, 11 Jan 2017 09:40:42 +0100 |
parents | 816e2f29711b |
children | af4378756105 |
files | hotspot/src/share/vm/ci/bcEscapeAnalyzer.cpp hotspot/src/share/vm/ci/ciMethod.hpp |
diffstat | 2 files changed, 34 insertions(+), 6 deletions(-) [+] |
line wrap: on
line diff
--- a/hotspot/src/share/vm/ci/bcEscapeAnalyzer.cpp Tue Jan 10 18:48:08 2017 +0000 +++ b/hotspot/src/share/vm/ci/bcEscapeAnalyzer.cpp Wed Jan 11 09:40:42 2017 +0100 @@ -895,8 +895,32 @@ ciMethod* target = s.get_method(ignored_will_link, &declared_signature); ciKlass* holder = s.get_declared_method_holder(); assert(declared_signature != NULL, "cannot be null"); - // Push appendix argument, if one. - if (s.has_appendix()) { + // If the current bytecode has an attached appendix argument, + // push an unknown object to represent that argument. (Analysis + // of dynamic call sites, especially invokehandle calls, needs + // the appendix argument on the stack, in addition to "regular" arguments + // pushed onto the stack by bytecode instructions preceding the call.) + // + // The escape analyzer does _not_ use the ciBytecodeStream::has_appendix(s) + // method to determine whether the current bytecode has an appendix argument. + // The has_appendix() method obtains the appendix from the + // ConstantPoolCacheEntry::_f1 field, which can happen concurrently with + // resolution of dynamic call sites. Callees in the + // ciBytecodeStream::get_method() call above also access the _f1 field; + // interleaving the get_method() and has_appendix() calls in the current + // method with call site resolution can lead to an inconsistent view of + // the current method's argument count. In particular, some interleaving(s) + // can cause the method's argument count to not include the appendix, which + // then leads to stack over-/underflow in the escape analyzer. + // + // Instead of pushing the argument if has_appendix() is true, the escape analyzer + // pushes an appendix for all call sites targeted by invokedynamic and invokehandle + // instructions, except if the call site is the _invokeBasic intrinsic + // (that intrinsic is always targeted by an invokehandle instruction but does + // not have an appendix argument). + if (target->is_loaded() && + Bytecodes::has_optional_appendix(s.cur_bc_raw()) && + target->intrinsic_id() != vmIntrinsics::_invokeBasic) { state.apush(unknown_obj); } // Pass in raw bytecode because we need to see invokehandle instructions.
--- a/hotspot/src/share/vm/ci/ciMethod.hpp Tue Jan 10 18:48:08 2017 +0000 +++ b/hotspot/src/share/vm/ci/ciMethod.hpp Wed Jan 11 09:40:42 2017 +0100 @@ -136,15 +136,19 @@ check_is_loaded(); return _signature->size() + (_flags.is_static() ? 0 : 1); } - // Report the number of elements on stack when invoking this method. - // This is different than the regular arg_size because invokedynamic - // has an implicit receiver. + // Report the number of elements on stack when invoking the current method. + // If the method is loaded, arg_size() gives precise information about the + // number of stack elements (using the method's signature and its flags). + // However, if the method is not loaded, the number of stack elements must + // be determined differently, as the method's flags are not yet available. + // The invoke_arg_size() method assumes in that case that all bytecodes except + // invokestatic and invokedynamic have a receiver that is also pushed onto the + // stack by the caller of the current method. int invoke_arg_size(Bytecodes::Code code) const { if (is_loaded()) { return arg_size(); } else { int arg_size = _signature->size(); - // Add a receiver argument, maybe: if (code != Bytecodes::_invokestatic && code != Bytecodes::_invokedynamic) { arg_size++;