RFR: 8035150 ShouldNotReachHere() in ConstantPool::copy_entry_to

[Staffan Larsen]

Thanks Dan and Markus!

On 26 feb 2014, at 15:28, Daniel D. Daugherty <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> 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> 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 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/
>>>>>> 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/serviceability-dev/attachments/20140226/6df7515d/attachment.html