RFR JDK-8193325: StackFrameInfo::getByteCodeIndex returns wrong value if bci > 32767

coleen.phillimore at oracle.com coleen.phillimore at oracle.com
Tue Aug 13 23:56:02 UTC 2019


Hi, I saw my name in this thread and had a discussion with Mandy. I 
don't like that the VM and JDK having this special coordinated dance of 
+1/-1, and the reason for this is to differentiate the value of 0 in 
StackFrame meaning either uninitialized or invalid.  If through some 
race, an unitialized value is observed, then the MemberName may not be 
filled in either.  In any case zero makes sense to return as the bci 
because it'll point to the line number beginning of the method, if used 
to get the line number.

The stackwalk API doesn't return invalid bci because it adjusts it:

int compiledVFrame::bci() const {
   int raw = raw_bci();
   return raw == SynchronizationEntryBCI ? 0 : raw;
}

At any rate, the 04 webrev looks good minus the +1/-1 dance and 
StackWalker.java comment.  Coupling the jdk and jvm like this feels like 
it's asking for bugs.  I like the simpler implementation that fixes the 
bug that we have.

Thanks,
Coleen


On 8/13/19 1:49 PM, Mandy Chung wrote:
>
>
> On 8/13/19 9:30 AM, Peter Levart wrote:
>> Usually the StackFrameInfo(s) are consumed as soon as they are 
>> returned from StackWalker API and never assigned to @Stable field. So 
>> there's no purpose of @Stable for bci field use. Except 
>> documentation. But documentation can be specified in the form of 
>> comments too.
>>
>
> There are several JDK classes whose fields are filled by VM and 
> comment is the form of documentation.  Until new use of StackFrame in 
> the future shows performance benefit, it's okay to stick with @Stable 
> original intent and keep the comment:
>
> +    private int bci;                    // zero and negative values 
> represent invalid BCI
>
> Please also review CSR:
>    https://bugs.openjdk.java.net/browse/JDK-8229487
>
> Mandy



More information about the core-libs-dev mailing list