RFR (S) 8216308: StackTraceElement::fill_in can use injected Class source-file

[coleen.phillimore at oracle.com]

On 1/14/19 8:23 AM, coleen.phillimore at oracle.com wrote:
>
>
> On 1/14/19 8:06 AM, David Holmes wrote:
>> On 14/01/2019 10:32 pm, Aleksey Shipilev wrote:
>>> On 1/12/19 1:43 AM, David Holmes wrote:
>>>>>> I'm also very unclear about how the redefinition case is 
>>>>>> currently handled. It seems that we will
>>>>>> normally intern NULL (and presumably get a NULL or empty-string 
>>>>>> oop?) unless ShowHiddenFrames is
>>>>>> set, in which case we use the unknown_class_name() - regardless 
>>>>>> of whether the frame is actually
>>>>>> hidden or not! This seems broken to me. (Separate bug to fix that 
>>>>>> is okay if it is indeed broken.)
>>>>>
>>>>> This looks like a bug, but I'm not sure what ShowHiddenFrames is 
>>>>> supposed to do here, or how it
>>>>> got there.  I think if Aleksey removed that with this patch it 
>>>>> would be fine with me.
>>>>
>>>> I think use of ShowHiddenFrames here is completely broken. But a 
>>>> seperate bug and some suitable
>>>> archaeology is needed to fix it the right way.
>>>
>>> Okay, are we in agreement that current patch does not break anything 
>>> new? If so, let's push the
>>
>> Okay I agree it doesn't break anything new though I'd be happier if 
>> the Backtrace::get_line_number issue was fixed. Otherwise it needs a 
>> follow up bug too - I'm starting to think it makes no sense to allow 
>> redefinition to occur within a method like this! And this is a 
>> distinct issue from ShowHiddenFrames.
>
> There's no practical way to disable redefinition here and it's not 
> something that happens.  You can file a bug for it if you like, but 
> it's not something worth fixing.

Looking again at the original code.  If you have a redefinition at 
StringTable::intern() the line number from the methodHandle (Method*) is 
correct.  In this case it's the "old" method.  It's where the original 
method had the exception, which is what you want to print.

Coleen

>
> thanks,
> Coleen
>
>>
>> David
>> -----
>>
>>
>>> current patch in its current form, and then follow up on 
>>> ShowHiddenFrames in a separate issue. This
>>> would also make current patch simply backportable to 11.
>>>
>>> Current patch (no changes since last time):
>>>    http://cr.openjdk.java.net/~shade/8216308/webrev.01/
>>>
>>> -Aleksey
>>>
>