RFR(S): 8029351: assert(bt != T_OBJECT) failed: Guard is incorrect in VM:defmeth

[Coleen Phillimore]

Actually, I agree with Christian that we should not accumulate more 
technical debt for expediency.

I think tag.is_method_type() and tag.is_method_handle() should also 
check if they have the error tag instead.  The rest of the constant pool 
entry is the same for these types.   I thought there was an is_object() 
one but now I don't see it.

thanks,
Coleen

On 12/9/2013 8:54 AM, Coleen Phillimore wrote:
>
> This looks good.   I don't know why I decided that these weren't 
> needed when I went through all of these with my previous bug fix, but 
> apparently they are.
> Thanks for fixing these.
> Coleen
>
> On 12/7/2013 10:12 PM, David Chase wrote:
>> RT-dev CC'd because this was more of a runtime bug, except that 
>> involved methodHandles.
>>
>> bug: https://bugs.openjdk.java.net/browse/JDK-8029351
>> webrev: http://cr.openjdk.java.net/~drchase/8029351/webrev.01/
>>
>> Running "defmeth", especially with -Xcomp and -XX:+AggressiveOpts, 
>> sometimes (1 in 25 to 1 in 50)
>> causes a crash in fastdebug, that in release builds is instead a 
>> signal-ignoring infinite loop.
>>
>> The cause was failure to include the method_handle_in_error and 
>> method_type_in_error
>> cases in the "is this an object?" test.  This immediately led to a 
>> blown assertion in a debug build.
>> Fix was to add the obvious clauses to the if:
>>
>>         tag.is_unresolved_klass() ||
>>         tag.is_string() ||
>>         tag.is_method_handle() ||
>> -      tag.is_method_type()) {
>> +      tag.is_method_type() ||
>> +      tag.is_method_handle_in_error() ||
>> +      tag.is_method_type_in_error()) {
>>       assert(bt == T_OBJECT, "Guard is incorrect");
>>       cts = CellTypeState::make_line_ref(bci);
>>     } else {
>>
>> Testing:
>> defmeth, fastdebug build with bug-provoking flags , 1000 times.
>> All ran w/o crash (i.e., only with other existing known failures), 
>> all produced the same output.
>>
>> David
>>
>