Correct set pitch in clEnqueueCopyBufferRect

validate dst and src buffers

Signed-off-by: Katarzyna Cencelewska <katarzyna.cencelewska@intel.com>

opencl/source/api/api.cpp

@@ -2363,7 +2363,8 @@ cl_int CL_API_CALL clEnqueueReadBufferRect(cl_command_queue commandQueue,
                                     bufferRowPitch,
                                     bufferSlicePitch,
                                     hostRowPitch,
-                                    hostSlicePitch) == false) {
+                                    hostSlicePitch,
+                                    true) == false) {
         retVal = CL_INVALID_VALUE;
         TRACING_EXIT(clEnqueueReadBufferRect, &retVal);
         return retVal;
@@ -2503,7 +2504,8 @@ cl_int CL_API_CALL clEnqueueWriteBufferRect(cl_command_queue commandQueue,
                                     bufferRowPitch,
                                     bufferSlicePitch,
                                     hostRowPitch,
-                                    hostSlicePitch) == false) {
+                                    hostSlicePitch,
+                                    true) == false) {
         retVal = CL_INVALID_VALUE;
         TRACING_EXIT(clEnqueueWriteBufferRect, &retVal);
         return retVal;
@@ -2680,6 +2682,25 @@ cl_int CL_API_CALL clEnqueueCopyBufferRect(cl_command_queue commandQueue,
         WithCastToInternal(dstBuffer, &pDstBuffer));
 
     if (CL_SUCCESS == retVal) {
+
+        if (!pSrcBuffer->bufferRectPitchSet(srcOrigin,
+                                            region,
+                                            srcRowPitch,
+                                            srcSlicePitch,
+                                            dstRowPitch,
+                                            dstSlicePitch,
+                                            true) ||
+            !pDstBuffer->bufferRectPitchSet(dstOrigin,
+                                            region,
+                                            srcRowPitch,
+                                            srcSlicePitch,
+                                            dstRowPitch,
+                                            dstSlicePitch,
+                                            false)) {
+            retVal = CL_INVALID_VALUE;
+            TRACING_EXIT(clEnqueueCopyBufferRect, &retVal);
+            return retVal;
+        }
         if (!pCommandQueue->validateCapabilityForOperation(CL_QUEUE_CAPABILITY_TRANSFER_BUFFER_RECT_INTEL, numEventsInWaitList, eventWaitList, event)) {
             retVal = CL_INVALID_OPERATION;
             TRACING_EXIT(clEnqueueCopyBufferRect, &retVal);

opencl/source/mem_obj/buffer.cpp

@@ -547,7 +547,8 @@ bool Buffer::bufferRectPitchSet(const size_t *bufferOrigin,
                                 size_t &bufferRowPitch,
                                 size_t &bufferSlicePitch,
                                 size_t &hostRowPitch,
-                                size_t &hostSlicePitch) {
+                                size_t &hostSlicePitch,
+                                bool isSrcBuffer) {
     if (bufferRowPitch == 0)
         bufferRowPitch = region[0];
     if (bufferSlicePitch == 0)
@@ -558,6 +559,10 @@ bool Buffer::bufferRectPitchSet(const size_t *bufferOrigin,
     if (hostSlicePitch == 0)
         hostSlicePitch = region[1] * hostRowPitch;
 
+    if (region[0] == 0 || region[1] == 0 || region[2] == 0) {
+        return false;
+    }
+
     if (bufferRowPitch < region[0] ||
         hostRowPitch < region[0]) {
         return false;
@@ -566,8 +571,9 @@ bool Buffer::bufferRectPitchSet(const size_t *bufferOrigin,
         (hostSlicePitch < region[1] * hostRowPitch || hostSlicePitch % hostRowPitch != 0)) {
         return false;
     }
-
+    auto slicePitch = isSrcBuffer ? bufferSlicePitch : hostSlicePitch;
-    if ((bufferOrigin[2] + region[2] - 1) * bufferSlicePitch + (bufferOrigin[1] + region[1] - 1) * bufferRowPitch + bufferOrigin[0] + region[0] > this->getSize()) {
+    auto rowPitch = isSrcBuffer ? bufferRowPitch : hostRowPitch;
+    if ((bufferOrigin[2] + region[2] - 1) * slicePitch + (bufferOrigin[1] + region[1] - 1) * rowPitch + bufferOrigin[0] + region[0] > this->getSize()) {
         return false;
     }
     return true;

opencl/source/mem_obj/buffer.h

@@ -144,7 +144,8 @@ class Buffer : public MemObj {
                             size_t &bufferRowPitch,
                             size_t &bufferSlicePitch,
                             size_t &hostRowPitch,
-                            size_t &hostSlicePitch);
+                            size_t &hostSlicePitch,
+                            bool isSrcBuffer);
 
     static size_t calculateHostPtrSize(const size_t *origin, const size_t *region, size_t rowPitch, size_t slicePitch);
 

opencl/test/unit_test/api/cl_enqueue_copy_buffer_rect_tests.inl

@@ -30,7 +30,7 @@ TEST_F(clEnqueueCopyBufferRectTests, GivenCorrectParametersWhenEnqueingCopyBuffe
     MockBuffer dstBuffer;
     size_t srcOrigin[] = {0, 0, 0};
     size_t dstOrigin[] = {0, 0, 0};
-    size_t region[] = {10, 10, 0};
+    size_t region[] = {10, 10, 1};
 
     auto retVal = clEnqueueCopyBufferRect(
         pCommandQueue,
@@ -74,7 +74,7 @@ TEST_F(clEnqueueCopyBufferRectTests, GivenQueueIncapableWhenEnqueingCopyBufferRe
     MockBuffer dstBuffer;
     size_t srcOrigin[] = {0, 0, 0};
     size_t dstOrigin[] = {0, 0, 0};
-    size_t region[] = {10, 10, 0};
+    size_t region[] = {10, 10, 1};
 
     this->disableQueueCapabilities(CL_QUEUE_CAPABILITY_TRANSFER_BUFFER_RECT_INTEL);
     auto retVal = clEnqueueCopyBufferRect(
@@ -95,4 +95,214 @@ TEST_F(clEnqueueCopyBufferRectTests, GivenQueueIncapableWhenEnqueingCopyBufferRe
     EXPECT_EQ(CL_INVALID_OPERATION, retVal);
 }
 
+TEST_F(clEnqueueCopyBufferRectTests, givenPitchesEqualZeroAndZerosInRegionWhenCallClEnqueueCopyBufferRectThenClInvalidValueIsReturned) {
+    MockBuffer srcBuffer;
+    MockBuffer dstBuffer;
+    size_t srcOrigin[] = {0, 0, 0};
+    size_t dstOrigin[] = {0, 0, 0};
+    size_t region[] = {0, 0, 0};
+
+    auto retVal = clEnqueueCopyBufferRect(
+        pCommandQueue,
+        &srcBuffer, //srcBuffer
+        &dstBuffer, //dstBuffer
+        srcOrigin,
+        dstOrigin,
+        region,
+        0, //srcRowPitch
+        0, //srcSlicePitch
+        0, //dstRowPitch
+        0, //dstSlicePitch
+        0, //numEventsInWaitList
+        nullptr,
+        nullptr);
+
+    EXPECT_EQ(CL_INVALID_VALUE, retVal);
+}
+
+TEST_F(clEnqueueCopyBufferRectTests, givenZeroInRegionWhenCallClEnqueueCopyBufferRectThenClInvalidValueIsReturned) {
+    MockBuffer srcBuffer;
+    MockBuffer dstBuffer;
+    size_t srcOrigin[] = {0, 0, 0};
+    size_t dstOrigin[] = {0, 0, 0};
+    size_t region[] = {0, 0, 0};
+
+    auto retVal = clEnqueueCopyBufferRect(
+        pCommandQueue,
+        &srcBuffer, //srcBuffer
+        &dstBuffer, //dstBuffer
+        srcOrigin,
+        dstOrigin,
+        region,
+        10, //srcRowPitch
+        0,  //srcSlicePitch
+        10, //dstRowPitch
+        0,  //dstSlicePitch
+        0,  //numEventsInWaitList
+        nullptr,
+        nullptr);
+
+    EXPECT_EQ(CL_INVALID_VALUE, retVal);
+
+    size_t region1[] = {10, 10, 0};
+
+    retVal = clEnqueueCopyBufferRect(
+        pCommandQueue,
+        &srcBuffer, //srcBuffer
+        &dstBuffer, //dstBuffer
+        srcOrigin,
+        dstOrigin,
+        region1,
+        10, //srcRowPitch
+        0,  //srcSlicePitch
+        10, //dstRowPitch
+        0,  //dstSlicePitch
+        0,  //numEventsInWaitList
+        nullptr,
+        nullptr);
+
+    EXPECT_EQ(CL_INVALID_VALUE, retVal);
+    size_t region2[] = {10, 0, 1};
+
+    retVal = clEnqueueCopyBufferRect(
+        pCommandQueue,
+        &srcBuffer, //srcBuffer
+        &dstBuffer, //dstBuffer
+        srcOrigin,
+        dstOrigin,
+        region2,
+        10, //srcRowPitch
+        0,  //srcSlicePitch
+        10, //dstRowPitch
+        0,  //dstSlicePitch
+        0,  //numEventsInWaitList
+        nullptr,
+        nullptr);
+
+    EXPECT_EQ(CL_INVALID_VALUE, retVal);
+    size_t region3[] = {10, 10, 0};
+
+    retVal = clEnqueueCopyBufferRect(
+        pCommandQueue,
+        &srcBuffer, //srcBuffer
+        &dstBuffer, //dstBuffer
+        srcOrigin,
+        dstOrigin,
+        region3,
+        10, //srcRowPitch
+        0,  //srcSlicePitch
+        10, //dstRowPitch
+        0,  //dstSlicePitch
+        0,  //numEventsInWaitList
+        nullptr,
+        nullptr);
+
+    EXPECT_EQ(CL_INVALID_VALUE, retVal);
+}
+
+TEST_F(clEnqueueCopyBufferRectTests, givenNonProperSrcBufferSizeWhenCallClEnqueueCopyBufferRectThenClInvalidValueIsReturned) {
+    MockBuffer srcBuffer;
+    srcBuffer.size = 10;
+    MockBuffer dstBuffer;
+    size_t srcOrigin[] = {0, 0, 0};
+    size_t dstOrigin[] = {0, 0, 0};
+    size_t region[] = {10, 10, 1};
+
+    auto retVal = clEnqueueCopyBufferRect(
+        pCommandQueue,
+        &srcBuffer, //srcBuffer
+        &dstBuffer, //dstBuffer
+        srcOrigin,
+        dstOrigin,
+        region,
+        10, //srcRowPitch
+        0,  //srcSlicePitch
+        10, //dstRowPitch
+        0,  //dstSlicePitch
+        0,  //numEventsInWaitList
+        nullptr,
+        nullptr);
+
+    EXPECT_EQ(CL_INVALID_VALUE, retVal);
+}
+
+TEST_F(clEnqueueCopyBufferRectTests, givenNonProperDstBufferSizeWhenCallClEnqueueCopyBufferRectThenClInvalidValueIsReturned) {
+    MockBuffer srcBuffer;
+    MockBuffer dstBuffer;
+    dstBuffer.size = 10;
+    size_t srcOrigin[] = {0, 0, 0};
+    size_t dstOrigin[] = {0, 0, 0};
+    size_t region[] = {10, 10, 1};
+
+    auto retVal = clEnqueueCopyBufferRect(
+        pCommandQueue,
+        &srcBuffer, //srcBuffer
+        &dstBuffer, //dstBuffer
+        srcOrigin,
+        dstOrigin,
+        region,
+        10, //srcRowPitch
+        0,  //srcSlicePitch
+        10, //dstRowPitch
+        0,  //dstSlicePitch
+        0,  //numEventsInWaitList
+        nullptr,
+        nullptr);
+
+    EXPECT_EQ(CL_INVALID_VALUE, retVal);
+}
+
+TEST_F(clEnqueueCopyBufferRectTests, givenPitchesEqualZeroAndNotZeroRegionWhenCallClEnqueueCopyBufferRectThenPitchIsSetBasedOnRegionAndClSuccessIsReturned) {
+    class CommandQueueMock : public MockCommandQueue {
+      public:
+        CommandQueueMock(Context *context, ClDevice *device, const cl_queue_properties *props) : MockCommandQueue(context, device, props) {}
+        cl_int enqueueCopyBufferRect(Buffer *srcBuffer, Buffer *dstBuffer, const size_t *srcOrigin, const size_t *dstOrigin,
+                                     const size_t *region, size_t argSrcRowPitch, size_t argSrcSlicePitch, size_t argDstRowPitch,
+                                     size_t argDstSlicePitch, cl_uint numEventsInWaitList,
+                                     const cl_event *eventWaitList, cl_event *event) override {
+            srcRowPitch = argSrcRowPitch;
+            srcSlicePitch = argSrcSlicePitch;
+            dstRowPitch = argDstRowPitch;
+            dstSlicePitch = argDstSlicePitch;
+            return CL_SUCCESS;
+        }
+
+        size_t srcRowPitch;
+        size_t srcSlicePitch;
+        size_t dstRowPitch;
+        size_t dstSlicePitch;
+    };
+
+    auto commandQueue = std::make_unique<CommandQueueMock>(pContext, pDevice, nullptr);
+    MockBuffer srcBuffer;
+    MockBuffer dstBuffer;
+    dstBuffer.size = 200;
+    srcBuffer.size = 200;
+    size_t srcOrigin[] = {0, 0, 0};
+    size_t dstOrigin[] = {0, 0, 0};
+    size_t region[] = {10, 20, 1};
+
+    auto retVal = clEnqueueCopyBufferRect(
+        commandQueue.get(),
+        &srcBuffer, //srcBuffer
+        &dstBuffer, //dstBuffer
+        srcOrigin,
+        dstOrigin,
+        region,
+        0, //srcRowPitch
+        0, //srcSlicePitch
+        0, //dstRowPitch
+        0, //dstSlicePitch
+        0, //numEventsInWaitList
+        nullptr,
+        nullptr);
+
+    EXPECT_EQ(CL_SUCCESS, retVal);
+
+    EXPECT_EQ(region[0], commandQueue->srcRowPitch);
+    EXPECT_EQ(region[0], commandQueue->dstRowPitch);
+
+    EXPECT_EQ(region[1], commandQueue->srcSlicePitch / commandQueue->srcRowPitch);
+    EXPECT_EQ(region[1], commandQueue->dstSlicePitch / commandQueue->dstRowPitch);
+}
 } // namespace ULT

opencl/test/unit_test/api/cl_enqueue_read_buffer_rect_tests.inl

@@ -75,7 +75,7 @@ TEST_F(clEnqueueReadBufferRectTest, GivenNullHostPtrWhenReadingRectangularRegion
     auto buffer = clCreateBuffer(
         pContext,
         CL_MEM_READ_WRITE,
-        20,
+        100,
         nullptr,
         &retVal);
     EXPECT_EQ(CL_SUCCESS, retVal);
@@ -110,7 +110,7 @@ TEST_F(clEnqueueReadBufferRectTest, GivenValidParametersWhenReadingRectangularRe
     auto buffer = clCreateBuffer(
         pContext,
         CL_MEM_READ_WRITE,
-        20,
+        100,
         nullptr,
         &retVal);
     EXPECT_EQ(CL_SUCCESS, retVal);
@@ -120,7 +120,7 @@ TEST_F(clEnqueueReadBufferRectTest, GivenValidParametersWhenReadingRectangularRe
 
     size_t buffOrigin[] = {0, 0, 0};
     size_t hostOrigin[] = {0, 0, 0};
-    size_t region[] = {10, 10, 0};
+    size_t region[] = {10, 10, 1};
 
     auto retVal = clEnqueueReadBufferRect(
         pCommandQueue,
@@ -144,11 +144,12 @@ TEST_F(clEnqueueReadBufferRectTest, GivenValidParametersWhenReadingRectangularRe
 
 TEST_F(clEnqueueReadBufferRectTest, GivenQueueIncapableWhenReadingRectangularRegionThenInvalidOperationIsReturned) {
     MockBuffer buffer{};
+    buffer.size = 100;
     char ptr[10];
 
     size_t buffOrigin[] = {0, 0, 0};
     size_t hostOrigin[] = {0, 0, 0};
-    size_t region[] = {10, 10, 0};
+    size_t region[] = {10, 10, 1};
 
     this->disableQueueCapabilities(CL_QUEUE_CAPABILITY_TRANSFER_BUFFER_RECT_INTEL);
     auto retVal = clEnqueueReadBufferRect(
@@ -174,7 +175,7 @@ TEST_F(clEnqueueReadBufferRectTest, GivenInvalidPitchWhenReadingRectangularRegio
     auto buffer = clCreateBuffer(
         pContext,
         CL_MEM_READ_WRITE,
-        20,
+        100,
         nullptr,
         &retVal);
     EXPECT_EQ(CL_SUCCESS, retVal);
@@ -184,7 +185,7 @@ TEST_F(clEnqueueReadBufferRectTest, GivenInvalidPitchWhenReadingRectangularRegio
 
     size_t buffOrigin[] = {0, 0, 0};
     size_t hostOrigin[] = {0, 0, 0};
-    size_t region[] = {10, 10, 0};
+    size_t region[] = {10, 10, 1};
     size_t bufferRowPitch = 9;
 
     auto retVal = clEnqueueReadBufferRect(

opencl/test/unit_test/api/cl_enqueue_write_buffer_rect_tests.inl

@@ -74,7 +74,7 @@ TEST_F(clEnqueueWriteBufferRectTests, GivenNullHostPtrWhenWritingRectangularRegi
     auto buffer = clCreateBuffer(
         pContext,
         CL_MEM_READ_WRITE,
-        20,
+        100,
         nullptr,
         &retVal);
     EXPECT_EQ(CL_SUCCESS, retVal);
@@ -107,11 +107,12 @@ TEST_F(clEnqueueWriteBufferRectTests, GivenNullHostPtrWhenWritingRectangularRegi
 
 TEST_F(clEnqueueWriteBufferRectTests, GivenCorrectParametersWhenWritingRectangularRegionThenSuccessIsReturned) {
     MockBuffer buffer{};
+    buffer.size = 100;
     char ptr[10];
 
     size_t buffOrigin[] = {0, 0, 0};
     size_t hostOrigin[] = {0, 0, 0};
-    size_t region[] = {10, 10, 0};
+    size_t region[] = {10, 10, 1};
 
     auto retVal = clEnqueueWriteBufferRect(
         pCommandQueue,
@@ -134,11 +135,12 @@ TEST_F(clEnqueueWriteBufferRectTests, GivenCorrectParametersWhenWritingRectangul
 
 TEST_F(clEnqueueWriteBufferRectTests, GivenQueueIncapableWhenWritingRectangularRegionThenInvalidOperationIsReturned) {
     MockBuffer buffer{};
+    buffer.size = 100;
     char ptr[10];
 
     size_t buffOrigin[] = {0, 0, 0};
     size_t hostOrigin[] = {0, 0, 0};
-    size_t region[] = {10, 10, 0};
+    size_t region[] = {10, 10, 1};
 
     this->disableQueueCapabilities(CL_QUEUE_CAPABILITY_TRANSFER_BUFFER_RECT_INTEL);
     auto retVal = clEnqueueWriteBufferRect(

opencl/test/unit_test/mem_obj/buffer_tests.cpp

@@ -77,6 +77,97 @@ TEST(Buffer, givenReadOnlySetOfInputFlagsWhenPassedToisReadOnlyMemoryPermittedBy
     EXPECT_TRUE(MockBuffer::isReadOnlyMemoryPermittedByFlags(memoryProperties));
 }
 
+TEST(TestBufferRectCheck, givenSmallerDstBufferWhenCallBufferRectPitchSetThenCorrectValidationIsDone) {
+    auto srcBuffer = std::make_unique<MockBuffer>();
+    ASSERT_NE(nullptr, srcBuffer);
+    srcBuffer->size = 500;
+
+    size_t originBuffer[] = {0, 0, 0};
+    size_t region[] = {10, 20, 1};
+    size_t srcRowPitch = 20u;
+    size_t srcSlicePitch = 0u;
+    size_t dstRowPitch = 10u;
+    size_t dstSlicePitch = 0u;
+
+    auto retVal = srcBuffer->bufferRectPitchSet(originBuffer, region, srcRowPitch, srcSlicePitch, dstRowPitch, dstSlicePitch, true);
+    EXPECT_TRUE(retVal);
+
+    auto dstBuffer = std::make_unique<MockBuffer>();
+    ASSERT_NE(nullptr, dstBuffer);
+    dstBuffer->size = 200;
+
+    EXPECT_GT(srcBuffer->size, dstBuffer->size);
+
+    retVal = dstBuffer->bufferRectPitchSet(originBuffer, region, srcRowPitch, srcSlicePitch, dstRowPitch, dstSlicePitch, false);
+    EXPECT_TRUE(retVal);
+    retVal = dstBuffer->bufferRectPitchSet(originBuffer, region, srcRowPitch, srcSlicePitch, dstRowPitch, dstSlicePitch, true);
+    EXPECT_FALSE(retVal);
+}
+
+TEST(TestBufferRectCheck, givenInvalidSrcPitchWhenCallBufferRectPitchSetThenReturnFalse) {
+    auto buffer = std::make_unique<MockBuffer>();
+    ASSERT_NE(nullptr, buffer);
+    buffer->size = 200;
+
+    size_t originBuffer[] = {0, 0, 0};
+    size_t region[] = {3, 1, 1};
+    size_t srcRowPitch = 10u;
+    size_t srcSlicePitch = 10u;
+    size_t dstRowPitch = 3u;
+    size_t dstSlicePitch = 10u;
+
+    auto retVal = buffer->bufferRectPitchSet(originBuffer, region, srcRowPitch, srcSlicePitch, dstRowPitch, dstSlicePitch, true);
+    EXPECT_FALSE(retVal);
+}
+
+TEST(TestBufferRectCheck, givenInvalidDstPitchWhenCallBufferRectPitchSetThenReturnFalse) {
+    auto buffer = std::make_unique<MockBuffer>();
+    ASSERT_NE(nullptr, buffer);
+    buffer->size = 200;
+
+    size_t originBuffer[] = {0, 0, 0};
+    size_t region[] = {3, 1, 1};
+    size_t srcRowPitch = 3u;
+    size_t srcSlicePitch = 10u;
+    size_t dstRowPitch = 10u;
+    size_t dstSlicePitch = 10u;
+
+    auto retVal = buffer->bufferRectPitchSet(originBuffer, region, srcRowPitch, srcSlicePitch, dstRowPitch, dstSlicePitch, true);
+    EXPECT_FALSE(retVal);
+}
+
+TEST(TestBufferRectCheck, givenInvalidDstAndSrcPitchWhenCallBufferRectPitchSetThenReturnFalse) {
+    auto buffer = std::make_unique<MockBuffer>();
+    ASSERT_NE(nullptr, buffer);
+    buffer->size = 200;
+
+    size_t originBuffer[] = {0, 0, 0};
+    size_t region[] = {3, 2, 1};
+    size_t srcRowPitch = 10u;
+    size_t srcSlicePitch = 10u;
+    size_t dstRowPitch = 10u;
+    size_t dstSlicePitch = 10u;
+
+    auto retVal = buffer->bufferRectPitchSet(originBuffer, region, srcRowPitch, srcSlicePitch, dstRowPitch, dstSlicePitch, true);
+    EXPECT_FALSE(retVal);
+}
+
+TEST(TestBufferRectCheck, givenCorrectDstAndSrcPitchWhenCallBufferRectPitchSetThenReturnTrue) {
+    auto buffer = std::make_unique<MockBuffer>();
+    ASSERT_NE(nullptr, buffer);
+    buffer->size = 200;
+
+    size_t originBuffer[] = {0, 0, 0};
+    size_t region[] = {3, 1, 1};
+    size_t srcRowPitch = 10u;
+    size_t srcSlicePitch = 10u;
+    size_t dstRowPitch = 10u;
+    size_t dstSlicePitch = 10u;
+
+    auto retVal = buffer->bufferRectPitchSet(originBuffer, region, srcRowPitch, srcSlicePitch, dstRowPitch, dstSlicePitch, true);
+    EXPECT_TRUE(retVal);
+}
+
 class BufferReadOnlyTest : public testing::TestWithParam<uint64_t> {
 };