Skip to content

Commit ed0f218

Browse files
[openmp][amdgpu] Tear down amdgpu plugin accurately
Moves DeviceInfo global to heap to accurately control lifetime. Moves calls from libomptarget to deinit_plugin later, plugins need to stay alive until very shortly before libomptarget is destructed. Leaving the deinit_plugin calls where initially inserted hits use after free from the dynamic_module.c offloading test (verified with valgrind that the new location is sound with respect to this) Reviewed By: tianshilei1992 Differential Revision: https://reviews.llvm.org/D130714
1 parent e74197b commit ed0f218

File tree

2 files changed

+32
-14
lines changed

2 files changed

+32
-14
lines changed

openmp/libomptarget/plugins/amdgpu/src/rtl.cpp

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1113,10 +1113,21 @@ class RTLDeviceInfoTy : HSALifetime {
11131113

11141114
pthread_mutex_t SignalPoolT::mutex = PTHREAD_MUTEX_INITIALIZER;
11151115

1116-
// Putting accesses to DeviceInfo global behind a function call prior
1117-
// to changing to use init_plugin/deinit_plugin calls
1118-
static RTLDeviceInfoTy DeviceInfoState;
1119-
static RTLDeviceInfoTy& DeviceInfo() { return DeviceInfoState; }
1116+
static RTLDeviceInfoTy *DeviceInfoState = nullptr;
1117+
static RTLDeviceInfoTy &DeviceInfo() { return *DeviceInfoState; }
1118+
1119+
int32_t __tgt_rtl_init_plugin() {
1120+
DeviceInfoState = new RTLDeviceInfoTy;
1121+
return (DeviceInfoState && DeviceInfoState->ConstructionSucceeded)
1122+
? OFFLOAD_SUCCESS
1123+
: OFFLOAD_FAIL;
1124+
}
1125+
1126+
int32_t __tgt_rtl_deinit_plugin() {
1127+
if (DeviceInfoState)
1128+
delete DeviceInfoState;
1129+
return OFFLOAD_SUCCESS;
1130+
}
11201131

11211132
namespace {
11221133

@@ -2051,9 +2062,6 @@ int32_t __tgt_rtl_is_valid_binary_info(__tgt_device_image *image,
20512062
return true;
20522063
}
20532064

2054-
int32_t __tgt_rtl_init_plugin() { return OFFLOAD_SUCCESS; }
2055-
int32_t __tgt_rtl_deinit_plugin() { return OFFLOAD_SUCCESS; }
2056-
20572065
int __tgt_rtl_number_of_devices() {
20582066
// If the construction failed, no methods are safe to call
20592067
if (DeviceInfo().ConstructionSucceeded) {

openmp/libomptarget/src/rtl.cpp

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,23 @@ __attribute__((constructor(101))) void init() {
6767

6868
__attribute__((destructor(101))) void deinit() {
6969
DP("Deinit target library!\n");
70+
71+
for (auto *R : PM->RTLs.UsedRTLs) {
72+
// Plugins can either destroy their local state using global variables
73+
// or attribute(destructor) functions or by implementing deinit_plugin
74+
// The hazard with plugin local destructors is they may be called before
75+
// or after this destructor. If the plugin is destroyed using global
76+
// state before this library finishes calling into it the plugin is
77+
// likely to crash. If good fortune means the plugin outlives this
78+
// library then there may be no crash.
79+
// Using deinit_plugin and no global destructors from the plugin works.
80+
if (R->deinit_plugin) {
81+
if (R->deinit_plugin() != OFFLOAD_SUCCESS) {
82+
DP("Failure deinitializing RTL %s!\n", R->RTLName.c_str());
83+
}
84+
}
85+
}
86+
7087
delete PM;
7188

7289
#ifdef OMPTARGET_PROFILE_ENABLED
@@ -567,13 +584,6 @@ void RTLsTy::unregisterLib(__tgt_bin_desc *Desc) {
567584
PM->TblMapMtx.unlock();
568585

569586
// TODO: Write some RTL->unload_image(...) function?
570-
for (auto *R : UsedRTLs) {
571-
if (R->deinit_plugin) {
572-
if (R->deinit_plugin() != OFFLOAD_SUCCESS) {
573-
DP("Failure deinitializing RTL %s!\n", R->RTLName.c_str());
574-
}
575-
}
576-
}
577587

578588
DP("Done unregistering library!\n");
579589
}

0 commit comments

Comments
 (0)