Skip to content

Commit

Permalink
[LIBC] Fix incorrect behavior with pthread_key_t when value was nullptr
Browse files Browse the repository at this point in the history
We should not call destructor if value is nullptr.

Reviewed By: sivachandra

Differential Revision: https://reviews.llvm.org/D148291
  • Loading branch information
goldsteinn committed Apr 20, 2023
1 parent de27291 commit 6f1a9ed
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 7 deletions.
3 changes: 2 additions & 1 deletion libc/src/__support/threads/thread.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,8 @@ void call_atexit_callbacks(ThreadAttributes *attrib) {
attrib->atexit_callback_mgr->call();
for (size_t i = 0; i < TSS_KEY_COUNT; ++i) {
TSSValueUnit &unit = tss_values[i];
if (unit.dtor != nullptr)
// Both dtor and value need to nonnull to call dtor
if (unit.dtor != nullptr && unit.payload != nullptr)
unit.dtor(unit.payload);
}
}
Expand Down
35 changes: 29 additions & 6 deletions libc/test/integration/src/pthread/pthread_tss_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,16 +21,19 @@ static constexpr int THREAD_DATA_INITVAL = 0x1234;
static constexpr int THREAD_DATA_FINIVAL = 0x4321;
static constexpr int THREAD_RUN_VAL = 0x600D;

int child_thread_data = THREAD_DATA_INITVAL;
int main_thread_data = THREAD_DATA_INITVAL;
static int child_thread_data = THREAD_DATA_INITVAL;
static int main_thread_data = THREAD_DATA_INITVAL;

pthread_key_t key;
void dtor(void *data) {
static pthread_key_t key;
static void dtor(void *data) {
auto *v = reinterpret_cast<int *>(data);
*v = THREAD_DATA_FINIVAL;
}

void *func(void *obj) {
// Used to test that we don't call the destructor when the mapped value in NULL.
static void dtor_failure(void *) { ASSERT_TRUE(false); }

static void *func(void *obj) {
ASSERT_EQ(__llvm_libc::pthread_setspecific(key, &child_thread_data), 0);
int *d = reinterpret_cast<int *>(__llvm_libc::pthread_getspecific(key));
ASSERT_TRUE(d != nullptr);
Expand All @@ -40,7 +43,14 @@ void *func(void *obj) {
return nullptr;
}

TEST_MAIN() {
static void *func_null_val(void *) {
// null value, we should not call dtor
ASSERT_EQ(__llvm_libc::pthread_setspecific(key, nullptr), 0);
ASSERT_EQ(__llvm_libc::pthread_getspecific(key), nullptr);
return nullptr;
}

static void standard_usage_test() {
ASSERT_EQ(__llvm_libc::pthread_key_create(&key, &dtor), 0);
ASSERT_EQ(__llvm_libc::pthread_setspecific(key, &main_thread_data), 0);
int *d = reinterpret_cast<int *>(__llvm_libc::pthread_getspecific(key));
Expand All @@ -56,7 +66,20 @@ TEST_MAIN() {
ASSERT_EQ(retval, nullptr);
ASSERT_EQ(arg, THREAD_RUN_VAL);
ASSERT_EQ(child_thread_data, THREAD_DATA_FINIVAL);
ASSERT_EQ(__llvm_libc::pthread_key_delete(key), 0);
}

static void null_value_test() {
pthread_t th;
ASSERT_EQ(__llvm_libc::pthread_key_create(&key, &dtor_failure), 0);
ASSERT_EQ(__llvm_libc::pthread_create(&th, nullptr, &func_null_val, nullptr),
0);
ASSERT_EQ(__llvm_libc::pthread_join(th, nullptr), 0);
ASSERT_EQ(__llvm_libc::pthread_key_delete(key), 0);
}

TEST_MAIN() {
standard_usage_test();
null_value_test();
return 0;
}

0 comments on commit 6f1a9ed

Please sign in to comment.