Description
[Some issues in BitMap set operations noted while reviewing nearby changes.]
In src/share/vm/utilities/bitMap.cpp
274 bool BitMap::contains(const BitMap& other) const {
...
280 bm_word_t word_union = dest_map[index] | other_map[index];
281 // If this has more bits set than dest_map[index], then other is not a
282 // subset.
283 if (word_union != dest_map[index]) return false;
This is assuming well-defined values for the bits after _size when _size is not word-aligned. I think no such guarantee exists. In particular, when the data array is externally managed it's just plopped in without any modifications, and trailing bits could be anything.
Many of the other set-related operations have similar issues.
In the same function(s)
src/share/vm/utilities/bitMap.cpp
274 bool BitMap::contains(const BitMap& other) const {
...
278 idx_t size = size_in_words();
279 for (idx_t index = 0; index < size_in_words(); index++) {
There are no uses of the size variable in this function. It pretty obviously was intended as manual CSE of the RHS of the iteration test. Some of the other set-related operations have a similar issue, while others use the size variable as expected. Given the simplicity of the loop bodies, an optimizing compiler can probably do the CSE itself, but there's no harm in doing it manually. But we shouldn't be declaring an unused variable; either use or eliminate, and make all the similar places do the same thing.
In src/share/vm/utilities/bitMap.cpp
274 bool BitMap::contains(const BitMap& other) const {
...
280 bm_word_t word_union = dest_map[index] | other_map[index];
281 // If this has more bits set than dest_map[index], then other is not a
282 // subset.
283 if (word_union != dest_map[index]) return false;
This is assuming well-defined values for the bits after _size when _size is not word-aligned. I think no such guarantee exists. In particular, when the data array is externally managed it's just plopped in without any modifications, and trailing bits could be anything.
Many of the other set-related operations have similar issues.
In the same function(s)
src/share/vm/utilities/bitMap.cpp
274 bool BitMap::contains(const BitMap& other) const {
...
278 idx_t size = size_in_words();
279 for (idx_t index = 0; index < size_in_words(); index++) {
There are no uses of the size variable in this function. It pretty obviously was intended as manual CSE of the RHS of the iteration test. Some of the other set-related operations have a similar issue, while others use the size variable as expected. Given the simplicity of the loop bodies, an optimizing compiler can probably do the CSE itself, but there's no harm in doing it manually. But we shouldn't be declaring an unused variable; either use or eliminate, and make all the similar places do the same thing.
Attachments
Issue Links
- relates to
-
JDK-8235714 BitMap::count_one_bits may count bits beyond unaligned end
-
- Resolved
-
-
JDK-8228692 BitMap::reallocate might not clear some bits
-
- Resolved
-