8318566: Heap walking functions should not use FilteredFieldStream

Reviewed-by: cjplummer, sspitsyn

src/hotspot/share/ci/ciField.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1999, 2023, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1999, 2024, 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
@@ -35,7 +35,7 @@
 #include "oops/oop.inline.hpp"
 #include "runtime/fieldDescriptor.inline.hpp"
 #include "runtime/handles.inline.hpp"
-#include "runtime/reflectionUtils.hpp"
+#include "runtime/reflection.hpp"
 
 // ciField
 //

src/hotspot/share/jvmci/jvmciCompilerToVM.cpp

@@ -61,7 +61,7 @@
 #include "runtime/globals_extension.hpp"
 #include "runtime/interfaceSupport.inline.hpp"
 #include "runtime/jniHandles.inline.hpp"
-#include "runtime/reflectionUtils.hpp"
+#include "runtime/reflection.hpp"
 #include "runtime/stackFrameStream.inline.hpp"
 #include "runtime/timerTrace.hpp"
 #include "runtime/vframe.inline.hpp"

src/hotspot/share/jvmci/jvmciRuntime.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2012, 2023, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2012, 2024, 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
@@ -54,7 +54,7 @@
 #include "runtime/java.hpp"
 #include "runtime/jniHandles.inline.hpp"
 #include "runtime/mutex.hpp"
-#include "runtime/reflectionUtils.hpp"
+#include "runtime/reflection.hpp"
 #include "runtime/sharedRuntime.hpp"
 #include "runtime/synchronizer.hpp"
 #if INCLUDE_G1GC

src/hotspot/share/oops/instanceKlass.cpp

@@ -85,7 +85,7 @@
 #include "runtime/mutexLocker.hpp"
 #include "runtime/orderAccess.hpp"
 #include "runtime/os.inline.hpp"
-#include "runtime/reflectionUtils.hpp"
+#include "runtime/reflection.hpp"
 #include "runtime/threads.hpp"
 #include "services/classLoadingService.hpp"
 #include "services/finalizerService.hpp"

