Revert "fix: respect arg size when setting kernel arg in OCL path"

This reverts commit c10ff0f.

Signed-off-by: Compute-Runtime-Validation <[email protected]>

Author: Compute-Runtime-Validation

opencl/source/kernel/kernel.cpp

@@ -1714,16 +1714,17 @@ cl_int Kernel::setArgImmediate(uint32_t argIndex,
         const auto &argAsVal = kernelInfo.kernelDescriptor.payloadMappings.explicitArgs[argIndex].as<ArgDescValue>();
         for (const auto &element : argAsVal.elements) {
             DEBUG_BREAK_IF(element.size <= 0);
-            if (static_cast<size_t>(element.sourceOffset + element.size) > argSize) {
-                return CL_INVALID_ARG_SIZE;
-            }
 
             auto pDst = ptrOffset(crossThreadData, element.offset);
             auto pSrc = ptrOffset(argVal, element.sourceOffset);
 
-            auto dstAvailableSpace = crossThreadDataEnd - pDst;
+            DEBUG_BREAK_IF(!(ptrOffset(pDst, element.size) <= crossThreadDataEnd));
 
-            memcpy_s(pDst, dstAvailableSpace, pSrc, element.size);
+            if (element.sourceOffset < argSize) {
+                size_t maxBytesToCopy = argSize - element.sourceOffset;
+                size_t bytesToCopy = std::min(static_cast<size_t>(element.size), maxBytesToCopy);
+                memcpy_s(pDst, element.size, pSrc, bytesToCopy);
+            }
         }
 
         retVal = CL_SUCCESS;

opencl/test/unit_test/api/api_tests_wrapper3.cpp

@@ -33,7 +33,6 @@
 #include "opencl/test/unit_test/api/cl_set_context_destructor_callback.inl"
 #include "opencl/test/unit_test/api/cl_set_event_callback_tests.inl"
 #include "opencl/test/unit_test/api/cl_set_kernel_arg_svm_pointer_tests.inl"
-#include "opencl/test/unit_test/api/cl_set_kernel_arg_tests.inl"
 #include "opencl/test/unit_test/api/cl_set_kernel_exec_info_tests.inl"
 #include "opencl/test/unit_test/api/cl_set_mem_object_destructor_callback_tests.inl"
 #include "opencl/test/unit_test/api/cl_set_performance_configuration_tests.inl"

opencl/test/unit_test/api/cl_set_kernel_arg_tests.inl

@@ -485,9 +485,9 @@ TEST_F(CloneKernelTest, givenArgSvmAllocWhenCloningKernelThenKernelInfoIsCorrect
 }
 
 TEST_F(CloneKernelTest, givenArgImmediateWhenCloningKernelThenKernelInfoIsCorrect) {
-    using TypeParam = unsigned long;
-    pKernelInfo->addArgImmediate(0, sizeof(TypeParam), 0x20);
+    pKernelInfo->addArgImmediate(0, sizeof(void *), 0x20);
 
+    using TypeParam = unsigned long;
     auto value = (TypeParam)0xAA55AA55UL;
 
     retVal = pSourceMultiDeviceKernel->setArg(0, sizeof(TypeParam), &value);

opencl/test/unit_test/kernel/clone_kernel_tests.cpp

@@ -221,7 +221,8 @@ TYPED_TEST(KernelArgImmediateTest, givenTooLargePatchSizeWhenSettingArgThenDontR
 
         const auto memoryBeyondLimitBefore = *reinterpret_cast<TypeParam *>(memoryBeyondLimitAddress);
 
-        auto retVal = pKernel->setArg(0, sizeof(memory), memory);
+        this->pKernelInfo->argAsVal(0).elements[0].size = sizeof(TypeParam) + 1;
+        auto retVal = pKernel->setArg(0, sizeof(TypeParam), &memory[0]);
 
         const auto memoryBeyondLimitAfter = *reinterpret_cast<TypeParam *>(memoryBeyondLimitAddress);
         EXPECT_EQ(memoryBeyondLimitBefore, memoryBeyondLimitAfter);
@@ -256,6 +257,43 @@ TYPED_TEST(KernelArgImmediateTest, givenNotTooLargePatchSizeWhenSettingArgThenDo
     }
 }
 
+TYPED_TEST(KernelArgImmediateTest, givenMulitplePatchesAndFirstPatchSizeTooLargeWhenSettingArgThenDontReadMemoryBeyondLimit) {
+    if (sizeof(TypeParam) == 1)
+        return; // multiple patch chars don't make sense
+
+    for (auto &rootDeviceIndex : this->context->getRootDeviceIndices()) {
+        auto pKernel = this->pMultiDeviceKernel->getKernel(rootDeviceIndex);
+        TypeParam memory[2];
+        std::memset(&memory[0], 0xaa, sizeof(TypeParam));
+        std::memset(&memory[1], 0xbb, sizeof(TypeParam));
+
+        auto &elements = this->pKernelInfo->argAsVal(3).elements;
+        const auto destinationMemoryAddress1 = pKernel->getCrossThreadData() +
+                                               elements[2].offset;
+        const auto destinationMemoryAddress2 = pKernel->getCrossThreadData() +
+                                               elements[1].offset;
+        const auto memoryBeyondLimitAddress1 = destinationMemoryAddress1 + sizeof(TypeParam);
+        const auto memoryBeyondLimitAddress2 = destinationMemoryAddress2 + sizeof(TypeParam) / 2;
+
+        const std::vector<unsigned char> memoryBeyondLimitBefore1(memoryBeyondLimitAddress1, memoryBeyondLimitAddress1 + sizeof(TypeParam));
+        const std::vector<unsigned char> memoryBeyondLimitBefore2(memoryBeyondLimitAddress2, memoryBeyondLimitAddress2 + sizeof(TypeParam) / 2);
+
+        elements[2].sourceOffset = 0;
+        elements[1].sourceOffset = sizeof(TypeParam) / 2;
+        elements[2].size = sizeof(TypeParam);
+        elements[1].size = sizeof(TypeParam) / 2;
+        auto retVal = pKernel->setArg(3, sizeof(TypeParam), &memory[0]);
+
+        EXPECT_EQ(0, std::memcmp(memoryBeyondLimitBefore1.data(), memoryBeyondLimitAddress1, sizeof(TypeParam)));
+        EXPECT_EQ(0, std::memcmp(memoryBeyondLimitBefore2.data(), memoryBeyondLimitAddress2, sizeof(TypeParam) / 2));
+
+        EXPECT_EQ(0, std::memcmp(&memory[0], destinationMemoryAddress1, sizeof(TypeParam)));
+        EXPECT_EQ(0, std::memcmp(&memory[0], destinationMemoryAddress2, sizeof(TypeParam) / 2));
+
+        EXPECT_EQ(CL_SUCCESS, retVal);
+    }
+}
+
 TYPED_TEST(KernelArgImmediateTest, givenMulitplePatchesAndSecondPatchSizeTooLargeWhenSettingArgThenDontReadMemoryBeyondLimit) {
     if (sizeof(TypeParam) == 1)
         return; // multiple patch chars don't make sense
@@ -272,17 +310,17 @@ TYPED_TEST(KernelArgImmediateTest, givenMulitplePatchesAndSecondPatchSizeTooLarg
         const auto destinationMemoryAddress2 = pKernel->getCrossThreadData() +
                                                elements[1].offset;
         const auto memoryBeyondLimitAddress1 = destinationMemoryAddress1 + sizeof(TypeParam) / 2;
-        const auto memoryBeyondLimitAddress2 = destinationMemoryAddress2 + sizeof(TypeParam);
+        const auto memoryBeyondLimitAddress2 = destinationMemoryAddress2 + sizeof(TypeParam) / 2;
 
         const std::vector<unsigned char> memoryBeyondLimitBefore1(memoryBeyondLimitAddress1, memoryBeyondLimitAddress1 + sizeof(TypeParam) / 2);
-        const std::vector<unsigned char> memoryBeyondLimitBefore2(memoryBeyondLimitAddress2, memoryBeyondLimitAddress2 + sizeof(TypeParam));
+        const std::vector<unsigned char> memoryBeyondLimitBefore2(memoryBeyondLimitAddress2, memoryBeyondLimitAddress2 + sizeof(TypeParam) / 2);
 
         elements[0].size = 0;
         elements[2].sourceOffset = 0;
         elements[1].sourceOffset = sizeof(TypeParam) / 2;
         elements[2].size = sizeof(TypeParam) / 2;
         elements[1].size = sizeof(TypeParam);
-        auto retVal = pKernel->setArg(3, sizeof(memory), memory);
+        auto retVal = pKernel->setArg(3, sizeof(TypeParam), &memory[0]);
 
         EXPECT_EQ(0, std::memcmp(memoryBeyondLimitBefore1.data(), memoryBeyondLimitAddress1, sizeof(TypeParam) / 2));
         EXPECT_EQ(0, std::memcmp(memoryBeyondLimitBefore2.data(), memoryBeyondLimitAddress2, sizeof(TypeParam) / 2));
@@ -295,9 +333,6 @@ TYPED_TEST(KernelArgImmediateTest, givenMulitplePatchesAndSecondPatchSizeTooLarg
 }
 
 TYPED_TEST(KernelArgImmediateTest, givenMultiplePatchesAndOneSourceOffsetBeyondArgumentWhenSettingArgThenDontCopyThisPatch) {
-    if (sizeof(TypeParam) == 1u) {
-        GTEST_SKIP();
-    }
     for (auto &rootDeviceIndex : this->context->getRootDeviceIndices()) {
         auto pKernel = this->pMultiDeviceKernel->getKernel(rootDeviceIndex);
         TypeParam memory[2];
@@ -310,23 +345,22 @@ TYPED_TEST(KernelArgImmediateTest, givenMultiplePatchesAndOneSourceOffsetBeyondA
         const auto destinationMemoryAddress2 = pKernel->getCrossThreadData() +
                                                elements[2].offset;
         const auto memoryBeyondLimitAddress1 = destinationMemoryAddress1 + sizeof(TypeParam);
-        const auto memoryBeyondLimitAddress2 = destinationMemoryAddress2 + 1;
+        const auto memoryBeyondLimitAddress2 = destinationMemoryAddress2;
 
         const std::vector<unsigned char> memoryBeyondLimitBefore1(memoryBeyondLimitAddress1, memoryBeyondLimitAddress1 + sizeof(TypeParam));
-        const std::vector<unsigned char> memoryBeyondLimitBefore2(memoryBeyondLimitAddress2, memoryBeyondLimitAddress2 + sizeof(TypeParam) - 1);
+        const std::vector<unsigned char> memoryBeyondLimitBefore2(memoryBeyondLimitAddress2, memoryBeyondLimitAddress2 + sizeof(TypeParam));
 
         elements[0].size = 0;
         elements[1].sourceOffset = 0;
         elements[1].size = sizeof(TypeParam);
         elements[2].sourceOffset = sizeof(TypeParam);
         elements[2].size = 1;
-        auto retVal = pKernel->setArg(3, sizeof(memory), memory);
+        auto retVal = pKernel->setArg(3, sizeof(TypeParam), &memory[0]);
 
         EXPECT_EQ(0, std::memcmp(memoryBeyondLimitBefore1.data(), memoryBeyondLimitAddress1, memoryBeyondLimitBefore1.size()));
         EXPECT_EQ(0, std::memcmp(memoryBeyondLimitBefore2.data(), memoryBeyondLimitAddress2, memoryBeyondLimitBefore2.size()));
 
         EXPECT_EQ(0, std::memcmp(&memory[0], destinationMemoryAddress1, sizeof(TypeParam)));
-        EXPECT_EQ(0, std::memcmp(&memory[1], destinationMemoryAddress2, 1));
 
         EXPECT_EQ(CL_SUCCESS, retVal);
     }