XS RFR: 8009731: loader constraint violation error message

[Coleen Phillimore]

On 3/26/2013 11:11 PM, David Holmes wrote:
> Looks good to me Karen!
>
> (Pity Coleen didn't suggest using a SymbolHandle back in 2006! ;-) )

I probably should have but then it'd have to be copy-constructed back to 
the caller.
>
>
> 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 :)
>

Yes, 'sed' and I did that when symbolOops were changed to Symbol*. There 
are remnants left over that we should clean up as we go.

Thanks,
Coleen

> 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
>>>>>>>
>>>
>>