United StatesChange Country, Oracle Worldwide Web Sites Communities I am a... I want to...
Bug ID: JDK-6224591 JVM stops merging state vectors for blocks where there's a monitor mismatch.
JDK-6224591 : JVM stops merging state vectors for blocks where there's a monitor mismatch.

Details
Type:
Bug
Submit Date:
2005-02-02
Status:
Closed
Updated Date:
2012-10-08
Project Name:
JDK
Resolved Date:
2005-03-17
Component:
hotspot
OS:
solaris_8
Sub-Component:
runtime
CPU:
generic
Priority:
P2
Resolution:
Fixed
Affected Versions:
1.3.1_13,1.4.2_05
Fixed Versions:

Related Reports
Backport:
Backport:
Backport:

Sub Tasks

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
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
                                     
2005-02-02
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
                                     
2005-02-02
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
                                     
2005-02-03



Hardware and Software, Engineered to Work Together