JDK-6224591 : JVM stops merging state vectors for blocks where there's a monitor mismatch.
  • Type: Bug
  • Component: hotspot
  • Sub-Component: runtime
  • Affected Version: 1.3.1_13,1.4.2_05
  • Priority: P2
  • Status: Closed
  • Resolution: Fixed
  • OS: solaris_8
  • CPU: generic
  • Submitted: 2005-02-02
  • Updated: 2012-10-08
  • Resolved: 2005-03-17
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
Other Other JDK 6
1.3.1_16Fixed 1.4.2_09Fixed 6 b28Fixed
Description
JVM crash during GC: Oop wrongly followed as reference 

The JVM is crashing during GC, even in interpreted mode (-Xint).  For a certain application it is readily reproducible, and when using -XX:+VerifyBeforeGC in a debug VM (java_g) it can be seen crashing before the first GC (ie. heap is not corrupt).

In the customer case, the problem is seen with an obfuscated class, and has not been seen with non-obfuscated classes.


Stack is as follows (not using VerifyBeforeGC):

  ---- called from signal handler with signal 10 (SIGBUS) ------
  [12] libjvm_g.so:oopDesc::mark(0x3, 0x3, 0x6, 0x1d0508, 0x21, 0xfd6811b8), at 0xfdc8600c 
  [13] libjvm_g.so:oopDesc::is_forwarded(0x3, 0x3, 0xc2e01004, 0x1, 0x0, 0x0), at 0xfdc84e18 
  [14] libjvm_g.so:FastScanClosure::do_oop(0xfd681638, 0xc2e00ffc, 0xc50, 0xfd6811ac, 0xfffffff8, 0xf6b70), at 0xfdc874b8 
  [15] libjvm_g.so:InterpreterFrameClosure::offset_do(0xfd6811c4, 0x1a, 0xc50, 0xfd6811ac, 0x1, 0x0), at 0xfdcb9054 
  [16] libjvm_g.so:InterpreterOopMap::iterate_oop(0xfd6811ac, 0xfd6811c4, 0xfd6811ac, 0xa, 0xfd681638, 0x0), at 0xfdfe6ca8 
  [17] libjvm_g.so:frame::oops_interpreted_do(0xfd6812c4, 0xfd681638, 0xfd6812d8, 0x0, 0x0, 0x0), at 0xfdcb2d80 
  [18] libjvm_g.so:frame::oops_do(0xfd6812c4, 0xfd681638, 0xfd6812d8, 0x0, 0x0, 0x0), at 0xfdcb361c  
  [19] libjvm_g.so:JavaThread::oops_do(0x23c740, 0xfd681638, 0x0, 0x0, 0x0, 0x0), at 0xfe1680f8 
  [20] libjvm_g.so:Threads::oops_do(0xfd681638, 0x1, 0x0, 0x0, 0x0, 0x0), at 0xfe16c614 
  [21] libjvm_g.so:GenCollectedHeap::process_strong_roots(0xcb068, 0x0, 0x1, 0x0, 0x1, 0xfd681614), at 0xfdcd57c0 
  [22] libjvm_g.so:DefNewGeneration::collect(0xcb478, 0x0, 0x0, 0x2004, 0x0, 0x0), at 0xfdc80b00 
  [23] libjvm_g.so:GenCollectedHeap::do_collection(0xcb068, 0x0, 0x0, 0x2004, 0x0, 0x0), at 0xfdcd5054 
  [24] libjvm_g.so:TwoGenerationCollectorPolicy::satisfy_failed_allocation(0xcabe0, 0x2004, 0x0, 0x0, 0xc2b8062c, 0x0), at 0xfdbdc52c 
  [25] libjvm_g.so:GenCollectedHeap::satisfy_failed_allocation(0xcb068, 0x2004, 0x0, 0x0, 0xc2b8062c, 0x0), at 0xfdcd5604 
  [26] libjvm_g.so:VM_GenCollectForAllocation::doit(0xc2b80610, 0xf6668, 0x0, 0xff37e000, 0xfd681e14, 0xfd681e04), at 0xfe2070e8 
  [27] libjvm_g.so:VM_Operation::evaluate(0xc2b80610, 0x1, 0x2710, 0x0, 0x4, 0xfd681e04), at 0xfe206a28 
  [28] libjvm_g.so:VMThread::evaluate_operation(0xf6668, 0xc2b80610, 0xfe418639, 0x0, 0x4, 0xff37e000), at 0xfe2026bc 
  [29] libjvm_g.so:VMThread::loop(0xf6668, 0x40, 0x40, 0x0, 0x4, 0xff37e000), at 0xfe202de0 
  [30] libjvm_g.so:VMThread::run(0xf6668, 0x40, 0xff37e000, 0xfd681d08, 0x4, 0x0), at 0xfe2023b4 
  [31] libjvm_g.so:_start(0xf6668, 0xff37f690, 0x1, 0x1, 0xff37e000, 0x0), at 0xfdffc3a4 
  


###@###.### 2005-2-02 15:34:00 GMT

Comments
EVALUATION This appears to be bug which has been in the code since it was implemented. A fix is relatively straightforward and is in the suggested fix section. ###@###.### 2005-2-03 22:18:31 GMT
03-02-2005

WORK AROUND In the specific real-world case, not obfuscating the code has not triggered the problem (which is not to say that plain javac-generated code cannot have the problem, but it seems unlikely). Not, unfortunately, a realistic work around with which to live. ###@###.### 2005-2-02 15:34:01 GMT
02-02-2005

