RFR (M) 8207359: Make SymbolTable increment_refcount disallow zero
Kim Barrett
kim.barrett at oracle.com
Wed Jul 18 22:14:14 UTC 2018
> On Jul 17, 2018, at 11:51 AM, coleen.phillimore at oracle.com wrote:
>
> Summary: Use cmpxchg for non permanent symbol refcounting, and pack refcount and length into an int.
>
> This is a precurser change to the concurrent SymbolTable change. Zeroed refcounted entries can be deleted at anytime so they cannot be allowed to be zero in runtime code. Thanks to Kim for writing the packing function and helping me avoid undefined behavior.
>
> open webrev at http://cr.openjdk.java.net/~coleenp/8207359.01/webrev
> bug link https://bugs.openjdk.java.net/browse/JDK-8207359
>
> Tested with solaris ptrace helper, mach5 tier1-5 including solaris. Added multithreaded gtest which exercises the code.
>
> Thanks,
> Coleen
------------------------------------------------------------------------------
src/hotspot/os/solaris/dtrace/jhelper.d
467 OFFSET_Symbol_length_and_refcount);
474 OFFSET_Symbol_length_and_refcount);
483 OFFSET_Symbol_length_and_refcount);
I think these only work on a big-endian platform, and are making
further assumptions about the Symbol implementation. I have no idea
what, if anything, to do about that here. At the very least, some
comments seem warranted.
Similarly in java.base/solaris/native/libjvm_db/libjvm_db.c, though
some comments were provided there.
------------------------------------------------------------------------------
src/hotspot/share/oops/symbol.cpp
233 void Symbol::decrement_refcount() {
234 if (refcount() != PERM_REFCOUNT) { // not a permanent symbol
235 int new_value = Atomic::sub((uint32_t)1, &_length_and_refcount);
This isn't safe, and can lose counts. Consider refcount is currently
PERM-1. Decrement checks and finds it's not PERM. Then three other
threads add references, one setting to PERM and two others leaving it
at PERM. Now decrement does the sub back to PERM-1, discarding two
valid references.
------------------------------------------------------------------------------
src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/oops/Symbol.java
78 public long getLength() {
79 int i = (int)length.getValue(this.addr);
80 return (i >> 16);
81 }
Consider a symbol whose length is in the range [2^15, 2^16). The cast
result is implementation defined. The right shift of a negative value
is implemention defined, and might (or might not) return a negative value.
------------------------------------------------------------------------------
test/hotspot/gtest/classfile/test_symbolTable.cpp
79 // Test overflowing refcount making symbol permanent
80 Symbol* bigsym = SymbolTable::new_symbol("bigsym", CATCH);
Add another check after that a PERM_REFCOUNT value is sticky against
decrement.
------------------------------------------------------------------------------
test/hotspot/gtest/threadHelper.inline.hpp
24 #ifndef GTEST_THREAD_HELPER_INLINE_HPP
s/THREAD_HELPER/THREADHELPER/ to match the usual Hotspot naming.
------------------------------------------------------------------------------
More information about the hotspot-runtime-dev
mailing list