8148871: Possible wrong expression stack depth at deopt point
Summary: Add a special case for invoke bytecodes and use the parameter size instead of the oop map in that case.
Reviewed-by: never, dlong, minqi
--- a/src/hotspot/share/interpreter/bytecode.cpp Wed Feb 28 22:38:53 2018 +0100
+++ b/src/hotspot/share/interpreter/bytecode.cpp Thu Mar 01 08:58:27 2018 +0100
@@ -123,6 +123,11 @@
assert(cpcache() != NULL, "do not call this from verifier or rewriter");
}
+int Bytecode_invoke::size_of_parameters() const {
+ ArgumentSizeComputer asc(signature());
+ return asc.size() + (has_receiver() ? 1 : 0);
+}
+
Symbol* Bytecode_member_ref::klass() const {
return constants()->klass_ref_at_noresolve(index());
--- a/src/hotspot/share/interpreter/bytecode.hpp Wed Feb 28 22:38:53 2018 +0100
+++ b/src/hotspot/share/interpreter/bytecode.hpp Thu Mar 01 08:58:27 2018 +0100
@@ -197,7 +197,7 @@
BasicType result_type() const; // returns the result type of the getfield or invoke
};
-// Abstraction for invoke_{virtual, static, interface, special}
+// Abstraction for invoke_{virtual, static, interface, special, dynamic, handle}
class Bytecode_invoke: public Bytecode_member_ref {
protected:
@@ -231,6 +231,8 @@
bool has_appendix() { return cpcache_entry()->has_appendix(); }
+ int size_of_parameters() const;
+
private:
// Helper to skip verification. Used is_valid() to check if the result is really an invoke
inline friend Bytecode_invoke Bytecode_invoke_check(const methodHandle& method, int bci);
--- a/src/hotspot/share/runtime/deoptimization.cpp Wed Feb 28 22:38:53 2018 +0100
+++ b/src/hotspot/share/runtime/deoptimization.cpp Thu Mar 01 08:58:27 2018 +0100
@@ -604,7 +604,7 @@
// Return BasicType of value being returned
JRT_LEAF(BasicType, Deoptimization::unpack_frames(JavaThread* thread, int exec_mode))
- // We are already active int he special DeoptResourceMark any ResourceObj's we
+ // We are already active in the special DeoptResourceMark any ResourceObj's we
// allocate will be freed at the end of the routine.
// It is actually ok to allocate handles in a leaf method. It causes no safepoints,
@@ -681,55 +681,41 @@
// at an uncommon trap for an invoke (where the compiler
// generates debug info before the invoke has executed)
Bytecodes::Code cur_code = str.next();
- if (cur_code == Bytecodes::_invokevirtual ||
- cur_code == Bytecodes::_invokespecial ||
- cur_code == Bytecodes::_invokestatic ||
- cur_code == Bytecodes::_invokeinterface ||
- cur_code == Bytecodes::_invokedynamic) {
+ if (Bytecodes::is_invoke(cur_code)) {
Bytecode_invoke invoke(mh, iframe->interpreter_frame_bci());
- Symbol* signature = invoke.signature();
- ArgumentSizeComputer asc(signature);
- cur_invoke_parameter_size = asc.size();
- if (invoke.has_receiver()) {
- // Add in receiver
- ++cur_invoke_parameter_size;
- }
+ cur_invoke_parameter_size = invoke.size_of_parameters();
if (i != 0 && !invoke.is_invokedynamic() && MethodHandles::has_member_arg(invoke.klass(), invoke.name())) {
callee_size_of_parameters++;
}
}
if (str.bci() < max_bci) {
- Bytecodes::Code bc = str.next();
- if (bc >= 0) {
+ Bytecodes::Code next_code = str.next();
+ if (next_code >= 0) {
// The interpreter oop map generator reports results before
// the current bytecode has executed except in the case of
// calls. It seems to be hard to tell whether the compiler
// has emitted debug information matching the "state before"
// a given bytecode or the state after, so we try both
- switch (cur_code) {
- case Bytecodes::_invokevirtual:
- case Bytecodes::_invokespecial:
- case Bytecodes::_invokestatic:
- case Bytecodes::_invokeinterface:
- case Bytecodes::_invokedynamic:
- case Bytecodes::_athrow:
- break;
- default: {
+ if (!Bytecodes::is_invoke(cur_code) && cur_code != Bytecodes::_athrow) {
+ // Get expression stack size for the next bytecode
+ if (Bytecodes::is_invoke(next_code)) {
+ Bytecode_invoke invoke(mh, str.bci());
+ next_mask_expression_stack_size = invoke.size_of_parameters();
+ } else {
InterpreterOopMap next_mask;
OopMapCache::compute_one_oop_map(mh, str.bci(), &next_mask);
next_mask_expression_stack_size = next_mask.expression_stack_size();
- // Need to subtract off the size of the result type of
- // the bytecode because this is not described in the
- // debug info but returned to the interpreter in the TOS
- // caching register
- BasicType bytecode_result_type = Bytecodes::result_type(cur_code);
- if (bytecode_result_type != T_ILLEGAL) {
- top_frame_expression_stack_adjustment = type2size[bytecode_result_type];
- }
- assert(top_frame_expression_stack_adjustment >= 0, "");
- try_next_mask = true;
- break;
}
+ // Need to subtract off the size of the result type of
+ // the bytecode because this is not described in the
+ // debug info but returned to the interpreter in the TOS
+ // caching register
+ BasicType bytecode_result_type = Bytecodes::result_type(cur_code);
+ if (bytecode_result_type != T_ILLEGAL) {
+ top_frame_expression_stack_adjustment = type2size[bytecode_result_type];
+ }
+ assert(top_frame_expression_stack_adjustment >= 0, "stack adjustment must be positive");
+ try_next_mask = true;
}
}
}
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/test/hotspot/jtreg/compiler/interpreter/TestVerifyStackAfterDeopt.java Thu Mar 01 08:58:27 2018 +0100
@@ -0,0 +1,58 @@
+/*
+ * Copyright (c) 2018, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+
+/*
+ * @test TestVerifyStackAfterDeopt
+ * @bug 8148871
+ * @summary Checks VerifyStack after deoptimization of array allocation slow call
+ * @run main/othervm -XX:+IgnoreUnrecognizedVMOptions -XX:TieredStopAtLevel=1
+ * -XX:+DeoptimizeALot -XX:+VerifyStack
+ * compiler.interpreter.TestVerifyStackAfterDeopt
+ */
+
+package compiler.interpreter;
+
+public class TestVerifyStackAfterDeopt {
+
+ private void method(Object[] a) {
+
+ }
+
+ private void test() {
+ // For the array allocation, C1 emits a slow call into the runtime
+ // that deoptimizes the caller frame due to -XX:+DeoptimizeALot.
+ // The VerifyStack code then gets confused because the following
+ // bytecode instruction is an invoke and the interpreter oop map
+ // generator reports the oop map after execution of that invoke.
+ method(new Object[0]);
+ }
+
+ public static void main(String[] args) {
+ TestVerifyStackAfterDeopt t = new TestVerifyStackAfterDeopt();
+ // Run long enough for C1 compilation to trigger and TLAB to fill up
+ for (int i = 0; i < 100_000; ++i) {
+ t.test();
+ }
+ }
+}