RFR: 8240530: CheckUnhandledOops breaks BacktraceBuilder::set_has_hidden_top_frame

[coleen.phillimore at oracle.com]

Hi,

I don't like the change to void* for _has_hidden_top_frame.

Can you change this line to

     if (_has_hidden_top_frame == _methods) {

instead?

Can you add an 'e' to Therefor

       // It would be nice to add java/lang/Boolean::TRUE here
       // to indicate that this backtrace has a hidden top frame.
       // But this code is used before TRUE is allocated.
       // Therefor let's just use an arbitrary legal oop
       // available right here. We only test for != null
       // anyways. _methods is a short[].

The reason that it's an oop is because the _backtrace is an objArrayOop 
of java.lang.Object[6], and hidden boolean is Object[5].  It's not a 
nice data structure but it needs to be fast.

Coleen


On 3/4/20 9:48 AM, Stefan Karlsson wrote:
> Hi all,
>
> Please review this patch to get rid of a unhandled oop null check in 
> the BacktraceBuilder.
>
> https://cr.openjdk.java.net/~stefank/8240530/webrev.01/
> https://bugs.openjdk.java.net/browse/JDK-8240530
>
> The patch changes the type from oop to void* to prevent NULL oops from 
> being overwritten with non-NULL values.
>
> I think an alternative fix would be to change the type of 
> _has_hidden_top_frame to bool. Maybe someone familiar with this code 
> knows why this was encoded with an oop NULL check instead of a plain 
> bool?
>
> Testing with tier1-3 with CheckUnhandledOops on by default.
>
> StefanK