From fd3042a04b2d76180cb90f688e8b33156fdf3d18 Mon Sep 17 00:00:00 2001 From: Alex Menkov Date: Mon, 5 Feb 2024 21:55:13 +0000 Subject: [PATCH] 8318566: Heap walking functions should not use FilteredFieldStream Reviewed-by: cjplummer, sspitsyn --- src/hotspot/share/ci/ciField.cpp | 4 +- src/hotspot/share/jvmci/jvmciCompilerToVM.cpp | 2 +- src/hotspot/share/jvmci/jvmciRuntime.cpp | 4 +- src/hotspot/share/oops/instanceKlass.cpp | 2 +- src/hotspot/share/prims/jvmtiTagMap.cpp | 60 +++++-- src/hotspot/share/prims/methodHandles.cpp | 3 +- src/hotspot/share/runtime/reflection.cpp | 1 - src/hotspot/share/runtime/reflection.hpp | 4 +- src/hotspot/share/runtime/reflectionUtils.cpp | 52 +----- src/hotspot/share/runtime/reflectionUtils.hpp | 164 ------------------ 10 files changed, 51 insertions(+), 245 deletions(-) diff --git a/src/hotspot/share/ci/ciField.cpp b/src/hotspot/share/ci/ciField.cpp index 0d4487fdf2f..0eddd87200a 100644 --- a/src/hotspot/share/ci/ciField.cpp +++ b/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 // diff --git a/src/hotspot/share/jvmci/jvmciCompilerToVM.cpp b/src/hotspot/share/jvmci/jvmciCompilerToVM.cpp index 74b17e07cae..98b45a0b1dd 100644 --- a/src/hotspot/share/jvmci/jvmciCompilerToVM.cpp +++ b/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" diff --git a/src/hotspot/share/jvmci/jvmciRuntime.cpp b/src/hotspot/share/jvmci/jvmciRuntime.cpp index 5e9999f2733..3a19745fe09 100644 --- a/src/hotspot/share/jvmci/jvmciRuntime.cpp +++ b/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 diff --git a/src/hotspot/share/oops/instanceKlass.cpp b/src/hotspot/share/oops/instanceKlass.cpp index 8026d10dc12..e236e583d24 100644 --- a/src/hotspot/share/oops/instanceKlass.cpp +++ b/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" diff --git a/src/hotspot/share/prims/jvmtiTagMap.cpp b/src/hotspot/share/prims/jvmtiTagMap.cpp index 77bbbbfa77c..bc91c105073 100644 --- a/src/hotspot/share/prims/jvmtiTagMap.cpp +++ b/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 { // 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* 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); - int max_field_index = f.field_count()-1; + // Static fields of interfaces and superclasses are reported as references from the interfaces/superclasses. + // 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 (FilteredFieldStream fld(ik, true, true); !fld.eos(); fld.next(), index++) { + for (FilteredJavaFieldStream fld(ik); !fld.done(); 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; - - int index = 0; - for (FilteredFieldStream fld(ik, false, false); !fld.eos(); fld.next(), index++) { - // ignore static fields - if (fld.access_flags().is_static()) { - continue; + for (InstanceKlass* klass = ik; klass != nullptr; klass = klass->java_super()) { + FilteredJavaFieldStream fld(klass); + int start_index = total_field_number - fld.field_count(); + for (int index = 0; !fld.done(); fld.next(), index++) { + // ignore static fields + if (fld.access_flags().is_static()) { + 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; diff --git a/src/hotspot/share/prims/methodHandles.cpp b/src/hotspot/share/prims/methodHandles.cpp index e1f1977b67e..3ffe4853b8c 100644 --- a/src/hotspot/share/prims/methodHandles.cpp +++ b/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" diff --git a/src/hotspot/share/runtime/reflection.cpp b/src/hotspot/share/runtime/reflection.cpp index f0597b00527..0a80359cfe3 100644 --- a/src/hotspot/share/runtime/reflection.cpp +++ b/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" diff --git a/src/hotspot/share/runtime/reflection.hpp b/src/hotspot/share/runtime/reflection.hpp index 91ed737ed5f..ebf44d54a6d 100644 --- a/src/hotspot/share/runtime/reflection.hpp +++ b/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 diff --git a/src/hotspot/share/runtime/reflectionUtils.cpp b/src/hotspot/share/runtime/reflectionUtils.cpp index 3518e1c050b..17335001e43 100644 --- a/src/hotspot/share/runtime/reflectionUtils.cpp +++ b/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 *FilteredFieldsMap::_filtered_fields = new (mtServiceability) GrowableArray(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; -} diff --git a/src/hotspot/share/runtime/reflectionUtils.hpp b/src/hotspot/share/runtime/reflectionUtils.hpp index 5d43d27f660..5753f46055e 100644 --- a/src/hotspot/share/runtime/reflectionUtils.hpp +++ b/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*_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* 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(_fd_buf); - field.reinitialize(_klass, _index); - return field; - } -}; - class FilteredField : public CHeapObj { 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: