From 69431da0c73ccb4f7887ade8a92c877da4910f76 Mon Sep 17 00:00:00 2001 From: Russ Butler Date: Fri, 26 Aug 2016 15:53:43 -0500 Subject: [PATCH 1/2] Add a test to check for init race conditions Test SingletonPtr initailization along the with lazy initailization that is built into the C++ language itself. --- TESTS/mbed_drivers/race_test/main.cpp | 134 ++++++++++++++++++++++++++ 1 file changed, 134 insertions(+) create mode 100644 TESTS/mbed_drivers/race_test/main.cpp diff --git a/TESTS/mbed_drivers/race_test/main.cpp b/TESTS/mbed_drivers/race_test/main.cpp new file mode 100644 index 00000000000..006c9a12b2f --- /dev/null +++ b/TESTS/mbed_drivers/race_test/main.cpp @@ -0,0 +1,134 @@ +/* + * Copyright (c) 2016-2016, ARM Limited, All Rights Reserved + * SPDX-License-Identifier: Apache-2.0 + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +#include "mbed.h" +#include "rtos.h" +#include "greentea-client/test_env.h" +#include "unity/unity.h" +#include "utest/utest.h" +#include "SingletonPtr.h" +#include + +using namespace utest::v1; + +#define TEST_STACK_SIZE 1024 +static uint32_t instance_count = 0; + +class TestClass { +public: + TestClass() { + printf("TestClass ctor start\r\n"); + Thread::wait(500); + instance_count++; + printf("TestClass ctor end\r\n"); + } + + void do_something() { + printf("Do something called\r\n"); + } + + ~TestClass() { + instance_count--; + } +}; + +static TestClass* get_test_class() +{ + static TestClass tc; + return &tc; +} + +static SingletonPtr test_class; + +static void main_func_race() +{ + get_test_class(); +} + +static void main_class_race() +{ + test_class->do_something(); +} + +void test_case_func_race() +{ + printf("Running function race test\r\n"); + Callback cb(main_func_race); + Thread *t1 = new Thread(osPriorityNormal, TEST_STACK_SIZE); + Thread *t2 = new Thread(osPriorityNormal, TEST_STACK_SIZE); + + // Start start first thread + t1->start(cb); + // Start second thread while the first is inside the constructor + Thread::wait(250); + t2->start(cb); + + // Wait for the threads to finish + t1->join(); + t2->join(); + + delete t1; + delete t2; + + TEST_ASSERT_EQUAL_UINT32(1, instance_count); + + // Reset instance count + instance_count = 0; +} + +void test_case_class_race() +{ + printf("Running class race test\r\n"); + Callback cb(main_class_race); + Thread *t1 = new Thread(osPriorityNormal, TEST_STACK_SIZE); + Thread *t2 = new Thread(osPriorityNormal, TEST_STACK_SIZE); + + // Start start first thread + t1->start(cb); + // Start second thread while the first is inside the constructor + Thread::wait(250); + t2->start(cb); + + // Wait for the threads to finish + t1->join(); + t2->join(); + + delete t1; + delete t2; + + TEST_ASSERT_EQUAL_UINT32(1, instance_count); + + // Reset instance count + instance_count = 0; +} + +Case cases[] = { + Case("function init race", test_case_func_race), + Case("class init race", test_case_class_race), +}; + +utest::v1::status_t greentea_test_setup(const size_t number_of_cases) +{ + GREENTEA_SETUP(20, "default_auto"); + return greentea_test_setup_handler(number_of_cases); +} + +Specification specification(greentea_test_setup, cases, greentea_test_teardown_handler); + +int main() +{ + Harness::run(specification); +} From ef45ef8dae94b655c0d51a38cde03e6c0153a893 Mon Sep 17 00:00:00 2001 From: Russ Butler Date: Fri, 26 Aug 2016 16:05:58 -0500 Subject: [PATCH 2/2] Fix GCC locks for lazy object initailization Implement the functions __cxa_guard_acquire, __cxa_guard_release and __cxa_guard_abort so lazily initialized function-local static objects are done so in a thread safe manner in GCC. --- hal/common/retarget.cpp | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/hal/common/retarget.cpp b/hal/common/retarget.cpp index 16c4c582210..928c3324916 100644 --- a/hal/common/retarget.cpp +++ b/hal/common/retarget.cpp @@ -713,6 +713,43 @@ extern "C" void __env_unlock( struct _reent *_r ) { __rtos_env_unlock(_r); } + +#define CXA_GUARD_INIT_DONE (1 << 0) +#define CXA_GUARD_INIT_IN_PROGRESS (1 << 1) +#define CXA_GUARD_MASK (CXA_GUARD_INIT_DONE | CXA_GUARD_INIT_IN_PROGRESS) + +extern "C" int __cxa_guard_acquire(int *guard_object_p) +{ + uint8_t *guard_object = (uint8_t *)guard_object_p; + if (CXA_GUARD_INIT_DONE == (*guard_object & CXA_GUARD_MASK)) { + return 0; + } + singleton_lock(); + if (CXA_GUARD_INIT_DONE == (*guard_object & CXA_GUARD_MASK)) { + singleton_unlock(); + return 0; + } + MBED_ASSERT(0 == (*guard_object & CXA_GUARD_MASK)); + *guard_object = *guard_object | CXA_GUARD_INIT_IN_PROGRESS; + return 1; +} + +extern "C" void __cxa_guard_release(int *guard_object_p) +{ + uint8_t *guard_object = (uint8_t *)guard_object_p; + MBED_ASSERT(CXA_GUARD_INIT_IN_PROGRESS == (*guard_object & CXA_GUARD_MASK)); + *guard_object = (*guard_object & ~CXA_GUARD_MASK) | CXA_GUARD_INIT_DONE; + singleton_unlock(); +} + +extern "C" void __cxa_guard_abort(int *guard_object_p) +{ + uint8_t *guard_object = (uint8_t *)guard_object_p; + MBED_ASSERT(CXA_GUARD_INIT_IN_PROGRESS == (*guard_object & CXA_GUARD_MASK)); + *guard_object = *guard_object & ~CXA_GUARD_INIT_IN_PROGRESS; + singleton_unlock(); +} + #endif void *operator new(std::size_t count)