SUGGESTED FIX *** /home/never/public_html/webrev/6224591/./src/share/vm/oops/generateOopMap.hpp- Thu Feb 3 14:15:17 2005 --- generateOopMap.hpp Thu Feb 3 13:53:55 2005 *************** *** 295,305 **** // Cell type methods void init_state(); void make_context_uninitialized (); int methodsig_to_effect (symbolOop signature, bool isStatic, CellTypeState* effect); ! bool merge_state_vectors (CellTypeState* cts, CellTypeState* bbts); void copy_state (CellTypeState *dst, CellTypeState *src); void merge_state_into_bb (BasicBlock *bb); static void merge_state (GenerateOopMap *gom, int bcidelta, int* data); void set_var (int localNo, CellTypeState cts); CellTypeState get_var (int localNo); --- 295,306 ---- // Cell type methods void init_state(); void make_context_uninitialized (); int methodsig_to_effect (symbolOop signature, bool isStatic, CellTypeState* effect); ! bool merge_local_state_vectors (CellTypeState* cts, CellTypeState* bbts); ! bool merge_monitor_state_vectors(CellTypeState* cts, CellTypeState* bbts); void copy_state (CellTypeState *dst, CellTypeState *src); void merge_state_into_bb (BasicBlock *bb); static void merge_state (GenerateOopMap *gom, int bcidelta, int* data); void set_var (int localNo, CellTypeState cts); CellTypeState get_var (int localNo); *** /home/never/public_html/webrev/6224591/./src/share/vm/oops/generateOopMap.cpp- Thu Feb 3 14:15:18 2005 --- generateOopMap.cpp Thu Feb 3 14:00:18 2005 *************** *** 681,693 **** assert(result.is_valid_state(), "checking that CTS merge maintains legal state"); return result; } ! // Merge the variable state from cts into bb. ! bool GenerateOopMap::merge_state_vectors(CellTypeState* cts, ! CellTypeState* bbts) { int i; int len = _max_locals + _stack_top; bool change = false; for (i = len - 1; i >= 0; i--) { --- 681,693 ---- assert(result.is_valid_state(), "checking that CTS merge maintains legal state"); return result; } ! // Merge the variable state for locals and stack from cts into bbts. ! bool GenerateOopMap::merge_local_state_vectors(CellTypeState* cts, ! CellTypeState* bbts) { int i; int len = _max_locals + _stack_top; bool change = false; for (i = len - 1; i >= 0; i--) { *************** *** 694,711 **** CellTypeState v = cts[i].merge(bbts[i], i); change = change || !v.equal(bbts[i]); bbts[i] = v; } if (_max_monitors > 0 && _monitor_top != bad_monitors) { // If there are no monitors in the program, or there has been // a monitor matching error before this point in the program, // then we do not merge in the monitor state. int base = _max_locals + _max_stack; ! len = base + _monitor_top; ! for (i = len - 1; i >= base; i--) { CellTypeState v = cts[i].merge(bbts[i], i); // Can we prove that, when there has been a change, it will already // have been detected at this point? That would make this equal // check here unnecessary. --- 694,718 ---- CellTypeState v = cts[i].merge(bbts[i], i); change = change || !v.equal(bbts[i]); bbts[i] = v; } + return change; + } + + // Merge the monitor stack state from cts into bbts. + bool GenerateOopMap::merge_monitor_state_vectors(CellTypeState* cts, + CellTypeState* bbts) { + int change = false; if (_max_monitors > 0 && _monitor_top != bad_monitors) { // If there are no monitors in the program, or there has been // a monitor matching error before this point in the program, // then we do not merge in the monitor state. int base = _max_locals + _max_stack; ! int len = base + _monitor_top; ! for (int i = len - 1; i >= base; i--) { CellTypeState v = cts[i].merge(bbts[i], i); // Can we prove that, when there has been a change, it will already // have been detected at this point? That would make this equal // check here unnecessary. *************** *** 737,755 **** void GenerateOopMap::merge_state_into_bb(BasicBlock *bb) { assert(bb->is_alive(), "merging state into a dead basicblock"); if (_stack_top == bb->_stack_top) { if (_monitor_top == bb->_monitor_top) { ! if (merge_state_vectors(_state, bb->_state)) { bb->set_changed(true); } } else { ! if (TraceMonitorMismatch) { report_monitor_mismatch("monitor stack height merge conflict"); } ! // When the monitor stacks are not matched, we set _monitor_top to // bad_monitors. This signals that, from here on, the monitor stack cannot // be trusted. In particular, monitorexit bytecodes may throw // exceptions. We mark this block as changed so that the change // propagates properly. bb->_monitor_top = bad_monitors; --- 744,767 ---- void GenerateOopMap::merge_state_into_bb(BasicBlock *bb) { assert(bb->is_alive(), "merging state into a dead basicblock"); if (_stack_top == bb->_stack_top) { + // always merge local state even if monitors don't match. + if (merge_local_state_vectors(_state, bb->_state)) { + bb->set_changed(true); + } if (_monitor_top == bb->_monitor_top) { ! // monitors still match so continue merging monitor states. ! if (merge_monitor_state_vectors(_state, bb->_state)) { bb->set_changed(true); } } else { ! if (TraceMonitorMismatch) { report_monitor_mismatch("monitor stack height merge conflict"); } ! // When the monitor stacks are not matched, we set _monitor_top to // bad_monitors. This signals that, from here on, the monitor stack cannot // be trusted. In particular, monitorexit bytecodes may throw // exceptions. We mark this block as changed so that the change // propagates properly. bb->_monitor_top = bad_monitors; ###@###.### 2005-2-03 22:18:30 GMT
02-02-2005