8080692: lots of jstack tests failing in pit
authorrbackman
Thu, 21 May 2015 18:10:18 +0200
changeset 30774 6745424a720f
parent 30635 8884c2399789
child 30775 7ef17210a4b4
8080692: lots of jstack tests failing in pit Reviewed-by: kvn, vlivanov
hotspot/agent/src/share/classes/sun/jvm/hotspot/HSDB.java
hotspot/agent/src/share/classes/sun/jvm/hotspot/compiler/ImmutableOopMapPair.java
hotspot/agent/src/share/classes/sun/jvm/hotspot/compiler/ImmutableOopMapSet.java
hotspot/agent/src/share/classes/sun/jvm/hotspot/ui/classbrowser/HTMLGenerator.java
hotspot/src/share/vm/compiler/oopMap.cpp
hotspot/src/share/vm/compiler/oopMap.hpp
--- a/hotspot/agent/src/share/classes/sun/jvm/hotspot/HSDB.java	Thu May 14 12:05:32 2015 -0700
+++ b/hotspot/agent/src/share/classes/sun/jvm/hotspot/HSDB.java	Thu May 21 18:10:18 2015 +0200
@@ -927,7 +927,7 @@
             if (curVFrame.isCompiledFrame()) {
               CodeBlob cb = VM.getVM().getCodeCache().findBlob(curFrame.getPC());
               ImmutableOopMapSet maps = cb.getOopMaps();
-              if ((maps == null) || (maps.getSize() == 0)) {
+              if ((maps == null) || (maps.getCount() == 0)) {
                 shouldSkipOopMaps = true;
               }
             }
--- a/hotspot/agent/src/share/classes/sun/jvm/hotspot/compiler/ImmutableOopMapPair.java	Thu May 14 12:05:32 2015 -0700
+++ b/hotspot/agent/src/share/classes/sun/jvm/hotspot/compiler/ImmutableOopMapPair.java	Thu May 21 18:10:18 2015 +0200
@@ -71,4 +71,8 @@
     offsetField = type.getCIntegerField("_oopmap_offset");
     classSize = type.getSize();
   }
+
+  public String toString() {
+    return "Pair{pc_offset = " + getPC() + ", data_offset = " + getOffset() + "}";
+  }
 }
--- a/hotspot/agent/src/share/classes/sun/jvm/hotspot/compiler/ImmutableOopMapSet.java	Thu May 14 12:05:32 2015 -0700
+++ b/hotspot/agent/src/share/classes/sun/jvm/hotspot/compiler/ImmutableOopMapSet.java	Thu May 21 18:10:18 2015 +0200
@@ -106,19 +106,19 @@
   /**
    * Returns the number of OopMaps in this ImmutableOopMapSet
    */
