RFR: 8035150 ShouldNotReachHere() in ConstantPool::copy_entry_to

[Coleen Phillimore]

I saw you checked this in.  The code looks fine.
I think UnresolvedClassInError should have been reverted to 
UnresolvedClass before copy_entry_to, just like JVM_CONSTANT_Class in 
jvmtiRedefineClasses.cpp around line 1224 and shouldn't be found in 
copy_entry_to.   It'll still work though because you won't change the 
resolution of JVM_CONSTANT_UnresolvedClassInError.
I haven't found how it got broken in jdk8 yet.

Coleen

On 2/26/2014 9:33 AM, Staffan Larsen wrote:
> Thanks Dan and Markus!
>
> On 26 feb 2014, at 15:28, Daniel D. Daugherty 
> <daniel.daugherty at oracle.com <mailto:daniel.daugherty at oracle.com>> wrote:
>
>> On 2/26/14 7:15 AM, Staffan Larsen wrote:
>>>
>>> On 26 feb 2014, at 15:03, Daniel D. Daugherty 
>>> <daniel.daugherty at oracle.com <mailto:daniel.daugherty at oracle.com>> 
>>> wrote:
>>>
>>>> On 2/26/14 1:31 AM, Staffan Larsen wrote:
>>>>> On 26 feb 2014, at 01:48, Daniel D. Daugherty 
>>>>> <daniel.daugherty at oracle.com <mailto:daniel.daugherty at oracle.com>> 
>>>>> wrote:
>>>>>
>>>>>> I concur with Markus. Pairing JVM_CONSTANT_UnresolvedClassInError 
>>>>>> with
>>>>>> JVM_CONSTANT_UnresolvedClass in the ConstantPool::copy_entry_to()
>>>>>> switch looks like the right thing to do.
>>>>> Good - thanks.
>>>>>
>>>>>> The usual questions:
>>>>>>
>>>>>> - why wasn't this failure mode seen before JDK8?
>>>>> No tests for this ? ;)
>>>>
>>>> I should have been more clear... :-) Why hasn't the NetBeans profiler
>>>> run into this before? That profiler is a wonderful test for the
>>>> RedefineClasses/RetransformClasses stuff…
>>>
>>> Ah, ok. No idea...
>>>
>>>>
>>>>
>>>>>
>>>>>> - was this failure caught somewhere else before JDK8 and changes
>>>>>>  in JDK8 exposed a new code path?
>>>>>>
>>>>>> Reasoning about this from a 30,000 foot view, I don't see any reason
>>>>>> why you can't redefine a class that has a constant pool ref that
>>>>>> refers to a class in error. You won't be able to use the error'ed
>>>>>> class, but there's no reason it can't be in there... Or does that
>>>>>> violate the rule that you can't redefine a class that isn't fully
>>>>>> linked (what ever that means...)???
>>>>>>
>>>>>> So what does your new test on JDK7 or JDK6? Just curious…
>>>>> The test passes on jdk7, but fails on jdk8. (I don’t have a jdk6). 
>>>>> I don’t know why it passes on jdk7, do you think it’s important to 
>>>>> track it down?
>>>>
>>>> The fact that it passes on JDK7 is the useful piece of data.
>>>> Figuring out why is much less important. BTW, which JDK7
>>>> version? One of the updates or GA/FCS?
>>>
>>> I used 7u45, but now I tested with 7u4 as well - passes there, too.
>>
>> Sounds like the change/breakage is limited to JDK8 so that's
>> a relief.
>>
>>
>>>
>>> Are you ok with pushing the change?
>>
>> Very much so.
>>
>> Dan
>>
>>
>>>
>>> Thanks,
>>> /Staffan
>>>
>>>>
>>>> Dan
>>>>
>>>>
>>>>>
>>>>> /Staffan
>>>>>
>>>>>> Dan
>>>>>>
>>>>>>
>>>>>> On 2/24/14 2:42 AM, Markus Gronlund wrote:
>>>>>>> Hi Staffan,
>>>>>>>
>>>>>>> I would think this is the correct fix.
>>>>>>>
>>>>>>> The other two constant pool "error" tags, besides 
>>>>>>> UnresolvedClassInError, which signal constant pool resolution 
>>>>>>> errors are MethodTypeInError and MethodHandleInError - these 
>>>>>>> error tags are associated with their corresponding "success" 
>>>>>>> tags in switch targets in ConstantPool::copy_entry_to(), as well 
>>>>>>> as in additional routines in constantPool.cpp.
>>>>>>>
>>>>>>> In addition, in other routines in ConstantPool.cpp, the error 
>>>>>>> tag JVM_CONSTANT_UnresolvedClassInError is associated with 
>>>>>>> JVM_CONSTANT_UnresolvedClass - 
>>>>>>>  ConstantPool::resolve_constant_at_impl() for example.
>>>>>>>
>>>>>>> Thanks
>>>>>>> Markus
>>>>>>>
>>>>>>>
>>>>>>> -----Original Message-----
>>>>>>> From: Staffan Larsen
>>>>>>> Sent: den 21 februari 2014 15:11
>>>>>>> To: hotspot-runtime-dev; serviceability-dev at openjdk.java.net 
>>>>>>> <mailto:serviceability-dev at openjdk.java.net> 
>>>>>>> serviceability-dev at openjdk.java.net 
>>>>>>> <mailto:serviceability-dev at openjdk.java.net>
>>>>>>> Subject: RFR: 8035150 ShouldNotReachHere() in 
>>>>>>> ConstantPool::copy_entry_to
>>>>>>>
>>>>>>> This is an attempt to solve a crash while redefining a class 
>>>>>>> that has unresolved class references in its constant pool. I 
>>>>>>> would appreciate some extra scrutiny here since I am unfamiliar 
>>>>>>> with this code path.
>>>>>>>
>>>>>>> I have also added a test that causes a JVM crash without the fix.
>>>>>>>
>>>>>>> The updates to the test library is all code copied from the jdk 
>>>>>>> version of the test library.
>>>>>>>
>>>>>>> webrev: http://cr.openjdk.java.net/~sla/8035150/webrev.00/ 
>>>>>>> <http://cr.openjdk.java.net/%7Esla/8035150/webrev.00/>
>>>>>>> bug: https://bugs.openjdk.java.net/browse/JDK-8035150
>>>>>>>
>>>>>>> Thanks,
>>>>>>> /Staffan
>>>
>>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/attachments/20140226/564b24e1/attachment.html