Skip to content

Commit

Permalink
ARROW-10860: [Java] Avoid integer overflow for generated classes in V…
Browse files Browse the repository at this point in the history
…ector

For the current implementation in the templates for Vector, `int * int` multiplication is used to calculate a buffer offset. The result may be larger than Integer.MAX_VALUE, which will lead to integer overflow and unexpected behaviors.

This PR is like a follow-up of apache#8721.

Closes apache#8876 from kiszk/ARROW-10860

Authored-by: Kazuaki Ishizaki <[email protected]>
Signed-off-by: liyafan82 <[email protected]>
  • Loading branch information
kiszk authored and liyafan82 committed Dec 10, 2020
1 parent 3deae8d commit b0b9269
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 17 deletions.
26 changes: 13 additions & 13 deletions java/vector/src/main/codegen/templates/DenseUnionVector.java
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ private void setReaderAndWriterIndex() {
typeBuffer.writerIndex(valueCount * TYPE_WIDTH);

offsetBuffer.readerIndex(0);
offsetBuffer.writerIndex(valueCount * OFFSET_WIDTH);
offsetBuffer.writerIndex((long) valueCount * OFFSET_WIDTH);
}

@Override
Expand Down Expand Up @@ -402,7 +402,7 @@ public void reAlloc() {
}
public int getOffset(int index) {
return offsetBuffer.getInt(index * OFFSET_WIDTH);
return offsetBuffer.getInt((long) index * OFFSET_WIDTH);
}
private void reallocTypeBuffer() {
Expand Down Expand Up @@ -546,9 +546,9 @@ public TransferPair makeTransferPair(ValueVector target) {
public void copyFrom(int inIndex, int outIndex, ValueVector from) {
Preconditions.checkArgument(this.getMinorType() == from.getMinorType());
DenseUnionVector fromCast = (DenseUnionVector) from;
int inOffset = fromCast.offsetBuffer.getInt(inIndex * OFFSET_WIDTH);
int inOffset = fromCast.offsetBuffer.getInt((long) inIndex * OFFSET_WIDTH);
fromCast.getReader().setPosition(inOffset);
int outOffset = offsetBuffer.getInt(outIndex * OFFSET_WIDTH);
int outOffset = offsetBuffer.getInt((long) outIndex * OFFSET_WIDTH);
getWriter().setPosition(outOffset);
ComplexCopier.copy(fromCast.reader, writer);
}
Expand Down Expand Up @@ -630,7 +630,7 @@ public void splitAndTransfer(int startIndex, int length) {
to.typeBuffer = refManager.transferOwnership(slicedBuffer, to.allocator).getTransferredBuffer();
// transfer offset byffer
while (to.offsetBuffer.capacity() < length * OFFSET_WIDTH) {
while (to.offsetBuffer.capacity() < (long) length * OFFSET_WIDTH) {
to.reallocOffsetBuffer();
}
Expand All @@ -643,10 +643,10 @@ public void splitAndTransfer(int startIndex, int length) {
for (int i = startIndex; i < startIndex + length; i++) {
byte typeId = typeBuffer.getByte(i);
to.offsetBuffer.setInt((i - startIndex) * OFFSET_WIDTH, typeCounts[typeId]);
to.offsetBuffer.setInt((long) (i - startIndex) * OFFSET_WIDTH, typeCounts[typeId]);
typeCounts[typeId] += 1;
if (typeStarts[typeId] == -1) {
typeStarts[typeId] = offsetBuffer.getInt(i * OFFSET_WIDTH);
typeStarts[typeId] = offsetBuffer.getInt((long) i * OFFSET_WIDTH);
}
}
Expand Down Expand Up @@ -697,8 +697,8 @@ public int getBufferSizeFor(final int count) {
if (count == 0) {
return 0;
}
return count * TYPE_WIDTH + count * OFFSET_WIDTH + DataSizeRoundingUtil.divideBy8Ceil(count)
+ internalStruct.getBufferSizeFor(count);
return (int) (count * TYPE_WIDTH + (long) count * OFFSET_WIDTH
+ DataSizeRoundingUtil.divideBy8Ceil(count) + internalStruct.getBufferSizeFor(count));
}
@Override
Expand Down Expand Up @@ -736,7 +736,7 @@ private ValueVector getVector(int index) {
public Object getObject(int index) {
ValueVector vector = getVector(index);
if (vector != null) {
int offset = offsetBuffer.getInt(index * OFFSET_WIDTH);
int offset = offsetBuffer.getInt((long) index * OFFSET_WIDTH);
return vector.isNull(offset) ? null : vector.getObject(offset);
}
return null;
Expand Down Expand Up @@ -799,7 +799,7 @@ public void setSafe(int index, DenseUnionHolder holder) {
if (writer == null) {
writer = new DenseUnionWriter(DenseUnionVector.this);
}
int offset = offsetBuffer.getInt(index * OFFSET_WIDTH);
int offset = offsetBuffer.getInt((long) index * OFFSET_WIDTH);
MinorType type = reader.getMinorType();
writer.setPosition(offset);
byte typeId = holder.typeId;
Expand Down Expand Up @@ -843,7 +843,7 @@ public void setSafe(int index, Nullable${name}Holder holder) {
int offset = vector.getValueCount();
vector.setValueCount(offset + 1);
vector.setSafe(offset, holder);
offsetBuffer.setInt(index * OFFSET_WIDTH, offset);
offsetBuffer.setInt((long) index * OFFSET_WIDTH, offset);
}
</#if>
</#list>
Expand All @@ -869,7 +869,7 @@ public int hashCode(int index, ArrowBufHasher hasher) {
if (isNull(index)) {
return 0;
}
int offset = offsetBuffer.getInt(index * OFFSET_WIDTH);
int offset = offsetBuffer.getInt((long) index * OFFSET_WIDTH);
return getVector(index).hashCode(offset, hasher);
}

Expand Down
8 changes: 4 additions & 4 deletions java/vector/src/main/codegen/templates/UnionListWriter.java
Original file line number Diff line number Diff line change
Expand Up @@ -180,27 +180,27 @@ public StructWriter struct(String name) {
@Override
public void startList() {
vector.startNewValue(idx());
writer.setPosition(checkedCastToInt(vector.getOffsetBuffer().getLong(((long) idx() + 1L) * OFFSET_WIDTH)));
writer.setPosition(checkedCastToInt(vector.getOffsetBuffer().getLong((idx() + 1L) * OFFSET_WIDTH)));
listStarted = true;
}

@Override
public void endList() {
vector.getOffsetBuffer().setLong(((long) idx() + 1L) * OFFSET_WIDTH, writer.idx());
vector.getOffsetBuffer().setLong((idx() + 1L) * OFFSET_WIDTH, writer.idx());
setPosition(idx() + 1);
listStarted = false;
}
<#else>
@Override
public void startList() {
vector.startNewValue(idx());
writer.setPosition(vector.getOffsetBuffer().getInt((idx() + 1) * OFFSET_WIDTH));
writer.setPosition(vector.getOffsetBuffer().getInt((idx() + 1L) * OFFSET_WIDTH));
listStarted = true;
}

@Override
public void endList() {
vector.getOffsetBuffer().setInt((idx() + 1) * OFFSET_WIDTH, writer.idx());
vector.getOffsetBuffer().setInt((idx() + 1L) * OFFSET_WIDTH, writer.idx());
setPosition(idx() + 1);
listStarted = false;
}
Expand Down

0 comments on commit b0b9269

Please sign in to comment.