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