Skip to content

Add a workaround for empty Slice copy SIGSEGV #12850

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
lukasz-stec wants to merge 1 commit into from
Closed

Add a workaround for empty Slice copy SIGSEGV #12850

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 11 additions & 5 deletions core/trino-main/src/main/java/io/trino/operator/output/SlicePositionsAppender.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -196,12 +196,18 @@ private void duplicateBytes(Block block, int position, int count, int startOffse
{
int length = block.getSliceLength(position);
int newByteCount = count * length;
ensureBytesCapacity(currentOffset + newByteCount);
if (length == 0) {
// empty Slice, there is no need to copy anything
Arrays.fill(offsets, positionCount + 1, positionCount + count + 1, startOffset);
}
else {
ensureBytesCapacity(currentOffset + newByteCount);

Slice slice = block.getSlice(position, 0, length);
for (int i = 0; i < count; i++) {
slice.getBytes(0, bytes, startOffset + (i * length), length);
offsets[positionCount + i + 1] = startOffset + ((i + 1) * length);
Slice slice = block.getSlice(position, 0, length);
for (int i = 0; i < count; i++) {
slice.getBytes(0, bytes, startOffset + (i * length), length);
offsets[positionCount + i + 1] = startOffset + ((i + 1) * length);
}
}

positionCount += count;
Expand Down
23 changes: 23 additions & 0 deletions core/trino-main/src/test/java/io/trino/operator/output/TestPositionsAppender.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@
import static io.trino.spi.type.TimestampType.createTimestampType;
import static io.trino.spi.type.TinyintType.TINYINT;
import static io.trino.spi.type.VarbinaryType.VARBINARY;
import static io.trino.spi.type.VarcharType.VARCHAR;
import static io.trino.spi.type.VarcharType.createUnboundedVarcharType;
import static java.util.Objects.requireNonNull;
import static org.testng.Assert.assertEquals;
Expand Down Expand Up @@ -215,6 +216,21 @@ public void testConsecutiveBuilds(Type type)
assertEquals(positionsAppender.build().getPositionCount(), 0);
}

// testcase for jit bug described https://github.com/trinodb/trino/issues/12821
@Test(priority = Integer.MIN_VALUE)
public void testSliceRle()
{
PositionsAppender positionsAppender = POSITIONS_APPENDER_FACTORY.create(VARCHAR, 10, DEFAULT_MAX_PAGE_SIZE_IN_BYTES);

// first append some not empty value to avoid RleAwarePositionsAppender for the empty value
positionsAppender.appendRle(new RunLengthEncodedBlock(singleValueBlock("some value"), 1));
// append empty value multiple times to trigger jit compilation
Block emptyStringBlock = singleValueBlock("");
for (int i = 0; i < 1000; i++) {
positionsAppender.appendRle(new RunLengthEncodedBlock(emptyStringBlock, 2000));
}
}

@DataProvider(name = "nullRleTypes")
public static Object[][] nullRleTypes()
{
Expand Down Expand Up @@ -254,6 +270,13 @@ public static Object[][] types()
};
}

private static Block singleValueBlock(String value)
{
BlockBuilder blockBuilder = VARCHAR.createBlockBuilder(null, 1);
VARCHAR.writeSlice(blockBuilder, Slices.utf8Slice(value));
return blockBuilder.build();
}

private IntArrayList allPositions(int count)
{
return new IntArrayList(IntStream.range(0, count).toArray());
Expand Down