stan/jdk-24
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

8342577: Clean up JVMTI breakpoint support

Browse Source 
8210637: Race in JvmtiCurrentBreakpoints::get_jvmti_breakpoints

Reviewed-by: cjplummer, sspitsyn
This commit is contained in:
Alex Menkov 2024-11-05 22:39:00 +00:00
parent 69bc088774
commit 471f112bca
2 changed files with 68 additions and 325 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

191
src/hotspot/share/prims/jvmtiImpl.cpp
View File

@ -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
@ -41,6 +41,7 @@
#include "prims/jvmtiEventController.inline.hpp"
#include "prims/jvmtiImpl.hpp"
#include "prims/jvmtiRedefineClasses.hpp"
#include "runtime/atomic.hpp"
#include "runtime/continuation.hpp"
#include "runtime/deoptimization.hpp"
#include "runtime/frame.inline.hpp"
@ -89,103 +90,6 @@ JvmtiAgentThread::call_start_function() {
_start_fn(_env->jvmti_external(), jni_environment(), (void*)_start_arg);
}
//
// class GrowableCache - private methods
//
void GrowableCache::recache() {
int len = _elements->length();
FREE_C_HEAP_ARRAY(address, _cache);
_cache = NEW_C_HEAP_ARRAY(address,len+1, mtInternal);
for (int i=0; i<len; i++) {
_cache[i] = _elements->at(i)->getCacheValue();
//
// The cache entry has gone bad. Without a valid frame pointer
// value, the entry is useless so we simply delete it in product
// mode. The call to remove() will rebuild the cache again
// without the bad entry.
//
if (_cache[i] == nullptr) {
assert(false, "cannot recache null elements");
remove(i);
return;
}
}
_cache[len] = nullptr;
_listener_fun(_this_obj,_cache);
}
//
// class GrowableCache - public methods
//
GrowableCache::GrowableCache() {
_this_obj = nullptr;
_listener_fun = nullptr;
_elements = nullptr;
_cache = nullptr;
}
GrowableCache::~GrowableCache() {
clear();
delete _elements;
FREE_C_HEAP_ARRAY(address, _cache);
}
void GrowableCache::initialize(void *this_obj, void listener_fun(void *, address*) ) {
_this_obj = this_obj;
_listener_fun = listener_fun;
_elements = new (mtServiceability) GrowableArray<GrowableElement*>(5, mtServiceability);
recache();
}
// number of elements in the collection
int GrowableCache::length() {
return _elements->length();
}
// get the value of the index element in the collection
GrowableElement* GrowableCache::at(int index) {
GrowableElement *e = (GrowableElement *) _elements->at(index);
assert(e != nullptr, "e != nullptr");
return e;
}
int GrowableCache::find(const GrowableElement* e) const {
return _elements->find_if([&](const GrowableElement* other_e) { return e->equals(other_e); });
}
// append a copy of the element to the end of the collection
void GrowableCache::append(GrowableElement* e) {
GrowableElement *new_e = e->clone();
_elements->append(new_e);
recache();
}
// remove the element at index
void GrowableCache::remove (int index) {
GrowableElement *e = _elements->at(index);
assert(e != nullptr, "e != nullptr");
_elements->remove(e);
delete e;
recache();
}
// clear out all elements, release all heap space and
// let our listener know that things have changed.
void GrowableCache::clear() {
int len = _elements->length();
for (int i=0; i<len; i++) {
delete _elements->at(i);
}
_elements->clear();
recache();
}
//
// class JvmtiBreakpoint
//
@ -194,20 +98,19 @@ JvmtiBreakpoint::JvmtiBreakpoint(Method* m_method, jlocation location)
: _method(m_method), _bci((int)location) {
assert(_method != nullptr, "No method for breakpoint.");
assert(_bci >= 0, "Negative bci for breakpoint.");
oop class_holder_oop = _method->method_holder()->klass_holder();
oop class_holder_oop = _method->method_holder()->klass_holder();
_class_holder = OopHandle(JvmtiExport::jvmti_oop_storage(), class_holder_oop);
}
JvmtiBreakpoint::JvmtiBreakpoint(const JvmtiBreakpoint& bp)
: _method(bp._method), _bci(bp._bci) {
_class_holder = OopHandle(JvmtiExport::jvmti_oop_storage(), bp._class_holder.resolve());
}
JvmtiBreakpoint::~JvmtiBreakpoint() {
_class_holder.release(JvmtiExport::jvmti_oop_storage());
}
void JvmtiBreakpoint::copy(JvmtiBreakpoint& bp) {
_method = bp._method;
_bci = bp._bci;
_class_holder = OopHandle(JvmtiExport::jvmti_oop_storage(), bp._class_holder.resolve());
}
bool JvmtiBreakpoint::equals(const JvmtiBreakpoint& bp) const {
return _method == bp._method
&& _bci == bp._bci;
@ -301,8 +204,8 @@ void VM_ChangeBreakpoints::doit() {
// a JVMTI internal collection of JvmtiBreakpoint
//
JvmtiBreakpoints::JvmtiBreakpoints(void listener_fun(void *,address *)) {
_bps.initialize(this,listener_fun);
JvmtiBreakpoints::JvmtiBreakpoints()
: _elements(5, mtServiceability) {
}
JvmtiBreakpoints:: ~JvmtiBreakpoints() {}
@ -312,9 +215,9 @@ void JvmtiBreakpoints::print() {
LogTarget(Trace, jvmti) log;
LogStream log_stream(log);
int n = _bps.length();
for (int i=0; i<n; i++) {
JvmtiBreakpoint& bp = _bps.at(i);
int n = length();
for (int i = 0; i < n; i++) {
JvmtiBreakpoint& bp = at(i);
log_stream.print("%d: ", i);
bp.print_on(&log_stream);
log_stream.cr();
@ -326,9 +229,9 @@ void JvmtiBreakpoints::print() {
void JvmtiBreakpoints::set_at_safepoint(JvmtiBreakpoint& bp) {
assert(SafepointSynchronize::is_at_safepoint(), "must be at safepoint");
int i = _bps.find(bp);
int i = find(bp);
if (i == -1) {
_bps.append(bp);
append(bp);
bp.set();
}
}
@ -336,18 +239,16 @@ void JvmtiBreakpoints::set_at_safepoint(JvmtiBreakpoint& bp) {
void JvmtiBreakpoints::clear_at_safepoint(JvmtiBreakpoint& bp) {
assert(SafepointSynchronize::is_at_safepoint(), "must be at safepoint");
int i = _bps.find(bp);
int i = find(bp);
if (i != -1) {
_bps.remove(i);
remove(i);
bp.clear();
}
}
int JvmtiBreakpoints::length() { return _bps.length(); }
int JvmtiBreakpoints::set(JvmtiBreakpoint& bp) {
if ( _bps.find(bp) != -1) {
return JVMTI_ERROR_DUPLICATE;
if (find(bp) != -1) {
return JVMTI_ERROR_DUPLICATE;
}
VM_ChangeBreakpoints set_breakpoint(VM_ChangeBreakpoints::SET_BREAKPOINT, &bp);
VMThread::execute(&set_breakpoint);
@ -355,8 +256,8 @@ int JvmtiBreakpoints::set(JvmtiBreakpoint& bp) {
}
int JvmtiBreakpoints::clear(JvmtiBreakpoint& bp) {
if ( _bps.find(bp) == -1) {
return JVMTI_ERROR_NOT_FOUND;
if (find(bp) == -1) {
return JVMTI_ERROR_NOT_FOUND;
}
VM_ChangeBreakpoints clear_breakpoint(VM_ChangeBreakpoints::CLEAR_BREAKPOINT, &bp);
@ -365,27 +266,14 @@ int JvmtiBreakpoints::clear(JvmtiBreakpoint& bp) {
}
void JvmtiBreakpoints::clearall_in_class_at_safepoint(Klass* klass) {
bool changed = true;
// We are going to run thru the list of bkpts
// and delete some. This deletion probably alters
// the list in some implementation defined way such
// that when we delete entry i, the next entry might
// no longer be at i+1. To be safe, each time we delete
// an entry, we'll just start again from the beginning.
// We'll stop when we make a pass thru the whole list without
// deleting anything.
while (changed) {
int len = _bps.length();
changed = false;
for (int i = 0; i < len; i++) {
JvmtiBreakpoint& bp = _bps.at(i);
if (bp.method()->method_holder() == klass) {
bp.clear();
_bps.remove(i);
// This changed 'i' so we have to start over.
changed = true;
break;
}
assert(SafepointSynchronize::is_at_safepoint(), "must be at safepoint");
// Go backwards because this removes entries that are freed.
for (int i = length() - 1; i >= 0; i--) {
JvmtiBreakpoint& bp = at(i);
if (bp.method()->method_holder() == klass) {
bp.clear();
remove(i);
}
}
}
@ -395,25 +283,18 @@ void JvmtiBreakpoints::clearall_in_class_at_safepoint(Klass* klass) {
//
JvmtiBreakpoints *JvmtiCurrentBreakpoints::_jvmti_breakpoints = nullptr;
address * JvmtiCurrentBreakpoints::_breakpoint_list = nullptr;
JvmtiBreakpoints& JvmtiCurrentBreakpoints::get_jvmti_breakpoints() {
if (_jvmti_breakpoints != nullptr) return (*_jvmti_breakpoints);
_jvmti_breakpoints = new JvmtiBreakpoints(listener_fun);
assert(_jvmti_breakpoints != nullptr, "_jvmti_breakpoints != nullptr");
if (_jvmti_breakpoints == nullptr) {
JvmtiBreakpoints* breakpoints = new JvmtiBreakpoints();
if (!Atomic::replace_if_null(&_jvmti_breakpoints, breakpoints)) {
// already created concurently
delete breakpoints;
}
}
return (*_jvmti_breakpoints);
}
void JvmtiCurrentBreakpoints::listener_fun(void *this_obj, address *cache) {
JvmtiBreakpoints *this_jvmti = (JvmtiBreakpoints *) this_obj;
assert(this_jvmti != nullptr, "this_jvmti != nullptr");
debug_only(int n = this_jvmti->length(););
assert(cache[n] == nullptr, "cache must be null terminated");
set_breakpoint_list(cache);
}
///////////////////////////////////////////////////////////////
//

202
src/hotspot/share/prims/jvmtiImpl.hpp
View File

@ -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
@ -36,240 +36,102 @@
#include "runtime/vmOperations.hpp"
#include "utilities/ostream.hpp"
//
// Forward Declarations
//
class JvmtiBreakpoint;
class JvmtiBreakpoints;
///////////////////////////////////////////////////////////////
//
// class GrowableCache, GrowableElement
// Used by : JvmtiBreakpointCache
// Used by JVMTI methods: none directly.
//
// GrowableCache is a permanent CHeap growable array of <GrowableElement *>
//
// In addition, the GrowableCache maintains a null terminated cache array of type address
// that's created from the element array using the function:
// address GrowableElement::getCacheValue().
//
// Whenever the GrowableArray changes size, the cache array gets recomputed into a new C_HEAP allocated
// block of memory. Additionally, every time the cache changes its position in memory, the
// void (*_listener_fun)(void *this_obj, address* cache)
// gets called with the cache's new address. This gives the user of the GrowableCache a callback
// to update its pointer to the address cache.
//
class GrowableElement : public CHeapObj<mtInternal> {
public:
virtual ~GrowableElement() {}
virtual address getCacheValue() =0;
virtual bool equals(const GrowableElement* e) const =0;
virtual GrowableElement* clone() =0;
};
class GrowableCache {
private:
// Object pointer passed into cache & listener functions.
void *_this_obj;
// Array of elements in the collection
GrowableArray<GrowableElement *> *_elements;
// Parallel array of cached values
address *_cache;
// Listener for changes to the _cache field.
// Called whenever the _cache field has it's value changed
// (but NOT when cached elements are recomputed).
void (*_listener_fun)(void *, address*);
// recache all elements after size change, notify listener
void recache();
public:
GrowableCache();
~GrowableCache();
void initialize(void *this_obj, void listener_fun(void *, address*) );
// number of elements in the collection
int length();
// get the value of the index element in the collection
GrowableElement* at(int index);
// find the index of the element, -1 if it doesn't exist
int find(const GrowableElement* e) const;
// append a copy of the element to the end of the collection, notify listener
void append(GrowableElement* e);
// remove the element at index, notify listener
void remove (int index);
// clear out all elements and release all heap space, notify listener
void clear();
};
///////////////////////////////////////////////////////////////
//
// class JvmtiBreakpointCache
// Used by : JvmtiBreakpoints
// Used by JVMTI methods: none directly.
// Note : typesafe wrapper for GrowableCache of JvmtiBreakpoint
//
class JvmtiBreakpointCache : public CHeapObj<mtInternal> {
private:
GrowableCache _cache;
public:
JvmtiBreakpointCache() {}
~JvmtiBreakpointCache() {}
void initialize(void *this_obj, void listener_fun(void *, address*) ) {
_cache.initialize(this_obj, listener_fun);
}
int length() { return _cache.length(); }
JvmtiBreakpoint& at(int index) { return (JvmtiBreakpoint&) *(_cache.at(index)); }
int find(JvmtiBreakpoint& e) { return _cache.find((GrowableElement *) &e); }
void append(JvmtiBreakpoint& e) { _cache.append((GrowableElement *) &e); }
void remove (int index) { _cache.remove(index); }
};
///////////////////////////////////////////////////////////////
//
// class JvmtiBreakpoint
// Used by : JvmtiBreakpoints
// Used by JVMTI methods: SetBreakpoint, ClearBreakpoint, ClearAllBreakpoints
// Note: Extends GrowableElement for use in a GrowableCache
//
// A JvmtiBreakpoint describes a location (class, method, bci) to break at.
//
typedef void (Method::*method_action)(int _bci);
class JvmtiBreakpoint : public GrowableElement {
class JvmtiBreakpoint : public CHeapObj<mtInternal> {
private:
Method* _method;
int _bci;
OopHandle _class_holder; // keeps _method memory from being deallocated
public:
JvmtiBreakpoint() : _method(nullptr), _bci(0) {}
JvmtiBreakpoint(Method* m_method, jlocation location);
JvmtiBreakpoint(const JvmtiBreakpoint& bp);
virtual ~JvmtiBreakpoint();
bool equals(const JvmtiBreakpoint& bp) const;
void copy(JvmtiBreakpoint& bp);
address getBcp() const;
void each_method_version_do(method_action meth_act);
void set();
void clear();
void print_on(outputStream* out) const;
Method* method() { return _method; }
// GrowableElement implementation
address getCacheValue() { return getBcp(); }
bool equals(const GrowableElement* e) const { return equals((const JvmtiBreakpoint&) *e); }
GrowableElement *clone() {
JvmtiBreakpoint *bp = new JvmtiBreakpoint();
bp->copy(*this);
return bp;
}
Method* method() const { return _method; }
};
///////////////////////////////////////////////////////////////
//
// class JvmtiBreakpoints
// Used by : JvmtiCurrentBreakpoints
// Used by JVMTI methods: none directly
// Note: A Helper class
//
// JvmtiBreakpoints is a GrowableCache of JvmtiBreakpoint.
// All changes to the GrowableCache occur at a safepoint using VM_ChangeBreakpoints.
//
// Because _bps is only modified at safepoints, its possible to always use the
// cached byte code pointers from _bps without doing any synchronization (see JvmtiCurrentBreakpoints).
//
// It would be possible to make JvmtiBreakpoints a static class, but I've made it
// CHeap allocated to emphasize its similarity to JvmtiFramePops.
// Contains growable array of JvmtiBreakpoint.
// All changes to the array occur at a safepoint.
//
class JvmtiBreakpoints : public CHeapObj<mtInternal> {
private:
GrowableArray<JvmtiBreakpoint*> _elements;
JvmtiBreakpointCache _bps;
int length() { return _elements.length(); }
JvmtiBreakpoint& at(int index) { return *_elements.at(index); }
int find(JvmtiBreakpoint& e) {
return _elements.find_if([&](const JvmtiBreakpoint * other_e) { return e.equals(*other_e); });
}
void append(JvmtiBreakpoint& e) {
JvmtiBreakpoint* new_e = new JvmtiBreakpoint(e);
_elements.append(new_e);
}
void remove(int index) {
JvmtiBreakpoint* e = _elements.at(index);
assert(e != nullptr, "e != nullptr");
_elements.remove_at(index);
delete e;
}
// These should only be used by VM_ChangeBreakpoints
// to insure they only occur at safepoints.
// Todo: add checks for safepoint
friend class VM_ChangeBreakpoints;
void set_at_safepoint(JvmtiBreakpoint& bp);
void clear_at_safepoint(JvmtiBreakpoint& bp);
friend class JvmtiCurrentBreakpoints;
JvmtiBreakpoints(); // accessible only for JvmtiCurrentBreakpoints
public:
JvmtiBreakpoints(void listener_fun(void *, address *));
~JvmtiBreakpoints();
int length();
void print();
int set(JvmtiBreakpoint& bp);
int clear(JvmtiBreakpoint& bp);
// used by VM_ChangeBreakpoints
void set_at_safepoint(JvmtiBreakpoint& bp);
void clear_at_safepoint(JvmtiBreakpoint& bp);
// used by VM_RedefineClasses
void clearall_in_class_at_safepoint(Klass* klass);
};
///////////////////////////////////////////////////////////////
//
// class JvmtiCurrentBreakpoints
//
// A static wrapper class for the JvmtiBreakpoints that provides:
// 1. a fast inlined function to check if a byte code pointer is a breakpoint (is_breakpoint).
// 2. a function for lazily creating the JvmtiBreakpoints class (this is not strictly necessary,
// but I'm copying the code from JvmtiThreadState which needs to lazily initialize
// JvmtiFramePops).
// 3. An oops_do entry point for GC'ing the breakpoint array.
// A static wrapper class for the JvmtiBreakpoints that provides
// a function for lazily creating the JvmtiBreakpoints class.
//
class JvmtiCurrentBreakpoints : public AllStatic {
private:
// Current breakpoints, lazily initialized by get_jvmti_breakpoints();
static JvmtiBreakpoints *_jvmti_breakpoints;
// null terminated cache of byte-code pointers corresponding to current breakpoints.
// Updated only at safepoints (with listener_fun) when the cache is moved.
// It exists only to make is_breakpoint fast.
static address *_breakpoint_list;
static inline void set_breakpoint_list(address *breakpoint_list) { _breakpoint_list = breakpoint_list; }
// Listener for the GrowableCache in _jvmti_breakpoints, updates _breakpoint_list.
static void listener_fun(void *this_obj, address *cache);
public:
static void initialize();
static void destroy();
// lazily create _jvmti_breakpoints and _breakpoint_list
// lazily create _jvmti_breakpoints
static JvmtiBreakpoints& get_jvmti_breakpoints();
};
///////////////////////////////////////////////////////////////
//
// class VM_ChangeBreakpoints
// Used by : JvmtiBreakpoints
// Used by JVMTI methods: none directly.
// Note: A Helper class.
//
// VM_ChangeBreakpoints implements a VM_Operation for ALL modifications to the JvmtiBreakpoints class.
//