XS RFR: 8009731: loader constraint violation error message

[David Holmes]

Looks good to me Karen!

(Pity Coleen didn't suggest using a SymbolHandle back in 2006! ;-) )


Also see inline ...

On 27/03/2013 10:45 AM, Karen Kinnear wrote:
> Thank you Coleen and Yumin and Dan. Good catches.
>
> I did Dan's suggested changes and included systemDictionary.hpp in the following webrev:
>
> http://cr.openjdk.java.net/~acorn/8009731.2/webrev/
>
> thanks,
> Karen
>
> On Mar 26, 2013, at 6:56 PM, Daniel D. Daugherty wrote:
>
>> On 3/26/13 4:33 PM, Karen Kinnear wrote:
>>> Thank you Coleen and David.
>>>
>>> http://cr.openjdk.java.net/~acorn/8009731.1/webrev/
>>
>> Thumbs up.
>>
>> src/share/vm/classfile/systemDictionary.cpp
>>     2201       Symbol* s = sig_strm.as_symbol(CHECK_NULL);
>>     2202       Symbol*  sig  = s;
>>         I know you didn't change these lines, but:
>>
>>         Local 's' isn't used except to set 'sig'. Is there a reason
>>         for the duplicate variable?

History lesson - here is the original code:

2014       symbolOop s = sig_strm.as_symbol(CHECK_NULL);
2015       symbolHandle sig (THREAD, s);

I'm guessing the conversion to Symbol was at least semi-automated :)

Cheers,
David
------



>>
>>          Feel free to ignore the above if you're pressed for time.
>>
>> src/share/vm/interpreter/linkResolver.cpp
>>     No comments.
>>
>> src/share/vm/oops/klassVtable.cpp
>>     No comments.
>>
>> Dan
>>
>>
>>
>>>
>>> Problem was introduced with 6990754, changeset 2059.
>>>
>>> I like Coleen's solution better - so we don't run the risk of someone "fixing" the apparent need for a ResourceMark
>>> in the future. So I changed this to use a Symbol*.
>>>
>>> vm.quick.testlist rerun in progress.
>>>
>>> thanks,
>>> Karen
>>>
>>> On Mar 25, 2013, at 8:47 PM, Coleen Phillimore wrote:
>>>
>>>> On 3/25/2013 8:10 PM, David Holmes wrote:
>>>>> Hi Karen,
>>>>>
>>>>> Looks good to me.
>>>>>
>>>>> On 26/03/2013 7:08 AM, Karen Kinnear wrote:
>>>>>> Webrev: http://cr.openjdk.java.net/~acorn/8009731/webrev/
>>>>> I can't quite determine why the extra ResourceMark caused the observed symptoms. I would have expected something more dramatic if we returned a string that was already released - are we simply overwriting it with a later class name (hence the wrong name in the message)? Do we also know when this regression was introduced?
>>>> So that's why I said "Looks ok" rather than "Looks good."   I think that function should return a Symbol* because it's not obvious to the casual observer that the as_C_string() string is resource allocated and the ResourceMark destructor will reclaim the memory. I think the error message might have been either garbled or pointing to a different string.  It could have been like that for a while and nobody noticed.
>>>>
>>>> Coleen
>>>>
>>>>>> Bug: https://jbs.oracle.com/bugs/browse/JDK-8009731
>>>>> This is not visible external to Oracle.
>>>>>
>>>>> Thanks,
>>>>> David
>>>>>
>>>>>> Fix loader constraint violation error message.
>>>>>>
>>>>>> tests:
>>>>>> bug report test
>>>>>> vm.quick.testlist in parallel
>>>>>>
>>
>