8039905: heapdump/OnOOMToFile and heapdump/OnOOMToPath fail with "assert(fr().interpreter_frame_expression_stack_size() >= length) failed: error in expression stack!"

Reviewed-by: coleenp, sspitsyn

hotspot/src/share/vm/interpreter/oopMapCache.cpp

@@ -180,7 +180,7 @@ InterpreterOopMap::~InterpreterOopMap() {
   }
 }
 
-bool InterpreterOopMap::is_empty() {
+bool InterpreterOopMap::is_empty() const {
   bool result = _method == NULL;
   assert(_method != NULL || (_bci == 0 &&
     (_mask_size == 0 || _mask_size == USHRT_MAX) &&
@@ -196,7 +196,7 @@ void InterpreterOopMap::initialize() {
   for (int i = 0; i < N; i++) _bit_mask[i] = 0;
 }
 
-void InterpreterOopMap::iterate_oop(OffsetClosure* oop_closure) {
+void InterpreterOopMap::iterate_oop(OffsetClosure* oop_closure) const {
   int n = number_of_entries();
   int word_index = 0;
   uintptr_t value = 0;
@@ -238,7 +238,7 @@ void InterpreterOopMap::iterate_all(OffsetClosure* oop_closure, OffsetClosure* v
 #endif
 
 
-void InterpreterOopMap::print() {
+void InterpreterOopMap::print() const {
   int n = number_of_entries();
   tty->print("oop map for ");
   method()->print_value();
@@ -469,7 +469,7 @@ void InterpreterOopMap::resource_copy(OopMapCacheEntry* from) {
   }
 }
 
-inline unsigned int OopMapCache::hash_value_for(methodHandle method, int bci) {
+inline unsigned int OopMapCache::hash_value_for(methodHandle method, int bci) const {
   // We use method->code_size() rather than method->identity_hash() below since
   // the mark may not be present if a pointer to the method is already reversed.
   return   ((unsigned int) bci)
@@ -522,7 +522,7 @@ void OopMapCache::flush_obsolete_entries() {
 
 void OopMapCache::lookup(methodHandle method,
                          int bci,
-                         InterpreterOopMap* entry_for) {
+                         InterpreterOopMap* entry_for) const {
   MutexLocker x(&_mut);
 
   OopMapCacheEntry* entry = NULL;

hotspot/src/share/vm/interpreter/oopMapCache.hpp

@@ -101,32 +101,31 @@ class InterpreterOopMap: ResourceObj {
 
   // access methods
   Method*        method() const                  { return _method; }
-  void           set_method(Method* v)         { _method = v; }
+  void           set_method(Method* v)           { _method = v; }
   int            bci() const                     { return _bci; }
   void           set_bci(int v)                  { _bci = v; }
   int            mask_size() const               { return _mask_size; }
   void           set_mask_size(int v)            { _mask_size = v; }
-  int            number_of_entries() const       { return mask_size() / bits_per_entry; }
   // Test bit mask size and return either the in-line bit mask or allocated
   // bit mask.
-  uintptr_t*  bit_mask()                         { return (uintptr_t*)(mask_size() <= small_mask_limit ? (intptr_t)_bit_mask : _bit_mask[0]); }
+  uintptr_t*  bit_mask() const                   { return (uintptr_t*)(mask_size() <= small_mask_limit ? (intptr_t)_bit_mask : _bit_mask[0]); }
 
   // return the word size of_bit_mask.  mask_size() <= 4 * MAX_USHORT
-  size_t mask_word_size() {
+  size_t mask_word_size() const {
     return (mask_size() + BitsPerWord - 1) / BitsPerWord;
   }
 
-  uintptr_t entry_at(int offset)            { int i = offset * bits_per_entry; return bit_mask()[i / BitsPerWord] >> (i % BitsPerWord); }
+  uintptr_t entry_at(int offset) const           { int i = offset * bits_per_entry; return bit_mask()[i / BitsPerWord] >> (i % BitsPerWord); }
 
-  void set_expression_stack_size(int sz)    { _expression_stack_size = sz; }
+  void set_expression_stack_size(int sz)         { _expression_stack_size = sz; }
 
 #ifdef ENABLE_ZAP_DEAD_LOCALS
-  bool is_dead(int offset)                       { return (entry_at(offset) & (1 << dead_bit_number)) != 0; }
+  bool is_dead(int offset) const                 { return (entry_at(offset) & (1 << dead_bit_number)) != 0; }
 #endif
 
   // Lookup
-  bool match(methodHandle method, int bci)       { return _method == method() && _bci == bci; }
+  bool match(methodHandle method, int bci) const { return _method == method() && _bci == bci; }
-  bool is_empty();
+  bool is_empty() const;
 
   // Initialization
   void initialize();
@@ -141,12 +140,13 @@ class InterpreterOopMap: ResourceObj {
   // in-line), allocate the space from a Resource area.
   void resource_copy(OopMapCacheEntry* from);
 
-  void iterate_oop(OffsetClosure* oop_closure);
+  void iterate_oop(OffsetClosure* oop_closure) const;
-  void print();
+  void print() const;
 
-  bool is_oop  (int offset)                      { return (entry_at(offset) & (1 << oop_bit_number )) != 0; }
+  int number_of_entries() const                  { return mask_size() / bits_per_entry; }
+  bool is_oop (int offset) const                 { return (entry_at(offset) & (1 << oop_bit_number )) != 0; }
 
-  int expression_stack_size()                    { return _expression_stack_size; }
+  int expression_stack_size() const              { return _expression_stack_size; }
 
 #ifdef ENABLE_ZAP_DEAD_LOCALS
   void iterate_all(OffsetClosure* oop_closure, OffsetClosure* value_closure, OffsetClosure* dead_closure);
@@ -161,10 +161,10 @@ class OopMapCache : public CHeapObj<mtClass> {
 
   OopMapCacheEntry* _array;
 
-  unsigned int hash_value_for(methodHandle method, int bci);
+  unsigned int hash_value_for(methodHandle method, int bci) const;
   OopMapCacheEntry* entry_at(int i) const;
 
-  Mutex _mut;
+  mutable Mutex _mut;
 
   void flush();
 
@@ -177,7 +177,7 @@ class OopMapCache : public CHeapObj<mtClass> {
 
   // Returns the oopMap for (method, bci) in parameter "entry".
   // Returns false if an oop map was not found.
-  void lookup(methodHandle method, int bci, InterpreterOopMap* entry);
+  void lookup(methodHandle method, int bci, InterpreterOopMap* entry) const;
 
   // Compute an oop map without updating the cache or grabbing any locks (for debugging)
   static void compute_one_oop_map(methodHandle method, int bci, InterpreterOopMap* entry);

hotspot/src/share/vm/runtime/vframe.cpp

@@ -260,66 +260,148 @@ Method* interpretedVFrame::method() const {
   return fr().interpreter_frame_method();
 }
 
-StackValueCollection* interpretedVFrame::locals() const {
+static StackValue* create_stack_value_from_oop_map(const InterpreterOopMap& oop_mask,
-  int length = method()->max_locals();
+                                                   int index,
+                                                   const intptr_t* const addr) {
+  // categorize using oop_mask
+  if (oop_mask.is_oop(index)) {
+    // reference (oop) "r"
+    Handle h(addr != NULL ? (*(oop*)addr) : (oop)NULL);
+    return new StackValue(h);
+  }
+  // value (integer) "v"
+  return new StackValue(addr != NULL ? *addr : 0);
+}
 
-  if (method()->is_native()) {
+static bool is_in_expression_stack(const frame& fr, const intptr_t* const addr) {
-    // If the method is native, max_locals is not telling the truth.
+  assert(addr != NULL, "invariant");
-    // maxlocals then equals the size of parameters
+
-    length = method()->size_of_parameters();
+  // Ensure to be 'inside' the expresion stack (i.e., addr >= sp for Intel).
+  // In case of exceptions, the expression stack is invalid and the sp
+  // will be reset to express this condition.
+  if (frame::interpreter_frame_expression_stack_direction() > 0) {
+    return addr <= fr.interpreter_frame_tos_address();
   }
 
-  StackValueCollection* result = new StackValueCollection(length);
+  return addr >= fr.interpreter_frame_tos_address();
+}
+
+static void stack_locals(StackValueCollection* result,
+                         int length,
+                         const InterpreterOopMap& oop_mask,
+                         const frame& fr) {
+
+  assert(result != NULL, "invariant");
+
+  for (int i = 0; i < length; ++i) {
+    const intptr_t* const addr = fr.interpreter_frame_local_at(i);
+    assert(addr != NULL, "invariant");
+    assert(addr >= fr.sp(), "must be inside the frame");
+
+    StackValue* const sv = create_stack_value_from_oop_map(oop_mask, i, addr);
+    assert(sv != NULL, "sanity check");
+
+    result->add(sv);
+  }
+}
+
+static void stack_expressions(StackValueCollection* result,
+                              int length,
+                              const InterpreterOopMap& oop_mask,
+                              const frame& fr) {
+
+  assert(result != NULL, "invariant");
+
+  for (int i = 0; i < length; ++i) {
+    const intptr_t* addr = fr.interpreter_frame_expression_stack_at(i);
+    assert(addr != NULL, "invariant");
+    if (!is_in_expression_stack(fr, addr)) {
+      // Need to ensure no bogus escapes.
+      addr = NULL;
+    }
+    StackValue* const sv = create_stack_value_from_oop_map(oop_mask, i, addr);
+    assert(sv != NULL, "sanity check");
+
+    result->add(sv);
+  }
+}
+
+StackValueCollection* interpretedVFrame::locals() const {
+  return stack_data(false);
+}
+
+StackValueCollection* interpretedVFrame::expressions() const {
+  return stack_data(true);
+}
+
+/*
+ * Worker routine for fetching references and/or values
+ * for a particular bci in the interpretedVFrame.
+ *
+ * Returns data for either "locals" or "expressions",
+ * using bci relative oop_map (oop_mask) information.
+ *
+ * @param expressions  bool switch controlling what data to return
+                       (false == locals / true == expression)
+ *
+ */
+StackValueCollection* interpretedVFrame::stack_data(bool expressions) const {
 
-  // Get oopmap describing oops and int for current bci
   InterpreterOopMap oop_mask;
+  // oopmap for current bci
   if (TraceDeoptimization && Verbose) {
-    // need the current JavaThread and not thread()
     methodHandle m_h(Thread::current(), method());
     OopMapCache::compute_one_oop_map(m_h, bci(), &oop_mask);
   } else {
     method()->mask_for(bci(), &oop_mask);
   }
-  // handle locals
-  for(int i=0; i < length; i++) {
-    // Find stack location
-    intptr_t *addr = locals_addr_at(i);
 
-    // Depending on oop/int put it in the right package
+  const int mask_len = oop_mask.number_of_entries();
-    StackValue *sv;
+
-    if (oop_mask.is_oop(i)) {
+  // If the method is native, method()->max_locals() is not telling the truth.
-      // oop value
+  // For our purposes, max locals instead equals the size of parameters.
-      Handle h(*(oop *)addr);
+  const int max_locals = method()->is_native() ?
-      sv = new StackValue(h);
+    method()->size_of_parameters() : method()->max_locals();
-    } else {
+
-      // integer
+  assert(mask_len >= max_locals, "invariant");
-      sv = new StackValue(*addr);
+
-    }
+  const int length = expressions ? mask_len - max_locals : max_locals;
-    assert(sv != NULL, "sanity check");
+  assert(length >= 0, "invariant");
-    result->add(sv);
+
+  StackValueCollection* const result = new StackValueCollection(length);
+
+  if (0 == length) {
+    return result;
   }
+
+  if (expressions) {
+    stack_expressions(result, length, oop_mask, fr());
+  } else {
+    stack_locals(result, length, oop_mask, fr());
+  }
+
+  assert(length == result->size(), "invariant");
+
   return result;
 }
 
 void interpretedVFrame::set_locals(StackValueCollection* values) const {
   if (values == NULL || values->size() == 0) return;
 
-  int length = method()->max_locals();
+  // If the method is native, max_locals is not telling the truth.
-  if (method()->is_native()) {
+  // maxlocals then equals the size of parameters
-    // If the method is native, max_locals is not telling the truth.
+  const int max_locals = method()->is_native() ?
-    // maxlocals then equals the size of parameters
+    method()->size_of_parameters() : method()->max_locals();
-    length = method()->size_of_parameters();
-  }
 
-  assert(length == values->size(), "Mismatch between actual stack format and supplied data");
+  assert(max_locals == values->size(), "Mismatch between actual stack format and supplied data");
 
   // handle locals
-  for (int i = 0; i < length; i++) {
+  for (int i = 0; i < max_locals; i++) {
     // Find stack location
     intptr_t *addr = locals_addr_at(i);
 
     // Depending on oop/int put it in the right package
-    StackValue *sv = values->at(i);
+    const StackValue* const sv = values->at(i);
     assert(sv != NULL, "sanity check");
     if (sv->type() == T_OBJECT) {
       *(oop *) addr = (sv->get_obj())();
@@ -329,61 +411,6 @@ void interpretedVFrame::set_locals(StackValueCollection* values) const {
   }
 }
 
-StackValueCollection* interpretedVFrame::expressions() const {
-
-  InterpreterOopMap oop_mask;
-
-  if (!method()->is_native()) {
-    // Get oopmap describing oops and int for current bci
-    if (TraceDeoptimization && Verbose) {
-      // need the current JavaThread and not thread()
-      methodHandle m_h(Thread::current(), method());
-      OopMapCache::compute_one_oop_map(m_h, bci(), &oop_mask);
-    } else {
-      method()->mask_for(bci(), &oop_mask);
-    }
-  }
-
-  // If the bci is a call instruction, i.e. any of the invoke* instructions,
-  // the InterpreterOopMap does not include expression/operand stack liveness
-  // info in the oop_mask/bit_mask. This can lead to a discrepancy of what
-  // is actually on the expression stack compared to what is given by the
-  // oop_map. We need to use the length reported in the oop_map.
-  int length = oop_mask.expression_stack_size();
-
-  assert(fr().interpreter_frame_expression_stack_size() >= length,
-    "error in expression stack!");
-
-  StackValueCollection* result = new StackValueCollection(length);
-
-  if (0 == length) {
-    return result;
-  }
-
-  int nof_locals = method()->max_locals();
-
-  // handle expressions
-  for(int i=0; i < length; i++) {
-    // Find stack location
-    intptr_t *addr = fr().interpreter_frame_expression_stack_at(i);
-
-    // Depending on oop/int put it in the right package
-    StackValue *sv;
-    if (oop_mask.is_oop(i + nof_locals)) {
-      // oop value
-      Handle h(*(oop *)addr);
-      sv = new StackValue(h);
-    } else {
-      // integer
-      sv = new StackValue(*addr);
-    }
-    assert(sv != NULL, "sanity check");
-    result->add(sv);
-  }
-  return result;
-}
-
-
 // ------------- cChunk --------------
 
 entryVFrame::entryVFrame(const frame* fr, const RegisterMap* reg_map, JavaThread* thread)

hotspot/src/share/vm/runtime/vframe.hpp

@@ -186,7 +186,7 @@ class interpretedVFrame: public javaVFrame {
  private:
   static const int bcp_offset;
   intptr_t* locals_addr_at(int offset) const;
-
+  StackValueCollection* stack_data(bool expressions) const;
   // returns where the parameters starts relative to the frame pointer
   int start_of_parameters() const;