RFR (XS) 8161280 - assert failed: reference count underflow for symbol

[Kim Barrett]

> On Aug 24, 2016, at 5:50 PM, David Holmes <david.holmes at oracle.com> wrote:
> 
> Hi Kim,
> 
> On 25/08/2016 7:46 AM, Kim Barrett wrote:
>>> On Aug 24, 2016, at 3:01 AM, Ioi Lam <ioi.lam at oracle.com> wrote:
>>> 
>>> Hi David,
>>> 
>>> Here's an updated version that added Atomic::add(jshort*, jshort) as you suggested.
>>> 
>>> To appease the "unused" warnings, I just added (void)new_value.
>>> 
>>> http://cr.openjdk.java.net/~iklam/jdk9/8161280-symbol-refcount-underflow.v03/
>>> 
>>> I am running RBT with "--test hotspot/test/:hotspot_all,vm.parallel_class_loading,vm.runtime.testlist" to make sure everything works.
>> 
>> ------------------------------------------------------------------------------
>> src/share/vm/runtime/atomic.hpp
>> 211   jint new_value = Atomic::add(add_value << 16, (volatile jint*)(dest-1));
>> 214   jint new_value = Atomic::add(add_value << 16, (volatile jint*)(dest));
>> 
>> Left-shift of a signed negative value is undefined behavior.
> 
> Okay so how do we fix that? It seems pretty obvious/simple what we want to do. Do we just cast to unsigned, shift and cast back?
> 
>> ------------------------------------------------------------------------------
>> src/share/vm/runtime/atomic.hpp
>> 216   return (jshort)(new_value >> 16); // preserves sign
>> 
>> Right-shift of a signed negative value is implementation-defined. It
>> may or may not sign-extend. (gcc defines it as sign-extending; I have
>> no idea about other compilers.)
> 
> Ditto.

Unfortunately, I don't have a particularly good answer.  This is a
rather ugly corner of C/C++.

One option might be to change the increment and decrement operations
to use unsigned arithmetic with unsigned range checks that correspond
to the ranges of interest. This would need Atomic::add for jushort
instead. This should dodge all the signed arithmetic issues. To help
with this we might extract into a helper the "safe" unsigned to signed
conversion support from JAVA_INTEGER_OP. This is clumsy and somewhat
obfuscated.

Alternatively, we can try other mechanisms for working around the
signed arithmetic issues. For left-shift, the simplest solution might
be to add JAVA_INTEGER_OP for << to the block of such near the end of
globalDefinitions.hpp, e.g.

JAVA_INTEGER_OP(<<, java_shift_left, jint, juint) 
JAVA_INTEGER_OP(<<, java_shift_left, jlong, julong) 

and update the comments describing these operations, since these don't
wrap, they just silently discard overflow. And that isn't a constant
expression (until we can use C++11 constexpr (might require C++14
constexpr)), which limits where we can use it.

That's not entirely nice, since it expands the usage of these
operations from "emulate Java operations" to more generally working
around the specification of C/C++ arithmetic.

Right-shift doesn't have that option. I don't know of a portable and
reliably efficient way to do a sign-extending right shift. "Hacker's
Delight" provides several formulas for it, but they all take 5-6
instructions. I wasn't able to provoke gcc into recognizing any of
them and generating the desired single instruction. Other compilers
might do better.

A pragmatic answer might be to just assume all the platforms we care
about are sign-extending (which is likely what we've been doing all
along), and add a little startup test to verify that assumption. If
the test trips, then figure out what to do.