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

[Coleen Phillimore]

Looks good to me.
Thanks,
Coleen

On 12/10/2013 01:58 PM, David Chase wrote:
> revised webrev: http://cr.openjdk.java.net/~drchase/8029351/webrev.02/
>
> tested with jtreg and ute-vm.quick.testlist and 1000 repetitions of defmeth.
>
> Turned out that there was no need to add a new method, there was ALREADY
> a method that did almost what we want, and I added asserts at the use site
> to ensure that the allegedly irrelevant difference was actually irrelevant.
> (And it should have caused catastrophe before anyway if it was not irrelevant.)
>
> David
>
> On 2013-12-09, at 12:58 PM, David Chase <david.r.chase at oracle.com> wrote:
>
>> On 2013-12-09, at 11:28 AM, Coleen Phillimore <coleen.phillimore at oracle.com> wrote:
>>>> So is including them ALSO safe, since they should never occur?
>>>> I'd prefer to use an existing "are you an object" code rather than inventing another one.
>>>> I could also insert asserts that those two cases are not leaking through.
>>> Yes, these two tags are also safe and true.   They might be expected in other call sites (now or in the future), so don't add asserts.
>> Ugh, before I saw your reply, I added them and have been testing.
>>
>> Where I put the asserts, if the _index cases had leaked through in the past,
>> would have been interesting, because they would have been treated as not-objects,
>> which I think leads to downstream evil (e.g., infinite loops in the VM).
>> So for now they are there, and for now they have not caused any problems,
>> testing with defmeth, jtreg, and now I'm vm.quick.testlist.
>>
>> Do you think that's okay, or should I remove the asserts?  That would only make
>> the tests I'm running less likely to fail, and have no effect on the release build anyway.
>>
>> David
>>