Skip to content

Commit c90a53c

Browse files
authored
[DevTSAN] Fix UR objects leak problem (#18880)
We forgot to destroy tsan interceptor, this will cause objects leak issue.
1 parent 503494f commit c90a53c

File tree

6 files changed

+96
-7
lines changed

6 files changed

+96
-7
lines changed

unified-runtime/source/loader/layers/sanitizer/tsan/tsan_ddi.cpp

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,32 @@ ur_result_t setupContext(ur_context_handle_t Context, uint32_t numDevices,
4343

4444
} // namespace
4545

46+
///////////////////////////////////////////////////////////////////////////////
47+
/// @brief Intercept function for urAdapterGet
48+
ur_result_t urAdapterGet(
49+
/// [in] the number of adapters to be added to phAdapters. If phAdapters
50+
/// is not NULL, then NumEntries should be greater than zero, otherwise
51+
/// ::UR_RESULT_ERROR_INVALID_SIZE, will be returned.
52+
uint32_t NumEntries,
53+
/// [out][optional][range(0, NumEntries)] array of handle of adapters. If
54+
/// NumEntries is less than the number of adapters available, then
55+
/// ::urAdapterGet shall only retrieve that number of platforms.
56+
ur_adapter_handle_t *phAdapters,
57+
/// [out][optional] returns the total number of adapters available.
58+
uint32_t *pNumAdapters) {
59+
auto pfnAdapterGet = getContext()->urDdiTable.Adapter.pfnGet;
60+
61+
ur_result_t result = pfnAdapterGet(NumEntries, phAdapters, pNumAdapters);
62+
if (result == UR_RESULT_SUCCESS && phAdapters) {
63+
const uint32_t NumAdapters = pNumAdapters ? *pNumAdapters : NumEntries;
64+
for (uint32_t i = 0; i < NumAdapters; ++i) {
65+
UR_CALL(getTsanInterceptor()->holdAdapter(phAdapters[i]));
66+
}
67+
}
68+
69+
return result;
70+
}
71+
4672
///////////////////////////////////////////////////////////////////////////////
4773
/// @brief Intercept function for urContextCreate
4874
__urdlllocal ur_result_t UR_APICALL urContextCreate(
@@ -1201,6 +1227,27 @@ ur_result_t urCheckVersion(ur_api_version_t version) {
12011227
return UR_RESULT_SUCCESS;
12021228
}
12031229

1230+
///////////////////////////////////////////////////////////////////////////////
1231+
/// @brief Exported function for filling application's Adapter table
1232+
/// with current process' addresses
1233+
///
1234+
/// @returns
1235+
/// - ::UR_RESULT_SUCCESS
1236+
/// - ::UR_RESULT_ERROR_UNINITIALIZED
1237+
/// - ::UR_RESULT_ERROR_INVALID_NULL_POINTER
1238+
/// - ::UR_RESULT_ERROR_UNSUPPORTED_VERSION
1239+
ur_result_t urGetAdapterProcAddrTable(
1240+
/// [in,out] pointer to table of DDI function pointers
1241+
ur_adapter_dditable_t *pDdiTable) {
1242+
if (nullptr == pDdiTable) {
1243+
return UR_RESULT_ERROR_INVALID_NULL_POINTER;
1244+
}
1245+
1246+
pDdiTable->pfnGet = ur_sanitizer_layer::tsan::urAdapterGet;
1247+
1248+
return UR_RESULT_SUCCESS;
1249+
}
1250+
12041251
///////////////////////////////////////////////////////////////////////////////
12051252
/// @brief Exported function for filling application's Context table
12061253
/// with current process' addresses
@@ -1381,6 +1428,11 @@ ur_result_t initTsanDDITable(ur_dditable_t *dditable) {
13811428
result = ur_sanitizer_layer::tsan::urCheckVersion(UR_API_VERSION_CURRENT);
13821429
}
13831430

1431+
if (UR_RESULT_SUCCESS == result) {
1432+
result =
1433+
ur_sanitizer_layer::tsan::urGetAdapterProcAddrTable(&dditable->Adapter);
1434+
}
1435+
13841436
if (UR_RESULT_SUCCESS == result) {
13851437
result =
13861438
ur_sanitizer_layer::tsan::urGetContextProcAddrTable(&dditable->Context);

unified-runtime/source/loader/layers/sanitizer/tsan/tsan_interceptor.cpp

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,23 @@ void ContextInfo::insertAllocInfo(ur_device_handle_t Device, TsanAllocInfo AI) {
103103
}
104104
}
105105

106+
TsanInterceptor::~TsanInterceptor() {
107+
// We must release these objects before releasing adapters, since
108+
// they may use the adapter in their destructor
109+
for (const auto &[_, DeviceInfo] : m_DeviceMap) {
110+
DeviceInfo->Shadow->Destroy();
111+
DeviceInfo->Shadow = nullptr;
112+
}
113+
114+
m_MemBufferMap.clear();
115+
m_KernelMap.clear();
116+
m_ContextMap.clear();
117+
118+
for (auto Adapter : m_Adapters) {
119+
getContext()->urDdiTable.Adapter.pfnRelease(Adapter);
120+
}
121+
}
122+
106123
ur_result_t TsanInterceptor::allocateMemory(ur_context_handle_t Context,
107124
ur_device_handle_t Device,
108125
const ur_usm_desc_t *Properties,

unified-runtime/source/loader/layers/sanitizer/tsan/tsan_interceptor.hpp

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,9 @@
2020
#include "tsan_shadow.hpp"
2121
#include "ur_sanitizer_layer.hpp"
2222

23+
#include <unordered_map>
24+
#include <unordered_set>
25+
2326
namespace ur_sanitizer_layer {
2427
namespace tsan {
2528

@@ -176,6 +179,8 @@ struct LaunchInfo {
176179

177180
class TsanInterceptor {
178181
public:
182+
~TsanInterceptor();
183+
179184
ur_result_t allocateMemory(ur_context_handle_t Context,
180185
ur_device_handle_t Device,
181186
const ur_usm_desc_t *Properties,
@@ -202,6 +207,16 @@ class TsanInterceptor {
202207

203208
std::shared_ptr<MemBuffer> getMemBuffer(ur_mem_handle_t MemHandle);
204209

210+
ur_result_t holdAdapter(ur_adapter_handle_t Adapter) {
211+
std::scoped_lock<ur_shared_mutex> Guard(m_AdaptersMutex);
212+
if (m_Adapters.find(Adapter) != m_Adapters.end()) {
213+
return UR_RESULT_SUCCESS;
214+
}
215+
UR_CALL(getContext()->urDdiTable.Adapter.pfnRetain(Adapter));
216+
m_Adapters.insert(Adapter);
217+
return UR_RESULT_SUCCESS;
218+
}
219+
205220
ur_result_t preLaunchKernel(ur_kernel_handle_t Kernel,
206221
ur_queue_handle_t Queue, LaunchInfo &LaunchInfo);
207222

@@ -255,6 +270,9 @@ class TsanInterceptor {
255270
std::unordered_map<ur_mem_handle_t, std::shared_ptr<MemBuffer>>
256271
m_MemBufferMap;
257272
ur_shared_mutex m_MemBufferMapMutex;
273+
274+
std::unordered_set<ur_adapter_handle_t> m_Adapters;
275+
ur_shared_mutex m_AdaptersMutex;
258276
};
259277

260278
} // namespace tsan

unified-runtime/source/loader/layers/sanitizer/tsan/tsan_shadow.cpp

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ ur_result_t ShadowMemoryCPU::Setup() {
4747
return URes;
4848
}
4949

50-
ur_result_t ShadowMemoryCPU::Destory() {
50+
ur_result_t ShadowMemoryCPU::Destroy() {
5151
if (ShadowBegin == 0 && ShadowEnd == 0)
5252
return UR_RESULT_SUCCESS;
5353
static ur_result_t URes = [this]() {
@@ -98,12 +98,10 @@ ur_result_t ShadowMemoryGPU::Setup() {
9898
return Result;
9999
}
100100
ShadowEnd = ShadowBegin + ShadowSize;
101-
// Retain the context which reserves shadow memory
102-
getContext()->urDdiTable.Context.pfnRetain(Context);
103101
return UR_RESULT_SUCCESS;
104102
}
105103

106-
ur_result_t ShadowMemoryGPU::Destory() {
104+
ur_result_t ShadowMemoryGPU::Destroy() {
107105
if (ShadowBegin == 0) {
108106
return UR_RESULT_SUCCESS;
109107
}

unified-runtime/source/loader/layers/sanitizer/tsan/tsan_shadow.hpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ struct ShadowMemory {
2828

2929
virtual ur_result_t Setup() = 0;
3030

31-
virtual ur_result_t Destory() = 0;
31+
virtual ur_result_t Destroy() = 0;
3232

3333
virtual RawShadow *MemToShadow(uptr Ptr) = 0;
3434

@@ -69,7 +69,7 @@ struct ShadowMemoryCPU final : public ShadowMemory {
6969

7070
ur_result_t Setup() override;
7171

72-
ur_result_t Destory() override;
72+
ur_result_t Destroy() override;
7373

7474
RawShadow *MemToShadow(uptr Ptr) override;
7575

@@ -92,7 +92,7 @@ struct ShadowMemoryGPU : public ShadowMemory {
9292

9393
ur_result_t Setup() override;
9494

95-
ur_result_t Destory() override;
95+
ur_result_t Destroy() override;
9696

9797
ur_result_t CleanShadow(ur_queue_handle_t Queue, uptr Ptr,
9898
uptr Size) override;

unified-runtime/source/loader/layers/sanitizer/ur_sanitizer_layer.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
#include "ur_sanitizer_layer.hpp"
1515
#include "asan/asan_ddi.hpp"
1616
#include "msan/msan_ddi.hpp"
17+
#include "tsan/tsan_ddi.hpp"
1718

1819
namespace ur_sanitizer_layer {
1920
context_t *getContext() {
@@ -39,6 +40,9 @@ ur_result_t context_t::tearDown() {
3940
case SanitizerType::MemorySanitizer:
4041
destroyMsanInterceptor();
4142
break;
43+
case SanitizerType::ThreadSanitizer:
44+
destroyTsanInterceptor();
45+
break;
4246
default:
4347
break;
4448
}

0 commit comments

Comments
 (0)