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

[Ioi Lam]

On 8/25/16 10:37 AM, Kim Barrett wrote:
>> On Aug 25, 2016, at 4:26 AM, Ioi Lam <ioi.lam at oracle.com> wrote:
>>
>> 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?
> Yes, that's what I expected to see.
>
>> -------------------------------
>>
>> 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:
> The suggestion of using a wrapper was a "perhaps", and not really
> intended for you to deal with while addressing the problem at hand.
> Sorry I was confusing about that.
>
> If we do something along that line (as a separate project), I suggest
> we keep with the names we've already got for similar operations,
> e.g. use java_shift_{left,right} to be consistent with java_add and
> friends.

Hi Kim,

Thanks for the clarification.

My RBT tests passed, so I will check in the code as is in my last webrev 
using the >> and << operators. I'll leave the general problem of 
java_shift_left/right as a future improvement.

Thanks
- Ioi


>> 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
>