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
--- 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++;