RFR (S) 8208172: SIGSEGV when owner of invokedynamic bootstrap method throws an exception - Symbol::increment_refcount()+0x0

[coleen.phillimore at oracle.com]

On 8/21/18 7:11 PM, Ioi Lam wrote:
> Hi Coleen,
>
> The fix looks good. I think it's defensive and can be easily back 
> ported independent of JDK-8209553 (ExceptionInInitializerError can 
> have a default detail message if the cause is given).

agree.
Thank you for the code review, Ioi!
Coleen
>
> Thanks
>
> - Ioi
>
>
>
> On 8/21/18 3:44 PM, coleen.phillimore at oracle.com wrote:
>>
>> Thanks, David.
>> Coleen
>>
>> On 8/20/18 10:25 PM, David Holmes wrote:
>>> On 21/08/2018 10:30 AM, coleen.phillimore at oracle.com wrote:
>>>>
>>>>
>>>> On 8/20/18 7:56 PM, David Holmes wrote:
>>>>> Hi Coleen,
>>>>>
>>>>> Looks good.
>>>>>
>>>>> On 21/08/2018 6:04 AM, coleen.phillimore at oracle.com wrote:
>>>>>> Summary: make table for resolution errors not always expect 
>>>>>> non-null message string.
>>>>>
>>>>> Thanks for fixing this. I'm unsure why we thought there had to be 
>>>>> a detail message? AFAIK there is no specified requirement for 
>>>>> this. Obviously having a detail message is the most common case.
>>>>>
>>>>> I'm also not clear why we only hit this in the indy bootstrap case?
>>>>
>>>> We always create LinkageErrors in the vm with a message, so we 
>>>> expected one to always be present.  The case where it's converted 
>>>> to ExceptionInInitializerError hadn't been seen, or expected to be 
>>>> missing this information.  It's probably possible to write a test 
>>>> to have LinkageError =>  EIIE without indy.
>>>
>>> The obvious direct example to produce EIIE from a static initializer 
>>> doesn't induce the problem. Interesting ...
>>>
>>>>>
>>>>> One minor comment:
>>>>>
>>>>> src/hotspot/share/classfile/resolutionErrors.cpp
>>>>>
>>>>> void ResolutionErrorEntry::set_message(Symbol* c) {
>>>>>     _message = c;
>>>>> +   if (_message != NULL) {
>>>>>       _message->increment_refcount();
>>>>> +   }
>>>>>   }
>>>>>
>>>>> Is there any reason not to move the assignment inside the if 
>>>>> clause as well?
>>>>>
>>>>
>>>> The C++ class isn't zero initialized, so we have to initialize the 
>>>> message to NULL.
>>>
>>> And no constructor that does the init either - okay.
>>>
>>> Thanks,
>>> David
>>>
>>>> Thanks,
>>>> Coleen
>>>>
>>>>> Thanks,
>>>>> David
>>>>>
>>>>>> Tested with hs-tier1-2, and all hotspot/jtreg/runtime tests, and 
>>>>>> test added.
>>>>>>
>>>>>> open webrev at http://cr.openjdk.java.net/~coleenp/8208172.01/webrev
>>>>>> bug link https://bugs.openjdk.java.net/browse/JDK-8208172
>>>>>>
>>>>>> Thanks,
>>>>>> Coleen
>>>>
>>
>