src/hotspot/share/prims/jvmtiTagMap.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2003, 2023, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2003, 2024, 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
@@ -397,6 +397,9 @@ class ClassFieldMap: public CHeapObj<mtInternal> {
   // constructor
   ClassFieldMap();
 
+  // calculates number of fields in all interfaces
+  static int interfaces_field_count(InstanceKlass* ik);
+
   // add a field
   void add(int index, char type, int offset);
 
@@ -424,6 +427,16 @@ ClassFieldMap::~ClassFieldMap() {
   delete _fields;
 }
 
+int ClassFieldMap::interfaces_field_count(InstanceKlass* ik) {
+  const Array<InstanceKlass*>* interfaces = ik->transitive_interfaces();
+  int count = 0;
+  for (int i = 0; i < interfaces->length(); i++) {
+    FilteredJavaFieldStream fld(interfaces->at(i));
+    count += fld.field_count();
+  }
+  return count;
+}
+
 void ClassFieldMap::add(int index, char type, int offset) {
   ClassFieldDescriptor* field = new ClassFieldDescriptor(index, type, offset);
   _fields->append(field);
@@ -431,48 +444,59 @@ void ClassFieldMap::add(int index, char type, int offset) {
 
 // Returns a heap allocated ClassFieldMap to describe the static fields
 // of the given class.
-//
 ClassFieldMap* ClassFieldMap::create_map_of_static_fields(Klass* k) {
   InstanceKlass* ik = InstanceKlass::cast(k);
 
   // create the field map
   ClassFieldMap* field_map = new ClassFieldMap();
 
-  FilteredFieldStream f(ik, false, false);
+  // Static fields of interfaces and superclasses are reported as references from the interfaces/superclasses.
-  int max_field_index = f.field_count()-1;
+  // Need to calculate start index of this class fields: number of fields in all interfaces and superclasses.
+  int index = interfaces_field_count(ik);
+  for (InstanceKlass* super_klass = ik->java_super(); super_klass != nullptr; super_klass = super_klass->java_super()) {
+    FilteredJavaFieldStream super_fld(super_klass);
+    index += super_fld.field_count();
+  }
 
-  int index = 0;
+  for (FilteredJavaFieldStream fld(ik); !fld.done(); fld.next(), index++) {
-  for (FilteredFieldStream fld(ik, true, true); !fld.eos(); fld.next(), index++) {
     // ignore instance fields
     if (!fld.access_flags().is_static()) {
       continue;
     }
-    field_map->add(max_field_index - index, fld.signature()->char_at(0), fld.offset());
+    field_map->add(index, fld.signature()->char_at(0), fld.offset());
   }
+
   return field_map;
 }
 
 // Returns a heap allocated ClassFieldMap to describe the instance fields
 // of the given class. All instance fields are included (this means public
-// and private fields declared in superclasses and superinterfaces too).
+// and private fields declared in superclasses too).
-//
 ClassFieldMap* ClassFieldMap::create_map_of_instance_fields(oop obj) {
   InstanceKlass* ik = InstanceKlass::cast(obj->klass());
 
   // create the field map
   ClassFieldMap* field_map = new ClassFieldMap();
 
-  FilteredFieldStream f(ik, false, false);
+  // fields of the superclasses are reported first, so need to know total field number to calculate field indices
+  int total_field_number = interfaces_field_count(ik);
+  for (InstanceKlass* klass = ik; klass != nullptr; klass = klass->java_super()) {
+    FilteredJavaFieldStream fld(klass);
+    total_field_number += fld.field_count();
+  }
 
-  int max_field_index = f.field_count()-1;
+  for (InstanceKlass* klass = ik; klass != nullptr; klass = klass->java_super()) {
-
+    FilteredJavaFieldStream fld(klass);
-  int index = 0;
+    int start_index = total_field_number - fld.field_count();
-  for (FilteredFieldStream fld(ik, false, false); !fld.eos(); fld.next(), index++) {
+    for (int index = 0; !fld.done(); fld.next(), index++) {
-    // ignore static fields
+      // ignore static fields
-    if (fld.access_flags().is_static()) {
+      if (fld.access_flags().is_static()) {
-      continue;
+        continue;
+      }
+      field_map->add(start_index + index, fld.signature()->char_at(0), fld.offset());
     }
-    field_map->add(max_field_index - index, fld.signature()->char_at(0), fld.offset());
+    // update total_field_number for superclass (decrease by the field count in the current class)
+    total_field_number = start_index;
   }
 
   return field_map;

src/hotspot/share/prims/methodHandles.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2008, 2023, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2008, 2024, 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,7 +55,6 @@
 #include "runtime/jniHandles.inline.hpp"
 #include "runtime/timerTrace.hpp"
 #include "runtime/reflection.hpp"
-#include "runtime/reflectionUtils.hpp"
 #include "runtime/safepointVerifiers.hpp"
 #include "runtime/signature.hpp"
 #include "runtime/stubRoutines.hpp"

src/hotspot/share/runtime/reflection.cpp

@@ -49,7 +49,6 @@
 #include "runtime/javaCalls.hpp"
 #include "runtime/javaThread.hpp"
 #include "runtime/reflection.hpp"
-#include "runtime/reflectionUtils.hpp"
 #include "runtime/signature.hpp"
 #include "runtime/vframe.inline.hpp"
 #include "utilities/formatBuffer.hpp"

src/hotspot/share/runtime/reflection.hpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1997, 2019, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1997, 2024, 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
@@ -40,8 +40,6 @@
 // rewritten using bytecodes; if it were, most of the rest of this
 // class could go away, as well as a few more entry points in jvm.cpp.
 
-class FieldStream;
-
 class Reflection: public AllStatic {
  public:
   // Constants defined by java reflection api classes

src/hotspot/share/runtime/reflectionUtils.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1999, 2023, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1999, 2024, 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
@@ -25,51 +25,9 @@
 #include "precompiled.hpp"
 #include "classfile/javaClasses.hpp"
 #include "classfile/vmClasses.hpp"
-#include "memory/universe.hpp"
 #include "oops/instanceKlass.inline.hpp"
 #include "runtime/reflectionUtils.hpp"
 
-KlassStream::KlassStream(InstanceKlass* klass, bool local_only,
-                         bool classes_only, bool walk_defaults) {
-  _klass = _base_klass = klass;
-  _base_class_search_defaults = false;
-  _defaults_checked = false;
-  if (classes_only) {
-    _interfaces = Universe::the_empty_instance_klass_array();
-  } else {
-    _interfaces = klass->transitive_interfaces();
-  }
-  _interface_index = _interfaces->length();
-  _local_only = local_only;
-  _classes_only = classes_only;
-  _walk_defaults = walk_defaults;
-}
-
-bool KlassStream::eos() {
-  if (index() >= 0) return false;
-  if (_local_only) return true;
-  if (!_klass->is_interface() && _klass->super() != nullptr) {
-    // go up superclass chain (not for interfaces)
-    _klass = _klass->java_super();
-  // Next for method walks, walk default methods
-  } else if (_walk_defaults && (_defaults_checked == false)  && (_base_klass->default_methods() != nullptr)) {
-      _base_class_search_defaults = true;
-      _klass = _base_klass;
-      _defaults_checked = true;
-  } else {
-    // Next walk transitive interfaces
-    if (_interface_index > 0) {
-      _klass = _interfaces->at(--_interface_index);
-    } else {
-      return true;
-    }
-  }
-  _index = length();
-  next();
-  return eos();
-}
-
-int FieldStream::length() { return _klass->java_fields_count(); }
 
 GrowableArray<FilteredField*> *FilteredFieldsMap::_filtered_fields =
   new (mtServiceability) GrowableArray<FilteredField*>(3, mtServiceability);
@@ -79,11 +37,3 @@ void FilteredFieldsMap::initialize() {
   int offset = reflect_ConstantPool::oop_offset();
   _filtered_fields->append(new FilteredField(vmClasses::reflect_ConstantPool_klass(), offset));
 }
-
-int FilteredFieldStream::field_count() {
-  int numflds = 0;
-  for (;!eos(); next()) {
-    numflds++;
-  }
-  return numflds;
-}

src/hotspot/share/runtime/reflectionUtils.hpp

@@ -28,136 +28,11 @@
 #include "memory/allStatic.hpp"
 #include "oops/fieldStreams.inline.hpp"
 #include "oops/instanceKlass.hpp"
-#include "oops/objArrayOop.hpp"
 #include "oops/oopsHierarchy.hpp"
-#include "runtime/handles.hpp"
 #include "runtime/reflection.hpp"
-#include "utilities/accessFlags.hpp"
 #include "utilities/globalDefinitions.hpp"
 #include "utilities/growableArray.hpp"
 
-// A KlassStream is an abstract stream for streaming over self, superclasses
-// and (super)interfaces. Streaming is done in reverse order (subclasses first,
-// interfaces last).
-//
-//    for (KlassStream st(k, false, false, false); !st.eos(); st.next()) {
-//      Klass* k = st.klass();
-//      ...
-//    }
-
-class KlassStream {
- protected:
-  InstanceKlass*      _klass;           // current klass/interface iterated over
-  InstanceKlass*      _base_klass;      // initial klass/interface to iterate over
-  Array<InstanceKlass*>*_interfaces;    // transitive interfaces for initial class
-  int                 _interface_index; // current interface being processed
-  bool                _local_only;      // process initial class/interface only
-  bool                _classes_only;    // process classes only (no interfaces)
-  bool                _walk_defaults;   // process default methods
-  bool                _base_class_search_defaults; // time to process default methods
-  bool                _defaults_checked; // already checked for default methods
-  int                 _index;
-
-  virtual int length() = 0;
-
- public:
-  // constructor
-  KlassStream(InstanceKlass* klass, bool local_only, bool classes_only, bool walk_defaults);
-
-  // testing
-  bool eos();
-
-  // iterating
-  virtual void next() = 0;
-
-  // accessors
-  int index() const                 { return _index; }
-  bool base_class_search_defaults() const { return _base_class_search_defaults; }
-  void base_class_search_defaults(bool b) { _base_class_search_defaults = b; }
-};
-
-
-// A MethodStream streams over all methods in a class, superclasses and (super)interfaces.
-// Streaming is done in reverse order (subclasses first, methods in reverse order)
-// Usage:
-//
-//    for (MethodStream st(k, false, false); !st.eos(); st.next()) {
-//      Method* m = st.method();
-//      ...
-//    }
-
-class MethodStream : public KlassStream {
- private:
-  int length()                    { return methods()->length(); }
-  Array<Method*>* methods() {
-    if (base_class_search_defaults()) {
-      base_class_search_defaults(false);
-      return _klass->default_methods();
-    } else {
-      return _klass->methods();
-    }
-  }
- public:
-  MethodStream(InstanceKlass* klass, bool local_only, bool classes_only)
-    : KlassStream(klass, local_only, classes_only, true) {
-    _index = length();
-    next();
-  }
-
-  void next() { _index--; }
-  Method* method() { return methods()->at(index()); }
-};
-
-
-// A FieldStream streams over all fields in a class, superclasses and (super)interfaces.
-// Streaming is done in reverse order (subclasses first, fields in reverse order)
-// Usage:
-//
-//    for (FieldStream st(k, false, false); !st.eos(); st.next()) {
-//      Symbol* field_name = st.name();
-//      ...
-//    }
-
-
-class FieldStream : public KlassStream {
- private:
-  int length();
-
-  fieldDescriptor _fd_buf;
-
- public:
-  FieldStream(InstanceKlass* klass, bool local_only, bool classes_only)
-    : KlassStream(klass, local_only, classes_only, false) {
-    _index = length();
-    next();
-  }
-
-  void next() { _index -= 1; }
-
-  // Accessors for current field
-  AccessFlags access_flags() const {
-    AccessFlags flags;
-    flags.set_flags(_klass->field_access_flags(_index));
-    return flags;
-  }
-  Symbol* name() const {
-    return _klass->field_name(_index);
-  }
-  Symbol* signature() const {
-    return _klass->field_signature(_index);
-  }
-  // missing: initval()
-  int offset() const {
-    return _klass->field_offset( index() );
-  }
-  // bridge to a heavier API:
-  fieldDescriptor& field_descriptor() const {
-    fieldDescriptor& field = const_cast<fieldDescriptor&>(_fd_buf);
-    field.reinitialize(_klass, _index);
-    return field;
-  }
-};
-
 class FilteredField : public CHeapObj<mtInternal>  {
  private:
   Klass* _klass;
@@ -199,45 +74,6 @@ class FilteredFieldsMap : AllStatic {
   }
 };
 
-
-// A FilteredFieldStream streams over all fields in a class, superclasses and
-// (super)interfaces. Streaming is done in reverse order (subclasses first,
-// fields in reverse order)
-//
-// Usage:
-//
-//    for (FilteredFieldStream st(k, false, false); !st.eos(); st.next()) {
-//      Symbol* field_name = st.name();
-//      ...
-//    }
-
-class FilteredFieldStream : public FieldStream {
- private:
-  int  _filtered_fields_count;
-  bool has_filtered_field() { return (_filtered_fields_count > 0); }
-  void skip_filtered_fields() {
-    if (has_filtered_field()) {
-      while (_index >= 0 && FilteredFieldsMap::is_filtered_field((Klass*)_klass, offset())) {
-        _index -= 1;
-      }
-    }
-  }
-
- public:
-  FilteredFieldStream(InstanceKlass* klass, bool local_only, bool classes_only)
-    : FieldStream(klass, local_only, classes_only) {
-    _filtered_fields_count = FilteredFieldsMap::filtered_fields_count(klass, local_only);
-    // skip filtered fields at the end
-    skip_filtered_fields();
-
-  }
-  int field_count();
-  void next() {
-    _index -= 1;
-    skip_filtered_fields();
-  }
-};
-
 // Iterate over Java fields filtering fields like reflection does.
 class FilteredJavaFieldStream : public JavaFieldStream {
 private: