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

[Ioi Lam]

On 8/24/16 11:52 PM, Kim Barrett wrote:
>> On Aug 24, 2016, at 8:46 PM, Ioi Lam <ioi.lam at oracle.com> wrote:
>>
>> Hi Kim,
>>
>> Thanks for pointing out the problems with the shift operators. I never knew that!
>>
>> Since I am shifting only by 16, can change the expressions to these?
>>
>>     jint(add_value) * 0x10000
> Yes.  I remembered that technique shortly after hitting send.
> Multiplying a signed value doesn't work (is UB) in the general case
> because of overflow, but we know the value ranges here are safe from
> that.
>
>>     jshort(new_value / 0x10000)
> No, because under 2s complement arithmetic, arithmetic right shift of
> a negative number is not necessarily equivalent to division by the
> corresponding power of 2.  [See, for example, "Arithmetic shifting
> considered harmful", Guy Steele, ACM SIGPLAN Notices, 11/1977.]
>
> Consider the 32bit value with all 1s in the upper 16 bits, and a
> non-zero value in the lower 16 bits.  If division is truncate, which
> it is defined to be for C99/C++11 (*), that value / 0x10000 == 0,
> rather than the desired -1.  Clear the low 16bits first and then
> divide, and I think it works for the case at hand, though I haven't
> proved it.  But pragmatically we’re probably better off assuming
> right shift works as expected, though perhaps in a wrapper to
> help indicate we’ve actually thought about the issue.
>
> (*) For C89/C++98 the rounding of division involving negative operands
> is implementation defined, perhaps in part to allow the "optimization"
> of division by a power of 2 to arithmetic right shift.
>
>> Does C/C++ preserve signs when multiplying/dividing with a positive constant?

Hi Kim,

I looked for the use of >> in our source code:

globalDefinitions.hpp:
inline jint high(jlong value)                    { return jint(value >> 
32); }

sharedRuntimeTrans.cpp:
static double __ieee754_log(double x) {
   double hfsq,f,s,z,R,w,t1,t2,dk;
   int k,hx,i,j;
   ...
   k += (hx>>20)-1023;

So maybe we already assume that >> "does the right thing" for us?

-------------------------------

Since I am doing something very specific (setting/extracting the top 16 
bits of a jint), I am a bit hesitant to add routines for general 
shifting. globalDefinitions.hpp has these:

inline int extract_low_short_from_int(jint x) {
   return x & 0xffff;
}
inline int extract_high_short_from_int(jint x) {
   return (x >> 16) & 0xffff;
}
inline int build_int_from_shorts( jushort low, jushort high ) {
   return ((int)((unsigned int)high << 16) | (unsigned int)low);
}

I am thinking of adding:

inline int extract_signed_high_short_from_int(jint x) {
   if (x >= 0) {
     return (x >> 16) & 0xffff;
   } else {
     return int((unsigned int)x >> 16) | 0xffff0000);
   }
}

inline int build_int_from_shorts(jshort low, jshort high) {
   return build_int_from_shorts(jushort(low) & 0xffff, jushort(high) & 
0xffff);
}

What do you think?
- Ioi