RFR: 8213415: BitMap::word_index_round_up overflow problems

[Thomas Schatzl]

Hi,

On Tue, 2019-05-28 at 14:10 -0400, Kim Barrett wrote:
> Please review this change to BitMap index alignment to check for
> overflows (in debug builds).  Also made a few related API cleanups.
> 
> Constructing a BitMap now ensures the size is such that rounding it
> up to a word boundary won't overflow.  This is the new
> max_size_in_bits() value. This lets us add some asserts and otherwise
> tidy things up in some places by making use of that information.
> 
> This engendered some changes to ParallelGC's ParMarkBitMap.  It no
> longer uses the obsolete BitMap::word_align_up, instead having its
> own internal helper for aligning range ends that knows about
> invariants in ParMarkBitMap.
> 
> Removed the following static functions from BitMap: word_align_up,
> word_align_down, and is_word_aligned.  word_align_up is no longer
> used, after the ParMarkBitMap changes.  The others were only used in
> a small number of places in tests and asserts, and didn't seem worth
> keeping instead of just using align.hpp stuff with BitsPerWord
> alignment value.
> 
> CR:                                 
> https://bugs.openjdk.java.net/browse/JDK-8213415
> 
> Webrev:
> http://cr.openjdk.java.net/~kbarrett/8213415/open.00/
> 
> Testing:
> mach5 tier1-3

Minor comment:

- bitmap.hpp:

  52 // the bitmap.  BitsPerWord bits per element.

The second part is not a sentence. Maybe join these with a "with"
instead of the full stop? Or "An element has BitsPerWord bits".

Not sure this fact actually needs mention though. Maybe it would be
useful to do a static assert like the following somewhere:

STATIC_ASSERT(sizeof(bm_word_t) * BitsPerByte == BitsPerWord);

To actually enforce this and not only talk about it in some comment?

It's up to you whether you pick up these suggestions.

Looks good.

Thanks,
  Thomas