XS RFR: 8009731: loader constraint violation error message

David Holmes david.holmes at oracle.com
Wed Mar 27 22:10:18 PDT 2013


Thanks Karen. Kind of sad the original code turned out to be so buggy. :(

I'm still not 100% sure what the relationship is between the "resolved 
klass" and the method's method_holder ??

David

On 28/03/2013 12:37 AM, Karen Kinnear wrote:
> Resending - thanks David for catching that this fix corrected the text and the overwritten string, but
> did not fix the resolved_klass to be the resolved_method->method_holder()..
>
> http://cr.openjdk.java.net/~acorn/8009731.3/webrev/
>
> Sorry - I am backporting two fixes from my lambda repo - the first to fix the error message and the second
> to fix the resolved_method for overpasses for the actual loader constraint check and I forgot that the original needed
> the resolved_method->method_holder() fixed in printing the error message (but the loader constraint check itself
> is correct today except for overpasses).
>
> The only changes from the last webrev are:
> 1) linkResolver.cpp - fix target for 2 the two pertinent error messages
> 2) systemDictionary.hpp - fix alignment as Coleen suggested
>
> thanks,
> Karen
>
> On Mar 27, 2013, at 8:16 AM, Coleen Phillimore wrote:
>
>> On 3/27/2013 8:11 AM, Karen Kinnear wrote:
>>> Thank you Dan, David, Coleen and Yumin for your quick reviews.
>>> I updated the bug to show the before and after.
>>
>> Final looks good.   In systemDictionary.hpp, do the arguments on the second line line up?   (minor nit - if you change that don't send out another review!!)
>>
>> You can have the queue first if you're ready.
>>
>> thanks,
>> Coleen
>>>
>>> thanks,
>>> Karen
>>>
>>> On Mar 26, 2013, at 9:05 PM, Daniel D. Daugherty wrote:
>>>
>>>>> http://cr.openjdk.java.net/~acorn/8009731.2/webrev/
>>>> src/share/vm/classfile/systemDictionary.cpp
>>>> src/share/vm/classfile/systemDictionary.hpp
>>>> src/share/vm/interpreter/linkResolver.cpp
>>>> src/share/vm/oops/klassVtable.cpp
>>>>     No comments on any of the above.
>>>>
>>>> Could you add a note to the bug showing the original test output
>>>> versus the revised test output? There's a lot of commentary in
>>>> the bug report and a clear statement of "what it was" versus
>>>> "what it is" will help...
>>>>
>>>> Dan
>>>>
>>>>
>>>> On 3/26/13 6:45 PM, 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?
>>>>>>
>>>>>>          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
>>>>>>>>>>
>>
>


More information about the hotspot-runtime-dev mailing list