From 05a9a528f75d3d37996290b828f634029daf0b53 Mon Sep 17 00:00:00 2001 From: "systems-assistant[bot]" <221163467+systems-assistant[bot]@users.noreply.github.com> Date: Tue, 2 Sep 2025 17:29:29 +0200 Subject: [PATCH] SWDEV-548482 - Address memory leaks in memory tests (#526) * SWDEV-548482 - Address memory leaks in memory tests * SWDEV-548482 - Added destroy calls * SWDEV-548482 - Address one more memory leak * SWDEV-548482 - Minor tweaks * SWDEV-548482 - Run clang-format * SWDEV-548482 - Add new lines * SWDEV-548482 - Run clang-format * SWDEV-548482 - Minor fix --------- Co-authored-by: Marko Arandjelovic --- .../memory/hipGetProcAddressMemoryApis.cc | 7 ++++- .../catch/unit/memory/hipHostRegister.cc | 4 ++- .../catch/unit/memory/hipHostUnregister.cc | 1 + .../catch/unit/memory/hipMallocManaged.cc | 3 +++ .../unit/memory/hipMallocManagedFlagsTst.cc | 17 +++++------- .../unit/memory/hipMallocMngdMultiThread.cc | 12 ++++++--- .../catch/unit/memory/hipMemAllocHost.cc | 3 ++- .../hipMemPoolExportToShareableHandle.cc | 2 ++ .../unit/memory/hipMemPoolImportPointer.cc | 1 + .../catch/unit/memory/hipMemPtrGetInfo.cc | 4 +++ .../memory/hipMemRangeGetAttribute_old.cc | 26 ++++++++++--------- 11 files changed, 51 insertions(+), 29 deletions(-) diff --git a/projects/hip-tests/catch/unit/memory/hipGetProcAddressMemoryApis.cc b/projects/hip-tests/catch/unit/memory/hipGetProcAddressMemoryApis.cc index dbef9de16f..4ff37b23bb 100644 --- a/projects/hip-tests/catch/unit/memory/hipGetProcAddressMemoryApis.cc +++ b/projects/hip-tests/catch/unit/memory/hipGetProcAddressMemoryApis.cc @@ -1121,6 +1121,7 @@ TEST_CASE("Unit_hipGetProcAddress_MemoryApisSetAndGetAttributes") { REQUIRE(allAttributesDataWithPtr.hostPointer == allAttributesData.hostPointer); REQUIRE(allAttributesDataWithPtr.isManaged == allAttributesData.isManaged); REQUIRE(allAttributesDataWithPtr.allocationFlags == allAttributesData.allocationFlags); + HIP_CHECK(hipFree(devPtr2)); } // Validating hipDrvPointerGetAttributes API @@ -1155,6 +1156,8 @@ TEST_CASE("Unit_hipGetProcAddress_MemoryApisSetAndGetAttributes") { REQUIRE(devicePointerWithPtr == devicePointer); REQUIRE(rangeSizeWithPtr == rangeSize); REQUIRE(startAddressWithPtr == startAddress); + + HIP_CHECK(hipFree(devPtr3)); } } @@ -5778,8 +5781,10 @@ TEST_CASE("Unit_hipGetProcAddress_MemoryApisStreamOrderedMemory") { size_t size = -1; HIP_CHECK(hipMemPtrGetInfo(dPtr, &size)); REQUIRE(size == 1024); - REQUIRE(dyn_hipMallocFromPoolAsync_ptr(&dPtr, 1, mem_pool, stream) == hipErrorOutOfMemory); + void* dPtr2 = nullptr; + REQUIRE(dyn_hipMallocFromPoolAsync_ptr(&dPtr2, 1, mem_pool, stream) == hipErrorOutOfMemory); + HIP_CHECK(dyn_hipFreeAsync_ptr(dPtr, stream)); HIP_CHECK(hipMemPoolDestroy(mem_pool)); HIP_CHECK(hipStreamDestroy(stream)); } diff --git a/projects/hip-tests/catch/unit/memory/hipHostRegister.cc b/projects/hip-tests/catch/unit/memory/hipHostRegister.cc index 5031320fc4..aa6dd03a52 100644 --- a/projects/hip-tests/catch/unit/memory/hipHostRegister.cc +++ b/projects/hip-tests/catch/unit/memory/hipHostRegister.cc @@ -234,7 +234,9 @@ TEMPLATE_TEST_CASE("Unit_hipHostRegister_DirectReferenceMultGpu", "", int, float HIP_CHECK(hipSetDevice(dev)); HIP_CHECK(hipGetDeviceProperties(&prop, dev)); std::string arch = prop.gcnArchName; - TEST_SKIP(arch, "Xnack+ is not supported. Skipping the test ...") + if (arch.find("xnack+") == std::string::npos) { + continue; // Skip if xnack is not supported + } // Register host memory for each device if (register_once == 0) { HIP_CHECK(hipHostRegister(A, sizeBytes, 0)); diff --git a/projects/hip-tests/catch/unit/memory/hipHostUnregister.cc b/projects/hip-tests/catch/unit/memory/hipHostUnregister.cc index a5bacd3ff0..c691e1c09d 100644 --- a/projects/hip-tests/catch/unit/memory/hipHostUnregister.cc +++ b/projects/hip-tests/catch/unit/memory/hipHostUnregister.cc @@ -72,6 +72,7 @@ TEST_CASE("Unit_hipHostUnregister_Ptr_Different_Than_Specified_To_Register") { std::vector alloc(2); HIP_CHECK(hipHostRegister(alloc.data(), alloc.size(), 0)); HIP_CHECK_ERROR(hipHostUnregister(&alloc.data()[1]), hipErrorHostMemoryNotRegistered); + HIP_CHECK(hipHostUnregister(alloc.data())); } TEST_CASE("Unit_hipHostUnregister_NotRegisteredPointer") { diff --git a/projects/hip-tests/catch/unit/memory/hipMallocManaged.cc b/projects/hip-tests/catch/unit/memory/hipMallocManaged.cc index 04f579f0fa..1f25f12dfa 100644 --- a/projects/hip-tests/catch/unit/memory/hipMallocManaged.cc +++ b/projects/hip-tests/catch/unit/memory/hipMallocManaged.cc @@ -70,6 +70,9 @@ TEST_CASE("Unit_hipMallocManaged_Basic") { HIP_CHECK(hipMallocManaged(&A, numElements * sizeof(float))); HIP_CHECK(hipMallocManaged(&B, numElements * sizeof(float))); HIP_CHECK(hipMallocManaged(&C, numElements * sizeof(float))); + HIP_CHECK(hipFree(A)); + HIP_CHECK(hipFree(B)); + HIP_CHECK(hipFree(C)); } /* diff --git a/projects/hip-tests/catch/unit/memory/hipMallocManagedFlagsTst.cc b/projects/hip-tests/catch/unit/memory/hipMallocManagedFlagsTst.cc index a0db16e192..4199837862 100644 --- a/projects/hip-tests/catch/unit/memory/hipMallocManagedFlagsTst.cc +++ b/projects/hip-tests/catch/unit/memory/hipMallocManagedFlagsTst.cc @@ -136,17 +136,14 @@ TEST_CASE("Unit_hipMallocManaged_AccessMultiStream") { } else { NumStrms = 4; } - hipStream_t** Stream = new hipStream_t*[NumStrms]; - for (int i = 0; i < NumStrms; ++i) { - Stream[i] = reinterpret_cast(malloc(sizeof(hipStream_t))); - } + std::vector streams(NumStrms); float *Ad = NULL, *Ah = NULL; Ah = new float[NUM_ELMS]; for (int i = 0; i < NumStrms; ++i) { if (MultiDevice >= 2) { HIP_CHECK(hipSetDevice(i)); } - HIP_CHECK(hipStreamCreate(Stream[i])); + HIP_CHECK(hipStreamCreate(&streams[i])); } HIP_CHECK(hipSetDevice(0)); // Testing hipMemAttachGlobal Flag @@ -168,8 +165,8 @@ TEST_CASE("Unit_hipMallocManaged_AccessMultiStream") { } HIP_CHECK(hipMalloc(&Ad, NUM_ELMS * sizeof(float))); HIP_CHECK(hipMemset(Ad, 0, NUM_ELMS * sizeof(float))); - MallcMangdFlgTst<<>>(NUM_ELMS, HmmAG, Ad); - HIP_CHECK(hipStreamSynchronize(*(Stream[i]))); + MallcMangdFlgTst<<>>(NUM_ELMS, HmmAG, Ad); + HIP_CHECK(hipStreamSynchronize(streams[i])); // Validating the results HIP_CHECK(hipMemcpy(Ah, Ad, NUM_ELMS * sizeof(float), hipMemcpyDeviceToHost)); for (int j = 0; j < NUM_ELMS; ++j) { @@ -203,8 +200,8 @@ TEST_CASE("Unit_hipMallocManaged_AccessMultiStream") { HIP_CHECK(hipSetDevice(i)); } HIP_CHECK(hipMemset(HmmAH2, 0, NUM_ELMS * sizeof(float))); - MallcMangdFlgTst<<>>(NUM_ELMS, HmmAH1, HmmAH2); - HIP_CHECK(hipStreamSynchronize(*(Stream[i]))); + MallcMangdFlgTst<<>>(NUM_ELMS, HmmAH1, HmmAH2); + HIP_CHECK(hipStreamSynchronize(streams[i])); for (int j = 0; j < NUM_ELMS; ++j) { if (HmmAH2[j] != (INIT_VAL * INIT_VAL)) { DataMismatch++; @@ -221,7 +218,7 @@ TEST_CASE("Unit_hipMallocManaged_AccessMultiStream") { HIP_CHECK(hipFree(HmmAH1)); HIP_CHECK(hipFree(HmmAH2)); for (int i = 0; i < NumStrms; ++i) { - HIP_CHECK(hipStreamDestroy(*(Stream[i]))); + HIP_CHECK(hipStreamDestroy(streams[i])); } REQUIRE(IfTestPassed); } diff --git a/projects/hip-tests/catch/unit/memory/hipMallocMngdMultiThread.cc b/projects/hip-tests/catch/unit/memory/hipMallocMngdMultiThread.cc index 4482c5e352..79f508a385 100644 --- a/projects/hip-tests/catch/unit/memory/hipMallocMngdMultiThread.cc +++ b/projects/hip-tests/catch/unit/memory/hipMallocMngdMultiThread.cc @@ -84,11 +84,12 @@ static void LaunchKrnl(int* Hmm1, size_t NumElms, int InitVal, int GpuOrdnl, int } static void LaunchKrnl2(int* Hmm, size_t NumElms, int InitVal, int HmmMem) { - int *ptr = nullptr, blockSize = 64, *HstPtr = nullptr; + int *ptr = nullptr, blockSize = 64; + std::unique_ptr host_ptr; hipStream_t strm; HIPCHECK(hipStreamCreate(&strm)); if (HmmMem == 0) { - HstPtr = reinterpret_cast(new int[NumElms]); + host_ptr = std::make_unique(NumElms); HIPCHECK(hipMalloc(&ptr, (sizeof(int) * NumElms))); } else { HIPCHECK(hipMallocManaged(&ptr, (sizeof(int) * NumElms))); @@ -102,9 +103,9 @@ static void LaunchKrnl2(int* Hmm, size_t NumElms, int InitVal, int HmmMem) { // Verifying the result int DataMismatch = 0; if (HmmMem == 0) { - HIPCHECK(hipMemcpy(HstPtr, ptr, (sizeof(int) * NumElms), hipMemcpyDeviceToHost)); + HIPCHECK(hipMemcpy(host_ptr.get(), ptr, (sizeof(int) * NumElms), hipMemcpyDeviceToHost)); for (size_t i = 0; i < NumElms; ++i) { - if (HstPtr[i] != (InitVal + 10)) { + if (host_ptr[i] != (InitVal + 10)) { DataMismatch++; } } @@ -418,6 +419,7 @@ TEST_CASE("Unit_hipMallocManaged_MultiKrnlComnHmm") { } } delete[] HstPtr; + HIP_CHECK(hipFree(Hmm)); } @@ -481,6 +483,8 @@ TEST_CASE("Unit_hipMallocManaged_MultiThrdMultiStrm") { thr.join(); } } + + HIP_CHECK(hipFree(Hmm1)); } diff --git a/projects/hip-tests/catch/unit/memory/hipMemAllocHost.cc b/projects/hip-tests/catch/unit/memory/hipMemAllocHost.cc index 8c121aff31..50cfa5bf25 100644 --- a/projects/hip-tests/catch/unit/memory/hipMemAllocHost.cc +++ b/projects/hip-tests/catch/unit/memory/hipMemAllocHost.cc @@ -125,8 +125,9 @@ TEST_CASE("Unit_hipMemAllocHost_VerifyAccess") { HIP_CHECK(hipDeviceSynchronize()); } - for (int device_index = 1; device_index < devices_number; device_index++) { + for (int device_index = 0; device_index < devices_number; device_index++) { REQUIRE(*devices_memories[device_index] == device_index); + HIP_CHECK(hipFree(devices_memories[device_index])); HIP_CHECK(hipCtxDestroy(devices_ctxs[device_index])); } } diff --git a/projects/hip-tests/catch/unit/memory/hipMemPoolExportToShareableHandle.cc b/projects/hip-tests/catch/unit/memory/hipMemPoolExportToShareableHandle.cc index 27a30305cb..142b62bf8c 100644 --- a/projects/hip-tests/catch/unit/memory/hipMemPoolExportToShareableHandle.cc +++ b/projects/hip-tests/catch/unit/memory/hipMemPoolExportToShareableHandle.cc @@ -107,10 +107,12 @@ TEST_CASE("Unit_hipMemPoolExportToShareableHandle_SameProc") { HIP_CHECK(hipMemcpyAsync(B_h.data(), ptrImp, byte_size, hipMemcpyDeviceToHost, stream)); HIP_CHECK(hipStreamSynchronize(stream)); REQUIRE(true == std::equal(B_h.begin(), B_h.end(), C_h.data())); + HIP_CHECK(hipFree(ptrImp)); HIP_CHECK(hipFreeAsync(reinterpret_cast(A_d), stream)); HIP_CHECK(hipStreamSynchronize(stream)); HIP_CHECK(hipStreamDestroy(stream)); HIP_CHECK(hipMemPoolDestroy(mempool)); + HIP_CHECK(hipMemPoolDestroy(mempoolImp)); } /** diff --git a/projects/hip-tests/catch/unit/memory/hipMemPoolImportPointer.cc b/projects/hip-tests/catch/unit/memory/hipMemPoolImportPointer.cc index 7356afc090..2ad37c7272 100644 --- a/projects/hip-tests/catch/unit/memory/hipMemPoolImportPointer.cc +++ b/projects/hip-tests/catch/unit/memory/hipMemPoolImportPointer.cc @@ -81,6 +81,7 @@ TEST_CASE("Unit_hipMemPoolImportPointer_Negative") { } HIP_CHECK(hipFree(reinterpret_cast(A_d))); HIP_CHECK(hipMemPoolDestroy(mempoolPfd)); + HIP_CHECK(hipMemPoolDestroy(mempoolImp)); } /** diff --git a/projects/hip-tests/catch/unit/memory/hipMemPtrGetInfo.cc b/projects/hip-tests/catch/unit/memory/hipMemPtrGetInfo.cc index a9307d0c7b..c09cdd3d73 100644 --- a/projects/hip-tests/catch/unit/memory/hipMemPtrGetInfo.cc +++ b/projects/hip-tests/catch/unit/memory/hipMemPtrGetInfo.cc @@ -51,4 +51,8 @@ TEST_CASE("Unit_hipMemPtrGetInfo_Basic") { REQUIRE(sGetSize == sSetSize); HIP_CHECK(hipMemPtrGetInfo(sPtr, &sGetSize)); REQUIRE(sGetSize == sSetSize); + + HIP_CHECK(hipFree(iPtr)); + HIP_CHECK(hipFree(fPtr)); + HIP_CHECK(hipFree(sPtr)); } diff --git a/projects/hip-tests/catch/unit/memory/hipMemRangeGetAttribute_old.cc b/projects/hip-tests/catch/unit/memory/hipMemRangeGetAttribute_old.cc index d15436b65f..f2d29e041b 100644 --- a/projects/hip-tests/catch/unit/memory/hipMemRangeGetAttribute_old.cc +++ b/projects/hip-tests/catch/unit/memory/hipMemRangeGetAttribute_old.cc @@ -153,10 +153,12 @@ TEST_CASE("Unit_hipMemRangeGetAttribute_NegativeTests") { int NumDevs; HIP_CHECK(hipGetDeviceCount(&NumDevs)); int data = RND_NUM; - int* OutData = new int[NumDevs]; - for (int m = 0; m < NumDevs; ++m) { - OutData[m] = RND_NUM; + + auto out_data = std::make_unique(NumDevs); + for (int i = 0; i < NumDevs; ++i) { + out_data[i] = RND_NUM; } + HIP_CHECK(hipMallocManaged(&devPtr, MEM_SIZE, hipMemAttachGlobal)); HIP_CHECK(hipMemAdvise(devPtr, MEM_SIZE, hipMemAdviseSetReadMostly, 0)); @@ -168,21 +170,21 @@ TEST_CASE("Unit_hipMemRangeGetAttribute_NegativeTests") { } // checking the behavior with dataSize > 4 and even SECTION("checking the behavior with dataSize > 4 and even") { - REQUIRE(CheckError( - hipMemRangeGetAttribute(OutData, 6, hipMemRangeAttributeReadMostly, devPtr, MEM_SIZE), - __LINE__)); + REQUIRE(CheckError(hipMemRangeGetAttribute(out_data.get(), 6, hipMemRangeAttributeReadMostly, + devPtr, MEM_SIZE), + __LINE__)); } // checking the behavior with dataSize > 4 and odd SECTION("checking the behavior with dataSize > 4 and odd") { - REQUIRE(CheckError( - hipMemRangeGetAttribute(OutData, 7, hipMemRangeAttributeReadMostly, devPtr, MEM_SIZE), - __LINE__)); + REQUIRE(CheckError(hipMemRangeGetAttribute(out_data.get(), 7, hipMemRangeAttributeReadMostly, + devPtr, MEM_SIZE), + __LINE__)); } // checking the behavior with dataSize which is not multiple of 4 SECTION("checking the behavior with dataSize which is not multiple of 4") { - REQUIRE(CheckError( - hipMemRangeGetAttribute(OutData, 27, hipMemRangeAttributeReadMostly, devPtr, MEM_SIZE), - __LINE__)); + REQUIRE(CheckError(hipMemRangeGetAttribute(out_data.get(), 27, hipMemRangeAttributeReadMostly, + devPtr, MEM_SIZE), + __LINE__)); } // checking the behaviour with devPtr(4th param) as NULL SECTION("checking the behaviour with devPtr(4th param) as NULL") {