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

[David Holmes]

On 14/01/2019 11:49 pm, coleen.phillimore at oracle.com wrote:
> 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.

Then why do we have the -1 handling for the initial redefinition case? 
It doesn't make sense to me that the logic is basically:

if method was redefined
   set line number = -1

< do other stuff that might lead to method redefinition >

set line number with value read from possibly redefined method

---

Surely the line number has to be the same, in terms of being valid or 
not, regardless of whether the redefinition occurred before this code or 
during it?

Thanks,
David


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