-  public long getSize() {
-    return countField.getValue(addr);
-  }
-
   public int getCount() { return (int) countField.getValue(addr); }
 
   private Address dataStart() {
-    return (addr.addOffsetTo(ImmutableOopMapSet.classSize * getCount()));
+    return (pairStart().addOffsetTo(ImmutableOopMapPair.classSize() * getCount()));
+  }
+
+  private Address pairStart() {
+    return addr.addOffsetTo(ImmutableOopMapSet.classSize);
   }
 
   public ImmutableOopMapPair pairAt(int index) {
     Assert.that((index >= 0) && (index < getCount()), "bad index");
-    return new ImmutableOopMapPair(addr.addOffsetTo(index * ImmutableOopMapPair.classSize()));
+    return new ImmutableOopMapPair(pairStart().addOffsetTo(index * ImmutableOopMapPair.classSize()));
   }
 
   /**
@@ -126,7 +126,7 @@
    */
   public ImmutableOopMap getMapAt(int index) {
     if (Assert.ASSERTS_ENABLED) {
-      Assert.that((index >= 0) && (index <= getSize()), "bad index");
+      Assert.that((index >= 0) && (index <= getCount()), "bad index");
     }
 
     ImmutableOopMapPair immutableOopMapPair = pairAt(index);
@@ -135,7 +135,7 @@
 
   public ImmutableOopMap findMapAtOffset(long pcOffset, boolean debugging) {
     int i;
-    int len = (int) getSize();
+    int len = getCount();
     if (Assert.ASSERTS_ENABLED) {
       Assert.that(len > 0, "must have pointer maps");
     }
@@ -253,14 +253,14 @@
     if (!VM.getVM().isDebugging()) {
       if (Assert.ASSERTS_ENABLED) {
         ImmutableOopMapSet maps = cb.getOopMaps();
-        Assert.that((maps != null) && (maps.getSize() > 0), "found null or empty ImmutableOopMapSet for CodeBlob");
+        Assert.that((maps != null) && (maps.getCount() > 0), "found null or empty ImmutableOopMapSet for CodeBlob");
       }
     } else {
       // Hack for some topmost frames that have been found with empty
       // OopMapSets. (Actually have not seen the null case, but don't
       // want to take any chances.) See HSDB.showThreadStackMemory().
       ImmutableOopMapSet maps = cb.getOopMaps();
-      if ((maps == null) || (maps.getSize() == 0)) {
+      if ((maps == null) || (maps.getCount() == 0)) {
         return;
       }
     }
@@ -311,8 +311,28 @@
     return pairAt(index);
   }
 
+  private int getSize() {
+    return (int) sizeField.getValue(addr);
+  }
+
   public ImmutableOopMap getMap(ImmutableOopMapPair pair) {
-    Assert.that(pair.getOffset() < (int) sizeField.getValue(), "boundary check");
+    Assert.that(pair.getOffset() < getSize(), "boundary check: this: " + this + " offset: " + pair);
     return new ImmutableOopMap(dataStart().addOffsetTo(pair.getOffset()));
   }
+
+  public String toString() {
+    StringBuilder builder = new StringBuilder();
+    builder.append("Set{ ")
+      .append("addr = ").append(addr)
+      .append(", count = ").append(getCount())
+      .append(", size = ").append(getSize())
+      .append(", pairs = [");
+
+    for (int i = 0; i < getCount(); ++i) {
+      builder.append(getPairAt(i));
+    }
+
+    builder.append("]");
+    return builder.toString();
+  }
 }
--- a/hotspot/agent/src/share/classes/sun/jvm/hotspot/ui/classbrowser/HTMLGenerator.java	Thu May 14 12:05:32 2015 -0700
+++ b/hotspot/agent/src/share/classes/sun/jvm/hotspot/ui/classbrowser/HTMLGenerator.java	Thu May 21 18:10:18 2015 +0200
@@ -1236,7 +1236,7 @@
 
    protected String genOopMapInfo(NMethod nmethod, PCDesc pcDesc) {
       ImmutableOopMapSet mapSet = nmethod.getOopMaps();
-      if (mapSet == null || (mapSet.getSize() <= 0))
+      if (mapSet == null || (mapSet.getCount() <= 0))
         return "";
       int pcOffset = pcDesc.getPCOffset();
       ImmutableOopMap map = mapSet.findMapAtOffset(pcOffset, VM.getVM().isDebugging());
--- a/hotspot/src/share/vm/compiler/oopMap.cpp	Thu May 14 12:05:32 2015 -0700
+++ b/hotspot/src/share/vm/compiler/oopMap.cpp	Thu May 21 18:10:18 2015 +0200
@@ -596,6 +596,17 @@
   oopmap->copy_data_to(addr);
 }
 
+#ifdef ASSERT
+int ImmutableOopMap::nr_of_bytes() const {
+  OopMapStream oms(this);
+
+  while (!oms.is_done()) {
+    oms.next();
+  }
+  return sizeof(ImmutableOopMap) + oms.stream_position();
+}
+#endif
+
 class ImmutableOopMapBuilder {
 private:
   class Mapping;
@@ -652,7 +663,7 @@
   }
 
 #ifdef ASSERT
-  void verify(address buffer, int size);
+  void verify(address buffer, int size, const ImmutableOopMapSet* set);
 #endif
 
   bool has_empty() const {
@@ -660,8 +671,8 @@
   }
 
   int size_for(const OopMap* map) const;
-  void fill_pair(ImmutableOopMapPair* pair, const OopMap* map, int offset);
-  int fill_map(ImmutableOopMapPair* pair, const OopMap* map, int offset);
+  void fill_pair(ImmutableOopMapPair* pair, const OopMap* map, int offset, const ImmutableOopMapSet* set);
+  int fill_map(ImmutableOopMapPair* pair, const OopMap* map, int offset, const ImmutableOopMapSet* set);
   void fill(ImmutableOopMapSet* set, int size);
 };
 
