Fix potential integer overflow in BufferArrayGrouper (#9605)

This change fixes a potential integer overflow in BufferArrayGrouper that
was flagged by LGTM. It also adds a check that the vectorized arrays are
initialized before aggregateVector is called.

The changes in HashTableUtils should not have any effect since the numbers
being multiplied are small, but the change will remove the warnings from
being flagged in LGTM.
This commit is contained in:
Suneet Saldanha 2020-04-09 17:46:15 -07:00 committed by GitHub
parent 9888268000
commit 65de636893
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 9 additions and 5 deletions

View File

@ -191,6 +191,10 @@ public class BufferArrayGrouper implements VectorGrouper, IntGrouper
throw new ISE("keySpace too large to handle"); throw new ISE("keySpace too large to handle");
} }
if (vAggregationPositions == null || vAggregationRows == null) {
throw new ISE("Grouper was not initialized for vectorization");
}
if (keySpace.getCapacity() == 0) { if (keySpace.getCapacity() == 0) {
// Empty key space, assume keys are all zeroes. // Empty key space, assume keys are all zeroes.
final int dimIndex = 1; final int dimIndex = 1;
@ -206,7 +210,7 @@ public class BufferArrayGrouper implements VectorGrouper, IntGrouper
} else { } else {
for (int i = 0; i < numRows; i++) { for (int i = 0; i < numRows; i++) {
// +1 matches what hashFunction() would do. // +1 matches what hashFunction() would do.
final int dimIndex = keySpace.getInt(i * Integer.BYTES) + 1; final int dimIndex = keySpace.getInt(((long) i) * Integer.BYTES) + 1;
if (dimIndex < 0 || dimIndex >= cardinalityWithMissingValue) { if (dimIndex < 0 || dimIndex >= cardinalityWithMissingValue) {
throw new IAE("Invalid dimIndex[%s]", dimIndex); throw new IAE("Invalid dimIndex[%s]", dimIndex);

View File

@ -65,19 +65,19 @@ public class HashTableUtils
case 9: case 9:
return 31 * (31 * (31 + memory.getInt(position)) + memory.getInt(position + Integer.BYTES)) return 31 * (31 * (31 + memory.getInt(position)) + memory.getInt(position + Integer.BYTES))
+ memory.getByte(position + 2 * Integer.BYTES); + memory.getByte(position + 2L * Integer.BYTES);
case 12: case 12:
return 31 * (31 * (31 + memory.getInt(position)) + memory.getInt(position + Integer.BYTES)) return 31 * (31 * (31 + memory.getInt(position)) + memory.getInt(position + Integer.BYTES))
+ memory.getInt(position + 2 * Integer.BYTES); + memory.getInt(position + 2L * Integer.BYTES);
case 13: case 13:
return 31 * (31 * (31 * (31 + memory.getInt(position)) + memory.getInt(position + Integer.BYTES)) return 31 * (31 * (31 * (31 + memory.getInt(position)) + memory.getInt(position + Integer.BYTES))
+ memory.getInt(position + 2 * Integer.BYTES)) + memory.getByte(position + 3 * Integer.BYTES); + memory.getInt(position + 2L * Integer.BYTES)) + memory.getByte(position + 3L * Integer.BYTES);
case 16: case 16:
return 31 * (31 * (31 * (31 + memory.getInt(position)) + memory.getInt(position + Integer.BYTES)) return 31 * (31 * (31 * (31 + memory.getInt(position)) + memory.getInt(position + Integer.BYTES))
+ memory.getInt(position + 2 * Integer.BYTES)) + memory.getInt(position + 3 * Integer.BYTES); + memory.getInt(position + 2L * Integer.BYTES)) + memory.getInt(position + 3L * Integer.BYTES);
default: default:
int hashCode = 1; int hashCode = 1;