RFR (S) 8211926: Catastrophic size_t underflow in BitMap::*_large methods
kim.barrett at oracle.com
Sat Nov 17 01:40:06 UTC 2018
> On Nov 12, 2018, at 2:14 AM, Thomas Stüfe <thomas.stuefe at gmail.com> wrote:
> Hi Kim,
> On Sat, Nov 10, 2018 at 6:55 PM Kim Barrett <kim.barrett at oracle.com> wrote:
>>> On Nov 10, 2018, at 7:58 AM, Thomas Stüfe <thomas.stuefe at gmail.com> wrote:
>>> - I find it confusing that we use the same type (idx_t) both for word-
>>> and bit-indices. It would make the code a bit more readable if those
>>> were separate types (even if they both were to alias to size_t,
>> I dislike this too. I’d be interested in seeing a proposal to fix that, though
>> I worry about the fanout. I *think* clients mostly operate in bits and the
>> use of words is mostly limited to the implementation, but I haven’t done
>> the searches needed to check that.
> Should I have the time I'll give this a try. Not sure when I will find
> the time though.
>>> - Generally, I find putting the decision between "large" and
>>> "non-large" APIs off to the caller a bit strange. As a user of bitmap,
>>> I cannot know at which point which variant would be better. I would
>>> prefer the class itself handling that.
>> I was thinking much the same thing while reviewing this change.
>> Maybe some of the public API in this area should be revisited.
>> In particular, I was surprised that one might call “large” functions
>> directly, rather than calling hinted functions with a “large” hint.
>> Perhaps the “large” functions should be private helpers that just
>> do what they are told (maybe with an assert as before), and the
>> calls to them should be based on the hint and the runtime size
>> check they are now performing. Or maybe the whole hint thing
>> should be reconsidered. I’d be interested in a proposal in this
>> area too.
> I would consider switching between loops and memset to be an internal
> tuning parameter which the caller should not have to bother with. I
> think hinting makes sense when I know something the code cannot know.
> But there are no hidden information here beside the range size. Unless
> I misunderstand the problem.
I mostly agree. The thing a hint could be used for is to order a sequence
of tests. For example, if hinted large, then check for large first, while if
hinted small, check for the single bit or all bits in one word cases first.
More information about the hotspot-dev