RFR (S) 8211926: Catastrophic size_t underflow in BitMap::*_large methods
Aleksey Shipilev
shade at redhat.com
Thu Nov 8 09:38:15 UTC 2018
Hi!
On 11/06/2018 01:07 AM, Kim Barrett wrote:
> ------------------------------------------------------------------------------
> src/hotspot/share/utilities/bitMap.cpp
> 272 if ((end_full_word < beg_full_word) || (end_full_word - beg_full_word < BitsPerWord)) {
>
> The use of BitsPerWord here at on line 289 seems like a units error.
> What does the number of bits in a word have to do with the number of
> words that is considered "small"? I suspect the old value of 32 was
> just a heuristic bound (which may or may not have had any measurements
> to back it up).
>
> Of course, the assertion error message in the old version of
> set_large_range is also confused; the comparison is in words but the
> message says bytes.
Oh. Good catch! I thought the comparison was on bit index aligned on word boundary, and suspected 32
meant "32 bits" from the old era. On a hunch, I replaced it with the actual platform-specific
constant, BitsPerWord in this case.
But this is not a good thing here. Instead, we should make it cleaner by introducing "small range"
constant and turning the following comment into assert:
http://cr.openjdk.java.net/~shade/8211926/webrev.02/
This still passes new gtest and hotspot-tier1. Does the patch make more sense now?
I'd like to keep the change small and contained, without significant rewrites, to simplify backports
without introducing new bugs. If additional check is significant on some path, users can just go to
non-large-hinted method versions.
-Aleksey
More information about the hotspot-dev
mailing list