Skip to content

Commit c10ff0f

Browse files
JablonskiMateuszCompute-Runtime-Automation
JablonskiMateusz
authored and
Compute-Runtime-Automation
committed
fix: respect arg size when setting kernel arg in OCL path
Fixes: #777 #777 Signed-off-by: Mateusz Jablonski <[email protected]>
1 parent 8154c21 commit c10ff0f

File tree

5 files changed

+99
-54
lines changed

5 files changed

+99
-54
lines changed

‎opencl/source/kernel/kernel.cpp

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1714,17 +1714,16 @@ cl_int Kernel::setArgImmediate(uint32_t argIndex,
17141714
const auto &argAsVal = kernelInfo.kernelDescriptor.payloadMappings.explicitArgs[argIndex].as<ArgDescValue>();
17151715
for (const auto &element : argAsVal.elements) {
17161716
DEBUG_BREAK_IF(element.size <= 0);
1717+
if (static_cast<size_t>(element.sourceOffset + element.size) > argSize) {
1718+
return CL_INVALID_ARG_SIZE;
1719+
}
17171720

17181721
auto pDst = ptrOffset(crossThreadData, element.offset);
17191722
auto pSrc = ptrOffset(argVal, element.sourceOffset);
17201723

1721-
DEBUG_BREAK_IF(!(ptrOffset(pDst, element.size) <= crossThreadDataEnd));
1724+
auto dstAvailableSpace = crossThreadDataEnd - pDst;
17221725

1723-
if (element.sourceOffset < argSize) {
1724-
size_t maxBytesToCopy = argSize - element.sourceOffset;
1725-
size_t bytesToCopy = std::min(static_cast<size_t>(element.size), maxBytesToCopy);
1726-
memcpy_s(pDst, element.size, pSrc, bytesToCopy);
1727-
}
1726+
memcpy_s(pDst, dstAvailableSpace, pSrc, element.size);
17281727
}
17291728

17301729
retVal = CL_SUCCESS;

‎opencl/test/unit_test/api/api_tests_wrapper3.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
#include "opencl/test/unit_test/api/cl_set_context_destructor_callback.inl"
3434
#include "opencl/test/unit_test/api/cl_set_event_callback_tests.inl"
3535
#include "opencl/test/unit_test/api/cl_set_kernel_arg_svm_pointer_tests.inl"
36+
#include "opencl/test/unit_test/api/cl_set_kernel_arg_tests.inl"
3637
#include "opencl/test/unit_test/api/cl_set_kernel_exec_info_tests.inl"
3738
#include "opencl/test/unit_test/api/cl_set_mem_object_destructor_callback_tests.inl"
3839
#include "opencl/test/unit_test/api/cl_set_performance_configuration_tests.inl"

‎opencl/test/unit_test/api/cl_set_kernel_arg_tests.inl

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
/*
2+
* Copyright (C) 2024-2025 Intel Corporation
3+
*
4+
* SPDX-License-Identifier: MIT
5+
*
6+
*/
7+
8+
#include "shared/source/compiler_interface/compiler_interface.h"
9+
#include "shared/source/compiler_interface/compiler_options.h"
10+
#include "shared/source/device/device.h"
11+
#include "shared/source/helpers/file_io.h"
12+
#include "shared/test/common/helpers/kernel_binary_helper.h"
13+
#include "shared/test/common/helpers/test_files.h"
14+
15+
#include "opencl/source/context/context.h"
16+
17+
#include "cl_api_tests.h"
18+
19+
using namespace NEO;
20+
21+
using ClSetKernelArgTests = ApiTests;
22+
23+
namespace ULT {
24+
25+
TEST_F(ClSetKernelArgTests, WhenSettingKernelArgThenArgSizeIsRespected) {
26+
cl_program pProgram = nullptr;
27+
size_t sourceSize = 0;
28+
std::string testFile;
29+
30+
KernelBinaryHelper kbHelper("simple_kernels", false);
31+
32+
testFile.append(clFiles);
33+
testFile.append("simple_kernels.cl");
34+
35+
auto pSource = loadDataFromFile(
36+
testFile.c_str(),
37+
sourceSize);
38+
39+
ASSERT_NE(0u, sourceSize);
40+
ASSERT_NE(nullptr, pSource);
41+
42+
const char *sources[1] = {pSource.get()};
43+
pProgram = clCreateProgramWithSource(
44+
pContext,
45+
1,
46+
sources,
47+
&sourceSize,
48+
&retVal);
49+
50+
EXPECT_NE(nullptr, pProgram);
51+
ASSERT_EQ(CL_SUCCESS, retVal);
52+
53+
retVal = clBuildProgram(
54+
pProgram,
55+
1,
56+
&testedClDevice,
57+
CompilerOptions::argInfo.data(),
58+
nullptr,
59+
nullptr);
60+
61+
ASSERT_EQ(CL_SUCCESS, retVal);
62+
63+
cl_kernel kernel = clCreateKernel(pProgram, "simple_kernel_1", &retVal);
64+
ASSERT_EQ(CL_SUCCESS, retVal);
65+
66+
uint32_t data = 1;
67+
retVal = clSetKernelArg(kernel, 1, 1, &data);
68+
EXPECT_EQ(CL_INVALID_ARG_SIZE, retVal);
69+
70+
retVal = clSetKernelArg(kernel, 1, sizeof(data), &data);
71+
EXPECT_EQ(CL_SUCCESS, retVal);
72+
73+
retVal = clReleaseKernel(kernel);
74+
EXPECT_EQ(CL_SUCCESS, retVal);
75+
76+
retVal = clReleaseProgram(pProgram);
77+
EXPECT_EQ(CL_SUCCESS, retVal);
78+
}
79+
} // namespace ULT

‎opencl/test/unit_test/kernel/clone_kernel_tests.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -485,9 +485,9 @@ TEST_F(CloneKernelTest, givenArgSvmAllocWhenCloningKernelThenKernelInfoIsCorrect
485485
}
486486

487487
TEST_F(CloneKernelTest, givenArgImmediateWhenCloningKernelThenKernelInfoIsCorrect) {
488-
pKernelInfo->addArgImmediate(0, sizeof(void *), 0x20);
489-
490488
using TypeParam = unsigned long;
489+
pKernelInfo->addArgImmediate(0, sizeof(TypeParam), 0x20);
490+
491491
auto value = (TypeParam)0xAA55AA55UL;
492492

493493
retVal = pSourceMultiDeviceKernel->setArg(0, sizeof(TypeParam), &value);

‎opencl/test/unit_test/kernel/kernel_immediate_arg_tests.cpp

Lines changed: 12 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (C) 2018-2024 Intel Corporation
2+
* Copyright (C) 2018-2025 Intel Corporation
33
*
44
* SPDX-License-Identifier: MIT
55
*
@@ -221,8 +221,7 @@ TYPED_TEST(KernelArgImmediateTest, givenTooLargePatchSizeWhenSettingArgThenDontR
221221

222222
const auto memoryBeyondLimitBefore = *reinterpret_cast<TypeParam *>(memoryBeyondLimitAddress);
223223

224-
this->pKernelInfo->argAsVal(0).elements[0].size = sizeof(TypeParam) + 1;
225-
auto retVal = pKernel->setArg(0, sizeof(TypeParam), &memory[0]);
224+
auto retVal = pKernel->setArg(0, sizeof(memory), memory);
226225

227226
const auto memoryBeyondLimitAfter = *reinterpret_cast<TypeParam *>(memoryBeyondLimitAddress);
228227
EXPECT_EQ(memoryBeyondLimitBefore, memoryBeyondLimitAfter);
@@ -257,43 +256,6 @@ TYPED_TEST(KernelArgImmediateTest, givenNotTooLargePatchSizeWhenSettingArgThenDo
257256
}
258257
}
259258

260-
TYPED_TEST(KernelArgImmediateTest, givenMulitplePatchesAndFirstPatchSizeTooLargeWhenSettingArgThenDontReadMemoryBeyondLimit) {
261-
if (sizeof(TypeParam) == 1)
262-
return; // multiple patch chars don't make sense
263-
264-
for (auto &rootDeviceIndex : this->context->getRootDeviceIndices()) {
265-
auto pKernel = this->pMultiDeviceKernel->getKernel(rootDeviceIndex);
266-
TypeParam memory[2];
267-
std::memset(&memory[0], 0xaa, sizeof(TypeParam));
268-
std::memset(&memory[1], 0xbb, sizeof(TypeParam));
269-
270-
auto &elements = this->pKernelInfo->argAsVal(3).elements;
271-
const auto destinationMemoryAddress1 = pKernel->getCrossThreadData() +
272-
elements[2].offset;
273-
const auto destinationMemoryAddress2 = pKernel->getCrossThreadData() +
274-
elements[1].offset;
275-
const auto memoryBeyondLimitAddress1 = destinationMemoryAddress1 + sizeof(TypeParam);
276-
const auto memoryBeyondLimitAddress2 = destinationMemoryAddress2 + sizeof(TypeParam) / 2;
277-
278-
const std::vector<unsigned char> memoryBeyondLimitBefore1(memoryBeyondLimitAddress1, memoryBeyondLimitAddress1 + sizeof(TypeParam));
279-
const std::vector<unsigned char> memoryBeyondLimitBefore2(memoryBeyondLimitAddress2, memoryBeyondLimitAddress2 + sizeof(TypeParam) / 2);
280-
281-
elements[2].sourceOffset = 0;
282-
elements[1].sourceOffset = sizeof(TypeParam) / 2;
283-
elements[2].size = sizeof(TypeParam);
284-
elements[1].size = sizeof(TypeParam) / 2;
285-
auto retVal = pKernel->setArg(3, sizeof(TypeParam), &memory[0]);
286-
287-
EXPECT_EQ(0, std::memcmp(memoryBeyondLimitBefore1.data(), memoryBeyondLimitAddress1, sizeof(TypeParam)));
288-
EXPECT_EQ(0, std::memcmp(memoryBeyondLimitBefore2.data(), memoryBeyondLimitAddress2, sizeof(TypeParam) / 2));
289-
290-
EXPECT_EQ(0, std::memcmp(&memory[0], destinationMemoryAddress1, sizeof(TypeParam)));
291-
EXPECT_EQ(0, std::memcmp(&memory[0], destinationMemoryAddress2, sizeof(TypeParam) / 2));
292-
293-
EXPECT_EQ(CL_SUCCESS, retVal);
294-
}
295-
}
296-
297259
TYPED_TEST(KernelArgImmediateTest, givenMulitplePatchesAndSecondPatchSizeTooLargeWhenSettingArgThenDontReadMemoryBeyondLimit) {
298260
if (sizeof(TypeParam) == 1)
299261
return; // multiple patch chars don't make sense
@@ -310,17 +272,17 @@ TYPED_TEST(KernelArgImmediateTest, givenMulitplePatchesAndSecondPatchSizeTooLarg
310272
const auto destinationMemoryAddress2 = pKernel->getCrossThreadData() +
311273
elements[1].offset;
312274
const auto memoryBeyondLimitAddress1 = destinationMemoryAddress1 + sizeof(TypeParam) / 2;
313-
const auto memoryBeyondLimitAddress2 = destinationMemoryAddress2 + sizeof(TypeParam) / 2;
275+
const auto memoryBeyondLimitAddress2 = destinationMemoryAddress2 + sizeof(TypeParam);
314276

315277
const std::vector<unsigned char> memoryBeyondLimitBefore1(memoryBeyondLimitAddress1, memoryBeyondLimitAddress1 + sizeof(TypeParam) / 2);
316-
const std::vector<unsigned char> memoryBeyondLimitBefore2(memoryBeyondLimitAddress2, memoryBeyondLimitAddress2 + sizeof(TypeParam) / 2);
278+
const std::vector<unsigned char> memoryBeyondLimitBefore2(memoryBeyondLimitAddress2, memoryBeyondLimitAddress2 + sizeof(TypeParam));
317279

318280
elements[0].size = 0;
319281
elements[2].sourceOffset = 0;
320282
elements[1].sourceOffset = sizeof(TypeParam) / 2;
321283
elements[2].size = sizeof(TypeParam) / 2;
322284
elements[1].size = sizeof(TypeParam);
323-
auto retVal = pKernel->setArg(3, sizeof(TypeParam), &memory[0]);
285+
auto retVal = pKernel->setArg(3, sizeof(memory), memory);
324286

325287
EXPECT_EQ(0, std::memcmp(memoryBeyondLimitBefore1.data(), memoryBeyondLimitAddress1, sizeof(TypeParam) / 2));
326288
EXPECT_EQ(0, std::memcmp(memoryBeyondLimitBefore2.data(), memoryBeyondLimitAddress2, sizeof(TypeParam) / 2));
@@ -333,6 +295,9 @@ TYPED_TEST(KernelArgImmediateTest, givenMulitplePatchesAndSecondPatchSizeTooLarg
333295
}
334296

335297
TYPED_TEST(KernelArgImmediateTest, givenMultiplePatchesAndOneSourceOffsetBeyondArgumentWhenSettingArgThenDontCopyThisPatch) {
298+
if (sizeof(TypeParam) == 1u) {
299+
GTEST_SKIP();
300+
}
336301
for (auto &rootDeviceIndex : this->context->getRootDeviceIndices()) {
337302
auto pKernel = this->pMultiDeviceKernel->getKernel(rootDeviceIndex);
338303
TypeParam memory[2];
@@ -345,22 +310,23 @@ TYPED_TEST(KernelArgImmediateTest, givenMultiplePatchesAndOneSourceOffsetBeyondA
345310
const auto destinationMemoryAddress2 = pKernel->getCrossThreadData() +
346311
elements[2].offset;
347312
const auto memoryBeyondLimitAddress1 = destinationMemoryAddress1 + sizeof(TypeParam);
348-
const auto memoryBeyondLimitAddress2 = destinationMemoryAddress2;
313+
const auto memoryBeyondLimitAddress2 = destinationMemoryAddress2 + 1;
349314

350315
const std::vector<unsigned char> memoryBeyondLimitBefore1(memoryBeyondLimitAddress1, memoryBeyondLimitAddress1 + sizeof(TypeParam));
351-
const std::vector<unsigned char> memoryBeyondLimitBefore2(memoryBeyondLimitAddress2, memoryBeyondLimitAddress2 + sizeof(TypeParam));
316+
const std::vector<unsigned char> memoryBeyondLimitBefore2(memoryBeyondLimitAddress2, memoryBeyondLimitAddress2 + sizeof(TypeParam) - 1);
352317

353318
elements[0].size = 0;
354319
elements[1].sourceOffset = 0;
355320
elements[1].size = sizeof(TypeParam);
356321
elements[2].sourceOffset = sizeof(TypeParam);
357322
elements[2].size = 1;
358-
auto retVal = pKernel->setArg(3, sizeof(TypeParam), &memory[0]);
323+
auto retVal = pKernel->setArg(3, sizeof(memory), memory);
359324

360325
EXPECT_EQ(0, std::memcmp(memoryBeyondLimitBefore1.data(), memoryBeyondLimitAddress1, memoryBeyondLimitBefore1.size()));
361326
EXPECT_EQ(0, std::memcmp(memoryBeyondLimitBefore2.data(), memoryBeyondLimitAddress2, memoryBeyondLimitBefore2.size()));
362327

363328
EXPECT_EQ(0, std::memcmp(&memory[0], destinationMemoryAddress1, sizeof(TypeParam)));
329+
EXPECT_EQ(0, std::memcmp(&memory[1], destinationMemoryAddress2, 1));
364330

365331
EXPECT_EQ(CL_SUCCESS, retVal);
366332
}

0 commit comments

Comments
 (0)