@@ -711,12 +722,13 @@
   return total;
 }
 
-void ImmutableOopMapBuilder::fill_pair(ImmutableOopMapPair* pair, const OopMap* map, int offset) {
+void ImmutableOopMapBuilder::fill_pair(ImmutableOopMapPair* pair, const OopMap* map, int offset, const ImmutableOopMapSet* set) {
+  assert(offset < set->nr_of_bytes(), "check");
   new ((address) pair) ImmutableOopMapPair(map->offset(), offset);
 }
 
-int ImmutableOopMapBuilder::fill_map(ImmutableOopMapPair* pair, const OopMap* map, int offset) {
-  fill_pair(pair, map, offset);
+int ImmutableOopMapBuilder::fill_map(ImmutableOopMapPair* pair, const OopMap* map, int offset, const ImmutableOopMapSet* set) {
+  fill_pair(pair, map, offset, set);
   address addr = (address) pair->get_from(_new_set); // location of the ImmutableOopMap
 
   new (addr) ImmutableOopMap(map);
@@ -732,9 +744,9 @@
     int size = 0;
 
     if (_mapping[i]._kind == Mapping::OOPMAP_NEW) {
-      size = fill_map(&pairs[i], map, _mapping[i]._offset);
+      size = fill_map(&pairs[i], map, _mapping[i]._offset, set);
     } else if (_mapping[i]._kind == Mapping::OOPMAP_DUPLICATE || _mapping[i]._kind == Mapping::OOPMAP_EMPTY) {
-      fill_pair(&pairs[i], map, _mapping[i]._offset);
+      fill_pair(&pairs[i], map, _mapping[i]._offset, set);
     }
 
     const ImmutableOopMap* nv = set->find_map_at_offset(map->offset());
@@ -743,10 +755,18 @@
 }
 
 #ifdef ASSERT
-void ImmutableOopMapBuilder::verify(address buffer, int size) {
+void ImmutableOopMapBuilder::verify(address buffer, int size, const ImmutableOopMapSet* set) {
   for (int i = 0; i < 8; ++i) {
     assert(buffer[size - 8 + i] == (unsigned char) 0xff, "overwritten memory check");
   }
+
+  for (int i = 0; i < set->count(); ++i) {
+    const ImmutableOopMapPair* pair = set->pair_at(i);
+    assert(pair->oopmap_offset() < set->nr_of_bytes(), "check size");
+    const ImmutableOopMap* map = pair->get_from(set);
+    int nr_of_bytes = map->nr_of_bytes();
+    assert(pair->oopmap_offset() + nr_of_bytes <= set->nr_of_bytes(), "check size + size");
+  }
 }
 #endif
 
@@ -760,7 +780,7 @@
   _new_set = new (buffer) ImmutableOopMapSet(_set, required);
   fill(_new_set, required);
 
-  DEBUG_ONLY(verify(buffer, required));
+  DEBUG_ONLY(verify(buffer, required, _new_set));
 
   return _new_set;
 }
--- a/hotspot/src/share/vm/compiler/oopMap.hpp	Thu May 14 12:05:32 2015 -0700
+++ b/hotspot/src/share/vm/compiler/oopMap.hpp	Thu May 21 18:10:18 2015 +0200
@@ -273,6 +273,9 @@
 
   bool has_derived_pointer() const PRODUCT_RETURN0;
   int count() const { return _count; }
+#ifdef ASSERT
+  int nr_of_bytes() const; // this is an expensive operation, only used in debug builds
+#endif
 
   // Printing
   void print_on(outputStream* st) const;
@@ -346,6 +349,9 @@
   bool is_done()                        { if(!_valid_omv) { find_next(); } return !_valid_omv; }
   void next()                           { find_next(); }
   OopMapValue current()                 { return _omv; }
+#ifdef ASSERT
+  int stream_position() const           { return _stream->position(); }
+#endif
 };