Fix sampler lifecycle

- Add refcount to sampler to avoid use after free

Change-Id: I0f03d8ed29b5b9dc5bee355ed266ac7437e92509
This commit is contained in:
Woloszyn, Wojciech
2018-08-10 04:42:52 -07:00
committed by sys_ocldev
parent 1a85f83235
commit 53d99ead24
22 changed files with 216 additions and 38 deletions

View File

@ -124,6 +124,15 @@ Kernel::~Kernel() {
kernelReflectionSurface = nullptr;
}
for (uint32_t i = 0; i < patchedArgumentsNum; i++) {
if (kernelInfo.kernelArgInfo.at(i).isSampler) {
auto sampler = castToObject<Sampler>(kernelArguments.at(i).object);
if (sampler) {
sampler->decRefInternal();
}
}
}
kernelArgHandlers.clear();
program->release();
}
@ -889,7 +898,7 @@ cl_int Kernel::setArgSvmAlloc(uint32_t argIndex, void *svmPtr, GraphicsAllocatio
return CL_SUCCESS;
}
void Kernel::storeKernelArg(uint32_t argIndex, kernelArgType argType, const void *argObject,
void Kernel::storeKernelArg(uint32_t argIndex, kernelArgType argType, void *argObject,
const void *argValue, size_t argSize,
GraphicsAllocation *argSvmAlloc, cl_mem_flags argSvmFlags) {
kernelArguments[argIndex].type = argType;
@ -1302,6 +1311,16 @@ cl_int Kernel::setArgSampler(uint32_t argIndex,
auto clSamplerObj = *(static_cast<const cl_sampler *>(argVal));
auto pSampler = castToObject<Sampler>(clSamplerObj);
if (pSampler) {
pSampler->incRefInternal();
}
if (kernelArguments.at(argIndex).object) {
auto oldSampler = castToObject<Sampler>(kernelArguments.at(argIndex).object);
UNRECOVERABLE_IF(!oldSampler);
oldSampler->decRefInternal();
}
if (pSampler && argSize == sizeof(cl_sampler *)) {
const auto &kernelArgInfo = kernelInfo.kernelArgInfo[argIndex];

View File

@ -66,7 +66,7 @@ class Kernel : public BaseObject<_cl_kernel> {
struct SimpleKernelArgInfo {
kernelArgType type;
const void *object;
void *object;
const void *value;
size_t size;
GraphicsAllocation *pSvmAlloc;
@ -283,7 +283,7 @@ class Kernel : public BaseObject<_cl_kernel> {
void storeKernelArg(uint32_t argIndex,
kernelArgType argType,
const void *argObject,
void *argObject,
const void *argValue,
size_t argSize,
GraphicsAllocation *argSvmAlloc = nullptr,

View File

@ -146,4 +146,10 @@ typedef Sampler *(*SamplerCreateFunc)(Context *context,
float lodMax);
typedef size_t (*getSamplerStateSizeHwFunc)();
template <>
inline Sampler *castToObject<Sampler>(const void *object) {
auto clSamplerObj = reinterpret_cast<const _cl_sampler *>(object);
return castToObject<Sampler>(const_cast<cl_sampler>(clSamplerObj));
}
} // namespace OCLRT

View File

@ -80,9 +80,9 @@ class MediaImageSetArgTest : public DeviceFixture,
}
void TearDown() override {
delete pKernelInfo;
delete srcImage;
delete pKernel;
delete pKernelInfo;
delete context;
DeviceFixture::TearDown();
}

View File

@ -65,8 +65,8 @@ class KernelArgSvmFixture : public api_fixture, public DeviceFixture {
}
void TearDown() override {
delete pKernelInfo;
delete pMockKernel;
delete pKernelInfo;
DeviceFixture::TearDown();
api_fixture::TearDown();

View File

@ -47,8 +47,8 @@ class KernelExecInfoFixture : public api_fixture {
clSVMFree(pContext, ptrSvm);
}
delete pKernelInfo;
delete pMockKernel;
delete pKernelInfo;
api_fixture::TearDown();
}

View File

@ -108,9 +108,9 @@ class CloneKernelFixture : public ContextFixture, public DeviceFixture {
}
void TearDown() override {
delete pKernelInfo;
delete pSourceKernel;
delete pClonedKernel;
delete pKernelInfo;
delete pProgram;
ContextFixture::TearDown();
DeviceFixture::TearDown();

View File

@ -90,8 +90,8 @@ class KernelArgAcceleratorFixture : public ContextFixture, public DeviceFixture
}
void TearDown() override {
delete pKernelInfo;
delete pKernel;
delete pKernelInfo;
delete pProgram;
ContextFixture::TearDown();
DeviceFixture::TearDown();

View File

@ -70,8 +70,8 @@ void KernelArgBufferFixture::SetUp() {
}
void KernelArgBufferFixture::TearDown() {
delete pKernelInfo;
delete pKernel;
delete pKernelInfo;
delete pProgram;
ContextFixture::TearDown();
DeviceFixture::TearDown();

View File

@ -60,8 +60,8 @@ struct KernelArgDevQueueTest : public DeviceFixture,
}
void TearDown() override {
delete pKernelInfo;
delete pKernel;
delete pKernelInfo;
delete pDeviceQueue;
DeviceHostQueueFixture<DeviceQueue>::TearDown();

View File

@ -79,8 +79,8 @@ class KernelArgPipeFixture : public ContextFixture, public DeviceFixture {
}
void TearDown() override {
delete pKernelInfo;
delete pKernel;
delete pKernelInfo;
delete pProgram;
ContextFixture::TearDown();
DeviceFixture::TearDown();

View File

@ -76,8 +76,8 @@ class KernelArgSvmFixture_ : public ContextFixture, public DeviceFixture {
}
void TearDown() override {
delete pKernelInfo;
delete pKernel;
delete pKernelInfo;
delete pProgram;
ContextFixture::TearDown();
DeviceFixture::TearDown();

View File

@ -81,8 +81,8 @@ class KernelArgImmediateTest : public Test<DeviceFixture> {
}
void TearDown() override {
delete pKernelInfo;
delete pKernel;
delete pKernelInfo;
DeviceFixture::TearDown();
}

View File

@ -70,8 +70,8 @@ class KernelSlmArgTest : public Test<DeviceFixture> {
}
void TearDown() override {
delete pKernelInfo;
delete pKernel;
delete pKernelInfo;
DeviceFixture::TearDown();
}

View File

@ -1740,8 +1740,8 @@ struct KernelExecutionEnvironmentTest : public Test<DeviceFixture> {
}
void TearDown() override {
delete pKernelInfo;
delete pKernel;
delete pKernelInfo;
DeviceFixture::TearDown();
}

View File

@ -83,11 +83,11 @@ class KernelTransformableTest : public ::testing::Test {
cl_int retVal = CL_SUCCESS;
MockContext context;
std::unique_ptr<MockProgram> program;
std::unique_ptr<MockKernel> pKernel;
std::unique_ptr<Sampler> sampler;
std::unique_ptr<KernelInfo> pKernelInfo;
std::unique_ptr<MockKernel> pKernel;
std::unique_ptr<Image> image;
std::unique_ptr<Sampler> sampler;
SKernelBinaryHeaderCommon kernelHeader;
char surfaceStateHeap[0x80];
};
@ -262,6 +262,7 @@ HWTEST_F(KernelTransformableTest, givenKernelWithTwoTransformableImagesAndTwoTra
EXPECT_FALSE(firstSurfaceState->getSurfaceArray());
EXPECT_EQ(SURFACE_TYPE::SURFACE_TYPE_SURFTYPE_3D, secondSurfaceState->getSurfaceType());
EXPECT_FALSE(secondSurfaceState->getSurfaceArray());
pKernel.reset();
}
HWTEST_F(KernelTransformableTest, givenKernelWithNonTransformableSamplersWhenResetSamplerWithNontransformableThenImagesNotChangedAgain) {

View File

@ -98,8 +98,8 @@ class BufferSetArgTest : public ContextFixture,
void TearDown() override {
delete buffer;
delete BufferDefaults::context;
delete pKernelInfo;
delete pKernel;
delete pKernelInfo;
delete pProgram;
ContextFixture::TearDown();
DeviceFixture::TearDown();

View File

@ -112,9 +112,9 @@ class ImageSetArgTest : public DeviceFixture,
}
void TearDown() override {
delete pKernelInfo;
delete srcImage;
delete pKernel;
delete pKernelInfo;
delete context;
DeviceFixture::TearDown();
}

View File

@ -52,8 +52,8 @@ TEST(PrintfHandlerTest, givenNotPreparedPrintfHandlerWhenGetSurfaceIsCalledThenR
delete printfHandler;
delete pPrintfSurface;
delete pKernelInfo;
delete pKernel;
delete pKernelInfo;
delete pProgram;
delete device;
}
@ -81,8 +81,8 @@ TEST(PrintfHandlerTest, givenPreparedPrintfHandlerWhenGetSurfaceIsCalledThenResu
delete printfHandler;
delete pPrintfSurface;
delete pKernelInfo;
delete pKernel;
delete pKernelInfo;
delete pProgram;
delete device;
}

View File

@ -27,6 +27,7 @@
#include "runtime/helpers/sampler_helpers.h"
#include "runtime/utilities/numeric.h"
#include "unit_tests/fixtures/device_fixture.h"
#include "unit_tests/fixtures/image_fixture.h"
#include "test.h"
#include "unit_tests/mocks/mock_context.h"
#include "unit_tests/mocks/mock_kernel.h"
@ -84,9 +85,9 @@ class SamplerSetArgFixture : public DeviceFixture {
}
void TearDown() {
delete pKernel;
delete pKernelInfo;
delete sampler;
delete pKernel;
delete context;
DeviceFixture::TearDown();
}
@ -165,6 +166,124 @@ HWTEST_F(SamplerSetArgTest, getKernelArgShouldReturnSampler) {
EXPECT_EQ(samplerObj, pKernel->getKernelArg(0));
}
HWTEST_F(SamplerSetArgTest, GivenSamplerObjectWhenSetKernelArgIsCalledThenIncreaseSamplerRefcount) {
cl_sampler samplerObj = Sampler::create(
context,
CL_TRUE,
CL_ADDRESS_MIRRORED_REPEAT,
CL_FILTER_NEAREST,
retVal);
auto pSampler = castToObject<Sampler>(samplerObj);
auto refCountBefore = pSampler->getRefInternalCount();
retVal = pKernel->setArg(
0,
sizeof(samplerObj),
&samplerObj);
ASSERT_EQ(CL_SUCCESS, retVal);
auto refCountAfter = pSampler->getRefInternalCount();
EXPECT_EQ(refCountBefore + 1, refCountAfter);
retVal = clReleaseSampler(samplerObj);
ASSERT_EQ(CL_SUCCESS, retVal);
}
HWTEST_F(SamplerSetArgTest, GivenSamplerObjectWhenSetKernelArgIsCalledAndKernelIsDeletedThenRefCountIsUnchanged) {
auto myKernel = std::make_unique<MockKernel>(program.get(), *pKernelInfo, *pDevice);
ASSERT_NE(nullptr, myKernel.get());
ASSERT_EQ(CL_SUCCESS, myKernel->initialize());
myKernel->setKernelArgHandler(0, &Kernel::setArgSampler);
myKernel->setKernelArgHandler(1, &Kernel::setArgSampler);
uint32_t crossThreadData[crossThreadDataSize] = {};
myKernel->setCrossThreadData(crossThreadData, sizeof(crossThreadData));
cl_sampler samplerObj = Sampler::create(
context,
CL_TRUE,
CL_ADDRESS_MIRRORED_REPEAT,
CL_FILTER_NEAREST,
retVal);
auto pSampler = castToObject<Sampler>(samplerObj);
auto refCountBefore = pSampler->getRefInternalCount();
retVal = myKernel->setArg(
0,
sizeof(samplerObj),
&samplerObj);
ASSERT_EQ(CL_SUCCESS, retVal);
myKernel.reset();
auto refCountAfter = pSampler->getRefInternalCount();
EXPECT_EQ(refCountBefore, refCountAfter);
retVal = clReleaseSampler(samplerObj);
ASSERT_EQ(CL_SUCCESS, retVal);
}
HWTEST_F(SamplerSetArgTest, GivenNewSamplerObjectWhensSetKernelArgIsCalledThenDecreaseOldSamplerRefcount) {
cl_sampler samplerObj = Sampler::create(
context,
CL_TRUE,
CL_ADDRESS_MIRRORED_REPEAT,
CL_FILTER_NEAREST,
retVal);
auto clSamplerObj = *(static_cast<const cl_sampler *>(&samplerObj));
cl_sampler s = clSamplerObj;
auto pSampler = castToObjectOrAbort<Sampler>(s);
retVal = pKernel->setArg(
0,
sizeof(samplerObj),
&samplerObj);
ASSERT_EQ(CL_SUCCESS, retVal);
auto refCountBefore = pSampler->getRefInternalCount();
cl_sampler samplerObj2 = Sampler::create(
context,
CL_TRUE,
CL_ADDRESS_MIRRORED_REPEAT,
CL_FILTER_NEAREST,
retVal);
retVal = pKernel->setArg(
0,
sizeof(samplerObj2),
&samplerObj2);
ASSERT_EQ(CL_SUCCESS, retVal);
auto refCountAfter = pSampler->getRefInternalCount();
EXPECT_EQ(refCountBefore - 1, refCountAfter);
retVal = clReleaseSampler(samplerObj);
ASSERT_EQ(CL_SUCCESS, retVal);
retVal = clReleaseSampler(samplerObj2);
ASSERT_EQ(CL_SUCCESS, retVal);
}
HWTEST_F(SamplerSetArgTest, GivenIncorrentSamplerObjectWhenSetKernelArgSamplerIsCalledThenLeaveRefcountAsIs) {
auto notSamplerObj = std::unique_ptr<Image>(ImageHelper<Image2dDefaults>::create(context));
auto pNotSampler = castToObject<Image>(notSamplerObj.get());
auto refCountBefore = pNotSampler->getRefInternalCount();
retVal = pKernel->setArgSampler(
0,
sizeof(notSamplerObj.get()),
notSamplerObj.get());
auto refCountAfter = pNotSampler->getRefInternalCount();
EXPECT_EQ(refCountBefore, refCountAfter);
}
HWTEST_F(SamplerSetArgTest, WithFilteringNearestAndAddressingClClampSetAsKernelArgumentSetsConstantBuffer) {
sampler = Sampler::create(

View File

@ -21,6 +21,7 @@
*/
#include "runtime/sampler/sampler.h"
#include "unit_tests/fixtures/image_fixture.h"
#include "unit_tests/mocks/mock_context.h"
#include "unit_tests/mocks/mock_sampler.h"
#include "gtest/gtest.h"
@ -130,3 +131,29 @@ INSTANTIATE_TEST_CASE_P(Sampler,
::testing::ValuesIn(normalizedCoordModes),
::testing::ValuesIn(addressingModes),
::testing::ValuesIn(filterModes)));
TEST(castToSamplerTest, GivenGenericPointerWhichHoldsSamplerObjectWhenCastToSamplerIsCalledThenCastWithSuccess) {
cl_int retVal;
auto context = std::make_unique<MockContext>();
cl_sampler clSampler = Sampler::create(
context.get(),
CL_TRUE,
CL_ADDRESS_MIRRORED_REPEAT,
CL_FILTER_NEAREST,
retVal);
ASSERT_EQ(CL_SUCCESS, retVal);
auto ptr = reinterpret_cast<void *>(clSampler);
auto sampler = castToObject<Sampler>(ptr);
EXPECT_NE(nullptr, sampler);
clReleaseSampler(clSampler);
}
TEST(castToSamplerTest, GivenGenericPointerWhichDoestNotHoldSamplerObjectWhenCastToSamplerIsCalledThenCastWithAFailure) {
auto context = std::make_unique<MockContext>();
auto notSamplerObj = std::unique_ptr<Image>(ImageHelper<Image2dDefaults>::create(context.get()));
auto ptr = reinterpret_cast<void *>(notSamplerObj.get());
auto notSampler = castToObject<Sampler>(ptr);
EXPECT_EQ(nullptr, notSampler);
}

View File

@ -41,13 +41,8 @@ class MockSchedulerKernel : public SchedulerKernel {
MockSchedulerKernel(Program *program, const KernelInfo &info, const Device &device) : SchedulerKernel(program, info, device) {
}
~MockSchedulerKernel() override {
if (kernelInfoOwner)
delete &kernelInfo;
}
static MockSchedulerKernel *create(Program &program, Device &device) {
KernelInfo *info = new KernelInfo;
static MockSchedulerKernel *create(Program &program, Device &device, KernelInfo *&info) {
info = new KernelInfo;
SPatchDataParameterStream dataParametrStream;
dataParametrStream.DataParameterStreamSize = 8;
dataParametrStream.Size = 8;
@ -70,11 +65,8 @@ class MockSchedulerKernel : public SchedulerKernel {
}
MockSchedulerKernel *mock = Kernel::create<MockSchedulerKernel>(&program, *info, nullptr);
mock->kernelInfoOwner = true;
return mock;
}
bool kernelInfoOwner = false;
};
TEST(SchedulerKernelTest, getLws) {
@ -142,8 +134,10 @@ TEST(SchedulerKernelTest, setArgsForSchedulerKernel) {
auto device = unique_ptr<MockDevice>(MockDevice::createWithNewExecutionEnvironment<MockDevice>(nullptr));
MockProgram program;
program.setDevice(device.get());
unique_ptr<MockSchedulerKernel> scheduler = unique_ptr<MockSchedulerKernel>(MockSchedulerKernel::create(program, *device.get()));
unique_ptr<KernelInfo> info(nullptr);
KernelInfo *infoPtr = nullptr;
unique_ptr<MockSchedulerKernel> scheduler = unique_ptr<MockSchedulerKernel>(MockSchedulerKernel::create(program, *device.get(), infoPtr));
info.reset(infoPtr);
unique_ptr<MockGraphicsAllocation> allocs[9];
for (uint32_t i = 0; i < 9; i++) {
@ -170,7 +164,10 @@ TEST(SchedulerKernelTest, setArgsForSchedulerKernelWithNullDebugQueue) {
MockProgram program;
program.setDevice(device.get());
unique_ptr<MockSchedulerKernel> scheduler = unique_ptr<MockSchedulerKernel>(MockSchedulerKernel::create(program, *device.get()));
unique_ptr<KernelInfo> info(nullptr);
KernelInfo *infoPtr = nullptr;
unique_ptr<MockSchedulerKernel> scheduler = unique_ptr<MockSchedulerKernel>(MockSchedulerKernel::create(program, *device.get(), infoPtr));
info.reset(infoPtr);
unique_ptr<MockGraphicsAllocation> allocs[9];
for (uint32_t i = 0; i < 9; i++) {
@ -200,7 +197,10 @@ TEST(SchedulerKernelTest, createKernelReflectionForForcedSchedulerDispatch) {
MockProgram program;
program.setDevice(device.get());
unique_ptr<MockSchedulerKernel> scheduler = unique_ptr<MockSchedulerKernel>(MockSchedulerKernel::create(program, *device.get()));
unique_ptr<KernelInfo> info(nullptr);
KernelInfo *infoPtr = nullptr;
unique_ptr<MockSchedulerKernel> scheduler = unique_ptr<MockSchedulerKernel>(MockSchedulerKernel::create(program, *device.get(), infoPtr));
info.reset(infoPtr);
scheduler->createReflectionSurface();
@ -215,7 +215,10 @@ TEST(SchedulerKernelTest, createKernelReflectionSecondTimeForForcedSchedulerDisp
MockProgram program;
program.setDevice(device.get());
unique_ptr<MockSchedulerKernel> scheduler = unique_ptr<MockSchedulerKernel>(MockSchedulerKernel::create(program, *device.get()));
unique_ptr<KernelInfo> info(nullptr);
KernelInfo *infoPtr = nullptr;
unique_ptr<MockSchedulerKernel> scheduler = unique_ptr<MockSchedulerKernel>(MockSchedulerKernel::create(program, *device.get(), infoPtr));
info.reset(infoPtr);
scheduler->createReflectionSurface();
@ -234,7 +237,10 @@ TEST(SchedulerKernelTest, createKernelReflectionForSchedulerDoesNothing) {
MockProgram program;
program.setDevice(device.get());
unique_ptr<MockSchedulerKernel> scheduler = unique_ptr<MockSchedulerKernel>(MockSchedulerKernel::create(program, *device.get()));
unique_ptr<KernelInfo> info(nullptr);
KernelInfo *infoPtr = nullptr;
unique_ptr<MockSchedulerKernel> scheduler = unique_ptr<MockSchedulerKernel>(MockSchedulerKernel::create(program, *device.get(), infoPtr));
info.reset(infoPtr);
scheduler->createReflectionSurface();