8239497: SEGV in EdgeUtils::field_name_symbol(Edge const&)

Reviewed-by: egahlin, tonyp

src/hotspot/share/jfr/leakprofiler/chains/edge.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2014, 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2014, 2020, 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
@@ -34,22 +34,7 @@ const oop Edge::pointee() const {
 }
 
 const oop Edge::reference_owner() const {
-  return is_root() ? (oop)NULL : _parent->reference().dereference();
-}
-
-static const Klass* resolve_klass(const oop obj) {
-  assert(obj != NULL, "invariant");
-  return java_lang_Class::is_instance(obj) ?
-    java_lang_Class::as_Klass(obj) : obj->klass();
-}
-
-const Klass* Edge::pointee_klass() const {
-  return resolve_klass(pointee());
-}
-
-const Klass* Edge::reference_owner_klass() const {
-  const oop ref_owner = reference_owner();
-  return ref_owner != NULL ? resolve_klass(ref_owner) : NULL;
+  return is_root() ? (oop)NULL : _parent->pointee();
 }
 
 size_t Edge::distance_to_root() const {

src/hotspot/share/jfr/leakprofiler/chains/edge.hpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2014, 2019, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2014, 2020, 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
@@ -46,9 +46,7 @@ class Edge {
     return _parent == NULL;
   }
   const oop pointee() const;
-  const Klass* pointee_klass() const;
   const oop reference_owner() const;
-  const Klass* reference_owner_klass() const;
   size_t distance_to_root() const;
 
   void* operator new (size_t sz, void* here) {

src/hotspot/share/jfr/leakprofiler/chains/edgeUtils.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2014, 2019, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2014, 2020, 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
@@ -30,6 +30,7 @@
 #include "jfr/leakprofiler/utilities/unifiedOopRef.inline.hpp"
 #include "oops/fieldStreams.inline.hpp"
 #include "oops/instanceKlass.hpp"
+#include "oops/instanceMirrorKlass.hpp"
 #include "oops/objArrayOop.inline.hpp"
 #include "oops/oopsHierarchy.hpp"
 #include "runtime/handles.inline.hpp"
@@ -38,59 +39,53 @@ bool EdgeUtils::is_leak_edge(const Edge& edge) {
   return (const Edge*)edge.pointee()->mark().to_pointer() == &edge;
 }
 
-static int field_offset(const StoredEdge& edge) {
-  assert(!edge.is_root(), "invariant");
-  const oop ref_owner = edge.reference_owner();
+static bool is_static_field(const oop ref_owner, const InstanceKlass* ik, int offset) {
+  assert(ref_owner != NULL, "invariant");
+  assert(ik != NULL, "invariant");
+  assert(ref_owner->klass() == ik, "invariant");
+  return ik->is_mirror_instance_klass() && offset >= InstanceMirrorKlass::cast(ik)->offset_of_static_fields();
+}
+
+static int field_offset(const Edge& edge, const oop ref_owner) {
   assert(ref_owner != NULL, "invariant");
-  UnifiedOopRef reference = edge.reference();
-  assert(!reference.is_null(), "invariant");
   assert(!ref_owner->is_array(), "invariant");
   assert(ref_owner->is_instance(), "invariant");
+  UnifiedOopRef reference = edge.reference();
+  assert(!reference.is_null(), "invariant");
   const int offset = (int)(reference.addr<uintptr_t>() - cast_from_oop<uintptr_t>(ref_owner));
-  assert(offset < (ref_owner->size() * HeapWordSize), "invariant");
+  assert(offset < ref_owner->size() * HeapWordSize, "invariant");
   return offset;
 }
 
-static const InstanceKlass* field_type(const StoredEdge& edge) {
-  assert(!edge.is_root() || !EdgeUtils::is_array_element(edge), "invariant");
-  return (const InstanceKlass*)edge.reference_owner_klass();
-}
-
-const Symbol* EdgeUtils::field_name_symbol(const Edge& edge) {
+const Symbol* EdgeUtils::field_name(const Edge& edge, jshort* modifiers) {
   assert(!edge.is_root(), "invariant");
-  assert(!is_array_element(edge), "invariant");
-  const int offset = field_offset(edge);
-  const InstanceKlass* ik = field_type(edge);
+  assert(!EdgeUtils::is_array_element(edge), "invariant");
+  assert(modifiers != NULL, "invariant");
+  const oop ref_owner = edge.reference_owner();
+  assert(ref_owner != NULL, "invariant");
+  assert(ref_owner->klass()->is_instance_klass(), "invariant");
+  const InstanceKlass* ik = InstanceKlass::cast(ref_owner->klass());
+  const int offset = field_offset(edge, ref_owner);
+  if (is_static_field(ref_owner, ik, offset)) {
+    assert(ik->is_mirror_instance_klass(), "invariant");
+    assert(java_lang_Class::as_Klass(ref_owner)->is_instance_klass(), "invariant");
+    ik = InstanceKlass::cast(java_lang_Class::as_Klass(ref_owner));
+  }
   while (ik != NULL) {
     JavaFieldStream jfs(ik);
     while (!jfs.done()) {
       if (offset == jfs.offset()) {
+        *modifiers = jfs.access_flags().as_short();
         return jfs.name();
       }
       jfs.next();
     }
-    ik = (InstanceKlass*)ik->super();
+    ik = (const InstanceKlass*)ik->super();
   }
+  *modifiers = 0;
   return NULL;
 }
 
-jshort EdgeUtils::field_modifiers(const Edge& edge) {
-  const int offset = field_offset(edge);
-  const InstanceKlass* ik = field_type(edge);
-
-  while (ik != NULL) {
-    JavaFieldStream jfs(ik);
-    while (!jfs.done()) {
-      if (offset == jfs.offset()) {
-        return jfs.access_flags().as_short();
-      }
-      jfs.next();
-    }
-    ik = (InstanceKlass*)ik->super();
-  }
-  return 0;
-}
-
 bool EdgeUtils::is_array_element(const Edge& edge) {
   assert(!edge.is_root(), "invariant");
   const oop ref_owner = edge.reference_owner();
@@ -99,7 +94,7 @@ bool EdgeUtils::is_array_element(const Edge& edge) {
 }
 
 static int array_offset(const Edge& edge) {
-  assert(!edge.is_root(), "invariant");
+  assert(EdgeUtils::is_array_element(edge), "invariant");
   const oop ref_owner = edge.reference_owner();
   assert(ref_owner != NULL, "invariant");
   UnifiedOopRef reference = edge.reference();
@@ -112,17 +107,15 @@ static int array_offset(const Edge& edge) {
 }
 
 int EdgeUtils::array_index(const Edge& edge) {
-  return is_array_element(edge) ? array_offset(edge) : 0;
+  return array_offset(edge);
 }
 
 int EdgeUtils::array_size(const Edge& edge) {
-  if (is_array_element(edge)) {
-    const oop ref_owner = edge.reference_owner();
-    assert(ref_owner != NULL, "invariant");
-    assert(ref_owner->is_objArray(), "invariant");
-    return ((objArrayOop)ref_owner)->length();
-  }
-  return 0;
+  assert(is_array_element(edge), "invariant");
+  const oop ref_owner = edge.reference_owner();
+  assert(ref_owner != NULL, "invariant");
+  assert(ref_owner->is_objArray(), "invariant");
+  return ((objArrayOop)ref_owner)->length();
 }
 
 const Edge* EdgeUtils::root(const Edge& edge) {

src/hotspot/share/jfr/leakprofiler/chains/edgeUtils.hpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2014, 2019, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2014, 2020, 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
@@ -44,8 +44,8 @@ class EdgeUtils : public AllStatic {
   static int array_index(const Edge& edge);
   static int array_size(const Edge& edge);
 
-  static const Symbol* field_name_symbol(const Edge& edge);
-  static jshort field_modifiers(const Edge& edge);
+  static const Symbol* field_name(const Edge& edge, jshort* modifiers);
+
 };
 
 #endif // SHARE_JFR_LEAKPROFILER_CHAINS_EDGEUTILS_HPP

src/hotspot/share/jfr/leakprofiler/checkpoint/objectSampleWriter.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2014, 2019, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2014, 2020, 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
@@ -298,22 +298,20 @@ static traceid get_field_info_id(const Edge& edge) {
   if (edge.is_root()) {
     return 0;
   }
-
   assert(!EdgeUtils::is_array_element(edge), "invariant");
-  const Symbol* const field_name_symbol = EdgeUtils::field_name_symbol(edge);
+  jshort field_modifiers;
+  const Symbol* const field_name_symbol = EdgeUtils::field_name(edge, &field_modifiers);
   if (field_name_symbol == NULL) {
     return 0;
   }
-
   if (field_infos == NULL) {
     field_infos = new FieldTable();
   }
   assert(field_infos != NULL, "invariant");
-
   ObjectSampleFieldInfo* const osfi = new ObjectSampleFieldInfo();
   assert(osfi != NULL, "invariant");
   osfi->_field_name_symbol = field_name_symbol;
-  osfi->_field_modifiers = EdgeUtils::field_modifiers(edge);
+  osfi->_field_modifiers = field_modifiers;
   return field_infos->store(osfi);
 }
 
@@ -527,7 +525,7 @@ static void add_old_object_sample_info(const StoredEdge* current, traceid id) {
   assert(oosi != NULL, "invariant");
   oosi->_id = id;
   oosi->_data._object = current->pointee();
-  oosi->_data._reference_id = current->parent() == NULL ? (traceid)0 : id;
+  oosi->_data._reference_id = current->parent() == NULL ? 0 : id;
   sample_infos->store(oosi);
 }
 
@@ -542,8 +540,8 @@ static void add_reference_info(const StoredEdge* current, traceid id, traceid pa
   assert(ri != NULL, "invariant");
 
   ri->_id = id;
-  ri->_data._array_info_id =  !current->is_skip_edge() ? get_array_info_id(*current, id) : 0;
-  ri->_data._field_info_id = ri->_data._array_info_id == 0 && !current->is_skip_edge() ? get_field_info_id(*current) : (traceid)0;
+  ri->_data._array_info_id =  current->is_skip_edge() ? 0 : get_array_info_id(*current, id);
+  ri->_data._field_info_id = ri->_data._array_info_id != 0 || current->is_skip_edge() ? 0 : get_field_info_id(*current);
   ri->_data._old_object_sample_id = parent_id;
   ri->_data._skip = current->skip_length();
   ref_infos->store(ri);
@@ -567,11 +565,11 @@ void ObjectSampleWriter::write(const StoredEdge* edge) {
   const StoredEdge* const parent = edge->parent();
   if (parent != NULL) {
     add_reference_info(edge, id, _store->get_id(parent));
-  } else {
-    if (is_gc_root(edge)) {
-      assert(edge->gc_root_id() == id, "invariant");
-      add_gc_root_info(edge, id);
-    }
+    return;
+  }
+  if (is_gc_root(edge)) {
+    assert(edge->gc_root_id() == id, "invariant");
+    add_gc_root_info(edge, id);
   }
 }
 

src/hotspot/share/oops/instanceMirrorKlass.hpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2011, 2019, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2011, 2020, 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
@@ -55,11 +55,13 @@ class InstanceMirrorKlass: public InstanceKlass {
  public:
   InstanceMirrorKlass() { assert(DumpSharedSpaces || UseSharedSpaces, "only for CDS"); }
 
-  // Casting from Klass*
   static InstanceMirrorKlass* cast(Klass* k) {
-    assert(InstanceKlass::cast(k)->is_mirror_instance_klass(),
-           "cast to InstanceMirrorKlass");
-    return static_cast<InstanceMirrorKlass*>(k);
+    return const_cast<InstanceMirrorKlass*>(cast(const_cast<const Klass*>(k)));
+  }
+
+  static const InstanceMirrorKlass* cast(const Klass* k) {
+    assert(InstanceKlass::cast(k)->is_mirror_instance_klass(), "cast to InstanceMirrorKlass");
+    return static_cast<const InstanceMirrorKlass*>(k);
   }
 
   // Returns the size of the instance including the extra static fields.