RFR: 8278871: [JVMCI] assert((uint)reason < 2* _trap_hist_limit) failed: oob

[Dean Long]

On Wed, 15 Dec 2021 23:11:58 GMT, Doug Simon <dnsimon at openjdk.org> wrote:

> This PR fixes a discrepancy between `MethodData::_trap_hist_list` and `Deoptimization::Reason_LIMIT` in terms of JVMCI specific usage.
> 
> JVMCI doubles the size of the deopt history for a method so that it can distinguish deopts in a normal method compilation from deopts in an OSR compilation:
> 
>     union {
>       intptr_t _align;
>       u1 _array[JVMCI_ONLY(2 *) MethodData::_trap_hist_limit];
>     } _trap_hist;
> 
> 
> To access the history for OSR deopts, the index for a deopt reason needs to be adjusted to the upper half of the history array. The amount used for the adjustment was incorrect and this PR fixes it:
> 
> 
>   if (update_total_trap_count) {
>     uint idx = reason;
> #if INCLUDE_JVMCI
>     if (is_osr) {
>       idx += Reason_TRAP_HISTORY_LENGTH;
>     }
> #endif
> 
> 
> I introduced `Reason_TRAP_HISTORY_LENGTH` as a replacement for `25 JVMCI_ONLY(+5),  // decoupled from Deoptimization::Reason_LIMIT` as this decoupling is unnecessary (as dangerous) as far as I can see.

Suggest change: use ARRAY_SIZE()

It seems a little fragile for asserts checking the array access to know the details of the array size computation (in case it changes in the future):

1999        assert((uint)reason < JVMCI_ONLY(2*) _trap_hist_limit, "oob");

How about using a constant/enum, or better yet, ARRAY_SIZE(_trap_hist._array) instead?

-------------

Changes requested by dlong (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/6855