RFR: 8144953: runtime/CommandLine/TraceExceptionsTest.java fails when exception is thrown in compiled code

Rachel Protacio rachel.protacio at oracle.com
Fri Jan 8 17:33:09 UTC 2016


Thanks for the review, Coleen!
Rachel

On 1/8/2016 12:30 PM, Coleen Phillimore wrote:
>
> It looks great!
> thanks,
> Coleen
>
> On 1/8/16 11:50 AM, Rachel Protacio wrote:
>> Cool. I've fixed the issue: 
>> http://cr.openjdk.java.net/~rprotacio/8144953.02/
>>
>> Thanks!
>> Rachel
>>
>> On 1/7/2016 5:27 PM, Coleen Phillimore wrote:
>>>
>>> Hi, I really like this except the second argument to log_exception 
>>> isn't needed.   It's just
>>>
>>> oop fmt = exception();   Or you can pass
>>>
>>> *+ message->as_C_string(), p2i(exception()), tempst.as_string());*
>>>
>>>
>>> See src/share/vm/runtime/handles.hpp for Handle - operator() turns 
>>> into an oop.
>>>
>>> But this looks really good.  If it was like this, I wouldn't have 
>>> introduced the bug in the first place.
>>>
>>> Thanks,
>>> Coleen
>>>
>>> On 1/7/16 3:55 PM, Rachel Protacio wrote:
>>>> Hi,
>>>>
>>>> I've refactored as suggested: 
>>>> http://cr.openjdk.java.net/~rprotacio/8144953.01/
>>>>
>>>> TraceExceptionsTest.java still passes, both when run regularly and 
>>>> when run with compiled code.
>>>>
>>>> Thanks,
>>>> Rachel
>>>>
>>>> On 1/6/2016 3:35 PM, Coleen Phillimore wrote:
>>>>>
>>>>>
>>>>> On 1/6/16 3:30 PM, Coleen Phillimore wrote:
>>>>>>
>>>>>> Yes, I missed the one in the compiler file.  I would really like 
>>>>>> if there was only one function for this logging though, but am 
>>>>>> trying to think of the best place for it.
>>>>>
>>>>> So can this be consolidated into Exceptions::log_exception() in 
>>>>> src/share/vm/utilities/exceptions.hpp/cpp ?
>>>>>
>>>>> Thanks,
>>>>> Coleen
>>>>>
>>>>>>
>>>>>> Coleen
>>>>>>
>>>>>>
>>>>>> On 1/6/16 2:47 PM, Rachel Protacio wrote:
>>>>>>> Thanks for the review, David. My impression was that no one had 
>>>>>>> realized that the detailed message was needed anywhere else in 
>>>>>>> that original 8048933, so this is just a matter of thoroughness, 
>>>>>>> but maybe Coleen can speak to that?
>>>>>>>
>>>>>>> Rachel
>>>>>>>
>>>>>>> On 1/6/2016 12:42 AM, David Holmes wrote:
>>>>>>>> Hi Rachel,
>>>>>>>>
>>>>>>>> On 6/01/2016 3:42 AM, Rachel Protacio wrote:
>>>>>>>>> Hi,
>>>>>>>>>
>>>>>>>>> Please review this fix allowing TraceExceptionTest.java to 
>>>>>>>>> pass with
>>>>>>>>> compiled code. c1_Runtime1.cpp had been missing the necessary 
>>>>>>>>> long-form
>>>>>>>>> message, which has been added, and at the same time I discovered
>>>>>>>>> bytecodeInterpreter.cpp was similarly lacking, though no test 
>>>>>>>>> had turned
>>>>>>>>> it up.
>>>>>>>>
>>>>>>>> So really this is extending the change introduced by:
>>>>>>>>
>>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8048933
>>>>>>>>
>>>>>>>> which begs the question as to why that change was only made for 
>>>>>>>> interpreted code in the first place?
>>>>>>>>
>>>>>>>> Changes look okay but I can't vouch for the validity of those 
>>>>>>>> calls in that particular context.
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> David
>>>>>>>>
>>>>>>>>> Passes JPRT and RBT tests.
>>>>>>>>>
>>>>>>>>> Open webrev: http://cr.openjdk.java.net/~rprotacio/8144953/
>>>>>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8144953
>>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>> Rachel
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>



More information about the hotspot-dev mailing list