diff --git a/runtime/command_queue/command_queue.cpp b/runtime/command_queue/command_queue.cpp index 1e5bc82671..06fe1716ff 100644 --- a/runtime/command_queue/command_queue.cpp +++ b/runtime/command_queue/command_queue.cpp @@ -313,7 +313,10 @@ cl_int CommandQueue::enqueueAcquireSharedObjects(cl_uint numObjects, const cl_me return CL_INVALID_MEM_OBJECT; } - memObject->peekSharingHandler()->acquire(memObject); + int result = memObject->peekSharingHandler()->acquire(memObject); + if (result != CL_SUCCESS) { + return result; + } memObject->acquireCount++; } auto status = enqueueMarkerWithWaitList( diff --git a/runtime/os_interface/windows/thk_wrapper.h b/runtime/os_interface/windows/thk_wrapper.h index 91d0c9f167..2b79615aba 100644 --- a/runtime/os_interface/windows/thk_wrapper.h +++ b/runtime/os_interface/windows/thk_wrapper.h @@ -1,5 +1,5 @@ /* - * Copyright (c) 2017, Intel Corporation + * Copyright (c) 2017 - 2018, Intel Corporation * * Permission is hereby granted, free of charge, to any person obtaining a * copy of this software and associated documentation files (the "Software"), @@ -85,7 +85,7 @@ class ThkWrapper { typedef NTSTATUS(APIENTRY *Func)(Param); public: - Func mFunc; + Func mFunc = nullptr; inline NTSTATUS operator()(Param param) const { if (UseTimer) { diff --git a/runtime/sharings/d3d/d3d_sharing.cpp b/runtime/sharings/d3d/d3d_sharing.cpp index e28133f32a..b26ea34ac2 100644 --- a/runtime/sharings/d3d/d3d_sharing.cpp +++ b/runtime/sharings/d3d/d3d_sharing.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2017, Intel Corporation + * Copyright (c) 2017 - 2018, Intel Corporation * * Permission is hereby granted, free of charge, to any person obtaining a * copy of this software and associated documentation files (the "Software"), @@ -35,19 +35,23 @@ template D3DSharing::D3DSharing(Context *context, D3DResource *resource, D3DResource *resourceStaging, unsigned int subresource, bool sharedResource) : sharedResource(sharedResource), subresource(subresource), resource(resource), resourceStaging(resourceStaging), context(context) { sharingFunctions = context->getSharing>(); - sharingFunctions->addRef(resource); - sharingFunctions->createQuery(&this->d3dQuery); - sharingFunctions->track(resource, subresource); + if (sharingFunctions) { + sharingFunctions->addRef(resource); + sharingFunctions->createQuery(&this->d3dQuery); + sharingFunctions->track(resource, subresource); + } }; template D3DSharing::~D3DSharing() { - sharingFunctions->untrack(resource, subresource); - if (!sharedResource) { - sharingFunctions->release(resourceStaging); + if (sharingFunctions) { + sharingFunctions->untrack(resource, subresource); + if (!sharedResource) { + sharingFunctions->release(resourceStaging); + } + sharingFunctions->release(resource); + sharingFunctions->release(d3dQuery); } - sharingFunctions->release(resource); - sharingFunctions->release(d3dQuery); }; template diff --git a/runtime/sharings/d3d/d3d_surface.cpp b/runtime/sharings/d3d/d3d_surface.cpp index cc86690b1f..0ea01a7e37 100644 --- a/runtime/sharings/d3d/d3d_surface.cpp +++ b/runtime/sharings/d3d/d3d_surface.cpp @@ -36,7 +36,9 @@ D3DSurface::D3DSurface(Context *context, cl_dx9_surface_info_khr *surfaceInfo, D : D3DSharing(context, surfaceInfo->resource, surfaceStaging, plane, sharedResource), adapterType(adapterType), surfaceInfo(*surfaceInfo), lockable(lockable), plane(plane), oclPlane(oclPlane), d3d9Surface(surfaceInfo->resource), d3d9SurfaceStaging(surfaceStaging) { - resourceDevice = sharingFunctions->getDevice(); + if (sharingFunctions) { + resourceDevice = sharingFunctions->getDevice(); + } }; Image *D3DSurface::create(Context *context, cl_dx9_surface_info_khr *surfaceInfo, cl_mem_flags flags, @@ -125,7 +127,7 @@ void D3DSurface::synchronizeObject(UpdateData *updateData) { sharingFunctions->lockRect(d3d9SurfaceStaging, &lockedRect, D3DLOCK_READONLY); } - auto image = castToObject(updateData->memObject); + auto image = castToObjectOrAbort(updateData->memObject); auto sys = lockedRect.pBits; auto gpu = context->getMemoryManager()->lockResource(image->getGraphicsAllocation()); auto pitch = static_cast(lockedRect.Pitch); @@ -148,6 +150,11 @@ void D3DSurface::synchronizeObject(UpdateData *updateData) { void D3DSurface::releaseResource(MemObj *memObject) { D3DLOCKED_RECT lockedRect = {}; + auto image = castToObject(memObject); + if (!image) { + return; + } + sharingFunctions->setDevice(resourceDevice); if (!sharedResource) { if (lockable) { @@ -156,7 +163,6 @@ void D3DSurface::releaseResource(MemObj *memObject) { sharingFunctions->lockRect(d3d9SurfaceStaging, &lockedRect, 0); } - auto image = castToObject(memObject); auto sys = lockedRect.pBits; auto gpu = context->getMemoryManager()->lockResource(image->getGraphicsAllocation()); auto pitch = static_cast(lockedRect.Pitch); @@ -306,3 +312,12 @@ cl_int D3DSurface::findImgFormat(D3DFORMAT d3dFormat, cl_image_format &imgFormat } return CL_SUCCESS; } + +int D3DSurface::validateUpdateData(UpdateData *updateData) { + UNRECOVERABLE_IF(updateData == nullptr); + auto image = castToObject(updateData->memObject); + if (!image) { + return CL_INVALID_MEM_OBJECT; + } + return CL_SUCCESS; +} diff --git a/runtime/sharings/d3d/d3d_surface.h b/runtime/sharings/d3d/d3d_surface.h index 8f351e094d..18ff4e029a 100644 --- a/runtime/sharings/d3d/d3d_surface.h +++ b/runtime/sharings/d3d/d3d_surface.h @@ -1,5 +1,5 @@ /* - * Copyright (c) 2017, Intel Corporation + * Copyright (c) 2017 - 2018, Intel Corporation * * Permission is hereby granted, free of charge, to any person obtaining a * copy of this software and associated documentation files (the "Software"), @@ -41,6 +41,7 @@ class D3DSurface : public D3DSharing { static cl_int findImgFormat(D3DFORMAT d3dFormat, cl_image_format &imgFormat, cl_uint plane, OCLPlane &oclPlane); void synchronizeObject(UpdateData *updateData) override; void releaseResource(MemObj *memObject) override; + int validateUpdateData(UpdateData *updateData) override; cl_dx9_surface_info_khr &getSurfaceInfo() { return surfaceInfo; } cl_dx9_media_adapter_type_khr &getAdapterType() { return adapterType; } diff --git a/runtime/sharings/sharing.cpp b/runtime/sharings/sharing.cpp index dca8e3a814..a0700b213f 100644 --- a/runtime/sharings/sharing.cpp +++ b/runtime/sharings/sharing.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2017, Intel Corporation + * Copyright (c) 2017 - 2018, Intel Corporation * * Permission is hereby granted, free of charge, to any person obtaining a * copy of this software and associated documentation files (the "Software"), @@ -27,21 +27,34 @@ #include namespace OCLRT { -void SharingHandler::acquire(MemObj *memObj) { +int SharingHandler::acquire(MemObj *memObj) { if (acquireCount == 0) { UpdateData updateData; auto currentSharedHandle = memObj->getGraphicsAllocation()->peekSharedHandle(); updateData.sharedHandle = currentSharedHandle; updateData.memObject = memObj; - synchronizeHandler(&updateData); + int result = synchronizeHandler(&updateData); + if (result != CL_SUCCESS) { + return result; + } DEBUG_BREAK_IF(updateData.synchronizationStatus != SynchronizeStatus::ACQUIRE_SUCCESFUL); DEBUG_BREAK_IF(currentSharedHandle != updateData.sharedHandle); } acquireCount++; + return CL_SUCCESS; } -void SharingHandler::synchronizeHandler(UpdateData *updateData) { - synchronizeObject(updateData); +int SharingHandler::synchronizeHandler(UpdateData *updateData) { + auto result = validateUpdateData(updateData); + if (result == CL_SUCCESS) { + synchronizeObject(updateData); + } + return result; +} + +int SharingHandler::validateUpdateData(UpdateData *updateData) { + UNRECOVERABLE_IF(updateData == nullptr); + return CL_SUCCESS; } void SharingHandler::release(MemObj *memObject) { diff --git a/runtime/sharings/sharing.h b/runtime/sharings/sharing.h index cc3d7384ac..2ca93fa961 100644 --- a/runtime/sharings/sharing.h +++ b/runtime/sharings/sharing.h @@ -1,5 +1,5 @@ /* - * Copyright (c) 2017, Intel Corporation + * Copyright (c) 2017 - 2018, Intel Corporation * * Permission is hereby granted, free of charge, to any person obtaining a * copy of this software and associated documentation files (the "Software"), @@ -50,7 +50,7 @@ class SharingFunctions { class SharingHandler { public: - void acquire(MemObj *memObj); + int acquire(MemObj *memObj); void release(MemObj *memObject); virtual ~SharingHandler() = default; @@ -58,7 +58,8 @@ class SharingHandler { virtual void releaseReusedGraphicsAllocation(){}; protected: - virtual void synchronizeHandler(UpdateData *updateData); + virtual int synchronizeHandler(UpdateData *updateData); + virtual int validateUpdateData(UpdateData *updateData); virtual void synchronizeObject(UpdateData *updateData) { updateData->synchronizationStatus = SYNCHRONIZE_ERROR; } virtual void releaseResource(MemObj *memObject){}; unsigned int acquireCount = 0u; diff --git a/unit_tests/command_queue/command_queue_tests.cpp b/unit_tests/command_queue/command_queue_tests.cpp index 30427ba3b6..86e3ab6a71 100644 --- a/unit_tests/command_queue/command_queue_tests.cpp +++ b/unit_tests/command_queue/command_queue_tests.cpp @@ -933,6 +933,24 @@ TEST(CommandQueue, givenEnqueueReleaseSharedObjectsWhenIncorrectArgumentsThenRet EXPECT_EQ(result, CL_INVALID_MEM_OBJECT); } +TEST(CommandQueue, givenEnqueueAcquireSharedObjectsCallWhenAcquireFailsThenCorrectErrorIsReturned) { + class MockSharingHandler : public SharingHandler { + int validateUpdateData(UpdateData *data) override { + return CL_INVALID_MEM_OBJECT; + } + }; + MockContext context; + CommandQueue cmdQ(&context, nullptr, 0); + auto buffer = std::unique_ptr(BufferHelper<>::create(&context)); + + MockSharingHandler *handler = new MockSharingHandler; + buffer->setSharingHandler(handler); + cl_mem memObject = buffer.get(); + auto retVal = cmdQ.enqueueAcquireSharedObjects(1, &memObject, 0, nullptr, nullptr, 0); + EXPECT_EQ(CL_INVALID_MEM_OBJECT, retVal); + buffer->setSharingHandler(nullptr); +} + HWTEST_F(CommandQueueCommandStreamTest, givenDebugKernelWhenSetupDebugSurfaceIsCalledThenSurfaceStateIsCorrectlySet) { using RENDER_SURFACE_STATE = typename FamilyType::RENDER_SURFACE_STATE; MockProgram program; diff --git a/unit_tests/d3d_sharing/d3d9_tests.cpp b/unit_tests/d3d_sharing/d3d9_tests.cpp index 8314cbdf7c..8c4a84432c 100644 --- a/unit_tests/d3d_sharing/d3d9_tests.cpp +++ b/unit_tests/d3d_sharing/d3d9_tests.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2017, Intel Corporation + * Copyright (c) 2017 - 2018, Intel Corporation * * Permission is hereby granted, free of charge, to any person obtaining a * copy of this software and associated documentation files (the "Software"), diff --git a/unit_tests/d3d_sharing/d3d_tests.cpp b/unit_tests/d3d_sharing/d3d_tests.cpp index 2c8140c577..7257bc38ef 100644 --- a/unit_tests/d3d_sharing/d3d_tests.cpp +++ b/unit_tests/d3d_sharing/d3d_tests.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2017, Intel Corporation + * Copyright (c) 2017 - 2018, Intel Corporation * * Permission is hereby granted, free of charge, to any person obtaining a * copy of this software and associated documentation files (the "Software"), @@ -28,10 +28,12 @@ #include "runtime/sharings/d3d/cl_d3d_api.h" #include "runtime/sharings/d3d/d3d_sharing.h" #include "runtime/sharings/d3d/d3d_buffer.h" +#include "runtime/sharings/d3d/d3d_surface.h" #include "runtime/sharings/d3d/d3d_texture.h" #include "gtest/gtest.h" #include "gmock/gmock.h" #include "test.h" +#include "unit_tests/mocks/mock_buffer.h" #include "unit_tests/mocks/mock_context.h" #include "unit_tests/mocks/mock_command_queue.h" #include "unit_tests/fixtures/platform_fixture.h" @@ -1385,4 +1387,22 @@ REGISTER_TYPED_TEST_CASE_P(D3DAuxTests, INSTANTIATE_TYPED_TEST_CASE_P(D3DSharingTests, D3DAuxTests, D3DTypes); +TEST(D3DSurfaceTest, givenD3DSurfaceWhenInvalidMemObjectIsPassedToValidateUpdateDataThenInvalidMemObjectErrorIsReturned) { + class MockD3DSurface : public D3DSurface { + public: + MockD3DSurface(Context *context, cl_dx9_surface_info_khr *surfaceInfo, D3DTypesHelper::D3D9::D3DTexture2d *surfaceStaging, cl_uint plane, + OCLPlane oclPlane, cl_dx9_media_adapter_type_khr adapterType, bool sharedResource, bool lockable) : D3DSurface(context, surfaceInfo, surfaceStaging, plane, + oclPlane, adapterType, sharedResource, lockable) {} + }; + MockContext context; + cl_dx9_surface_info_khr surfaceInfo = {}; + OCLPlane oclPlane = OCLPlane::NO_PLANE; + std::unique_ptr surface(new MockD3DSurface(&context, &surfaceInfo, nullptr, 0, oclPlane, 0, false, false)); + + MockBuffer buffer; + UpdateData updateData; + updateData.memObject = &buffer; + auto result = surface->validateUpdateData(&updateData); + EXPECT_EQ(CL_INVALID_MEM_OBJECT, result); +} } // namespace OCLRT diff --git a/unit_tests/os_interface/windows/gdi_interface_tests.cpp b/unit_tests/os_interface/windows/gdi_interface_tests.cpp index d372e8aec4..eb723d6bc2 100644 --- a/unit_tests/os_interface/windows/gdi_interface_tests.cpp +++ b/unit_tests/os_interface/windows/gdi_interface_tests.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2017, Intel Corporation + * Copyright (c) 2017 - 2018, Intel Corporation * * Permission is hereby granted, free of charge, to any person obtaining a * copy of this software and associated documentation files (the "Software"), @@ -42,4 +42,8 @@ TEST(GdiInterface, failLoad) { Os::gdiDllName = oldName; } +TEST(ThkWrapperTest, givenThkWrapperWhenConstructedThenmFuncIsInitialized) { + OCLRT::ThkWrapper wrapper; + EXPECT_EQ(nullptr, wrapper.mFunc); +} #endif diff --git a/unit_tests/sharings/sharing_tests.cpp b/unit_tests/sharings/sharing_tests.cpp index a6857d6036..f97d7d3c80 100644 --- a/unit_tests/sharings/sharing_tests.cpp +++ b/unit_tests/sharings/sharing_tests.cpp @@ -30,7 +30,7 @@ using namespace OCLRT; TEST(sharingHandler, givenBasicSharingHandlerWhenSynchronizeObjectThenErrorIsReturned) { struct SH : SharingHandler { - void synchronizeHandlerMock(UpdateData *updateData) { return synchronizeHandler(updateData); } + int synchronizeHandlerMock(UpdateData *updateData) { return synchronizeHandler(updateData); } } sharingHandler; @@ -105,3 +105,13 @@ TEST(sharingHandler, givenMemObjWhenAcquireTwoTimesThenReleaseShouldBeCalledTwoT EXPECT_EQ(sharingHandler.release(memObj.get()), 0u); EXPECT_EQ(sharingHandler.releaseCount, 1); } + +TEST(sharingHandler, givenSharingHandlerWhenValidateUpdateDataIsCalledWithNonNullInputThenAbortIsNotCalled) { + class MockSharingHandler : SharingHandler { + public: + using SharingHandler::validateUpdateData; + }; + MockSharingHandler sharingHandler; + UpdateData updateData; + sharingHandler.validateUpdateData(&updateData); +}