From c7ee1d6574ca5d95869152c5f8e4e6d02a7fa6bc Mon Sep 17 00:00:00 2001 From: John Wellbelove Date: Tue, 24 Nov 2020 19:20:24 +0000 Subject: [PATCH] Fixed non-initialisation of in_use flag. --- .github/workflows/clang.yml | 62 +++- include/etl/buffer_descriptors.h | 12 +- test/test_buffer_descriptors.cpp | 572 ++++++++++++++++--------------- 3 files changed, 350 insertions(+), 296 deletions(-) diff --git a/.github/workflows/clang.yml b/.github/workflows/clang.yml index ea88089a..35b012c1 100644 --- a/.github/workflows/clang.yml +++ b/.github/workflows/clang.yml @@ -1,11 +1,50 @@ name: clang on: push: - branches: [ hotfix/clang-ci-unit-test-crash ] + branches: [ master ] pull_request: branches: [ master ] jobs: + build-clang-9-linux: + name: Clang-9 Linux + runs-on: ${{ matrix.os }} + strategy: + matrix: + os: [ubuntu-20.04] + + steps: + - uses: actions/checkout@v2 + + - name: Build + run: | + export CC=clang-9 + export CXX=clang++-9 + cmake -D BUILD_TESTS=ON ./ + clang --version + make + + - name: Run tests + run: ./test/etl_tests + + build-clang-9-linux-no-stl: + name: Clang-9 Linux - No STL + runs-on: ${{ matrix.os }} + strategy: + matrix: + os: [ubuntu-20.04] + + steps: + - uses: actions/checkout@v2 + + - name: Build + run: | + cmake -DBUILD_TESTS=ON -DNO_STL=ON ./ + gcc --version + make + + - name: Run tests + run: ./test/etl_tests build-clang-10-osx: name: Clang-10 OSX @@ -27,3 +66,24 @@ jobs: - name: Run tests run: ./test/etl_tests + + build-clang-10-osx-no-stl: + name: Clang-10 OSX - No STL + runs-on: ${{ matrix.os }} + strategy: + matrix: + os: [macos-10.15] + + steps: + - uses: actions/checkout@v2 + + - name: Build + run: | + export CC=clang + export CXX=clang++ + cmake -D BUILD_TESTS=ON -DNO_STL=ON ./ + clang --version + make + + - name: Run tests + run: ./test/etl_tests diff --git a/include/etl/buffer_descriptors.h b/include/etl/buffer_descriptors.h index d3c94789..78076442 100644 --- a/include/etl/buffer_descriptors.h +++ b/include/etl/buffer_descriptors.h @@ -197,17 +197,7 @@ namespace etl typedef etl::delegate callback_type; //********************************* - buffer_descriptors(TBuffer* pbuffers_) - { - for (size_t i = 0U; i < N_BUFFERS; ++i) - { - descriptor_items[i].pbuffer = pbuffers_ + (i * BUFFER_SIZE); - descriptor_items[i].in_use = false; - } - } - - //********************************* - buffer_descriptors(TBuffer* pbuffers_, const callback_type& callback_) + buffer_descriptors(TBuffer* pbuffers_, callback_type callback_ = callback_type()) : callback(callback_) { for (size_t i = 0U; i < N_BUFFERS; ++i) diff --git a/test/test_buffer_descriptors.cpp b/test/test_buffer_descriptors.cpp index 7ed1e645..cb82c6e3 100644 --- a/test/test_buffer_descriptors.cpp +++ b/test/test_buffer_descriptors.cpp @@ -41,7 +41,7 @@ SOFTWARE. #include "etl/buffer_descriptors.h" #if defined(ETL_TARGET_OS_WINDOWS) -#include + #include #endif #define REALTIME_TEST 0 @@ -49,8 +49,8 @@ SOFTWARE. namespace { constexpr size_t BUFFER_SIZE = 16U; - constexpr size_t N_BUFFERS = 4U; - constexpr size_t DATA_COUNT = BUFFER_SIZE / 2; + constexpr size_t N_BUFFERS = 4U; + constexpr size_t DATA_COUNT = BUFFER_SIZE / 2; using BD = etl::buffer_descriptors; @@ -62,13 +62,13 @@ namespace void receive(BD::notification n) { pbuffer = n.get_descriptor().data(); - count = n.get_count(); + count = n.get_count(); } void clear() { pbuffer = nullptr; - count = 0U; + count = 0U; } BD::pointer pbuffer; @@ -79,42 +79,42 @@ namespace SUITE(test_buffer_descriptors) { - ////************************************************************************* - //TEST(test_constructor_plus_buffer) - //{ - // BD bd(&buffers[0][0]); + //************************************************************************* + TEST(test_constructor_plus_buffer) + { + BD bd(&buffers[0][0]); - // CHECK_EQUAL(N_BUFFERS, bd.N_BUFFERS); - // CHECK_EQUAL(BUFFER_SIZE, bd.BUFFER_SIZE); - // CHECK(!bd.is_valid()); - //} + CHECK_EQUAL(N_BUFFERS, bd.N_BUFFERS); + CHECK_EQUAL(BUFFER_SIZE, bd.BUFFER_SIZE); + CHECK(!bd.is_valid()); + } - ////************************************************************************* - //TEST(test_constructor_plus_buffer_and_callback) - //{ - // receiver.clear(); - // BD::callback_type callback = BD::callback_type::create(receiver); + //************************************************************************* + TEST(test_constructor_plus_buffer_and_callback) + { + receiver.clear(); + BD::callback_type callback = BD::callback_type::create(receiver); - // BD bd(&buffers[0][0], callback); + BD bd(&buffers[0][0], callback); - // CHECK_EQUAL(N_BUFFERS, bd.N_BUFFERS); - // CHECK_EQUAL(BUFFER_SIZE, bd.BUFFER_SIZE); - // CHECK(bd.is_valid()); - //} + CHECK_EQUAL(N_BUFFERS, bd.N_BUFFERS); + CHECK_EQUAL(BUFFER_SIZE, bd.BUFFER_SIZE); + CHECK(bd.is_valid()); + } - ////************************************************************************* - //TEST(test_constructor_plus_buffer_set_callback) - //{ - // receiver.clear(); - // BD::callback_type callback = BD::callback_type::create(receiver); + //************************************************************************* + TEST(test_constructor_plus_buffer_set_callback) + { + receiver.clear(); + BD::callback_type callback = BD::callback_type::create(receiver); - // BD bd(&buffers[0][0]); - // bd.set_callback(callback); + BD bd(&buffers[0][0]); + bd.set_callback(callback); - // CHECK_EQUAL(N_BUFFERS, bd.N_BUFFERS); - // CHECK_EQUAL(BUFFER_SIZE, bd.BUFFER_SIZE); - // CHECK(bd.is_valid()); - //} + CHECK_EQUAL(N_BUFFERS, bd.N_BUFFERS); + CHECK_EQUAL(BUFFER_SIZE, bd.BUFFER_SIZE); + CHECK(bd.is_valid()); + } //************************************************************************* TEST(test_buffers) @@ -125,258 +125,262 @@ namespace { BD::descriptor desc = bd.allocate(); + CHECK(desc.is_valid()); + CHECK(desc.is_allocated()); + CHECK(!desc.is_released()); CHECK_EQUAL(BUFFER_SIZE, desc.max_size()); + CHECK_EQUAL(uintptr_t(&buffers[i][0]), uintptr_t(desc.data())); } } - // - // //************************************************************************* - // TEST(test_buffers_with_allocate_fill) - // { - // std::array test = - // { - // char(0xFF), char(0xFF), char(0xFF), char(0xFF), char(0xFF), char(0xFF), char(0xFF), char(0xFF), - // char(0xFF), char(0xFF), char(0xFF), char(0xFF), char(0xFF), char(0xFF), char(0xFF), char(0xFF) - // }; - // - // BD bd(&buffers[0][0]); - // - // for (size_t i = 0U; i < N_BUFFERS; ++i) - // { - // BD::descriptor desc = bd.allocate(char(0xFF)); - // - // CHECK_EQUAL(BUFFER_SIZE, desc.max_size()); - // CHECK_EQUAL(uintptr_t(&buffers[i][0]), uintptr_t(desc.data())); - // CHECK_ARRAY_EQUAL(test.data(), desc.data(), BUFFER_SIZE); - // } - // } - // - // //************************************************************************* - // TEST(test_notifications) - // { - // std::array test = { 0, 1, 2, 3, 4, 5, 6, 7, 0, 0, 0, 0, 0, 0, 0, 0 }; - // - // std::fill(&buffers[0][0], &buffers[N_BUFFERS - 1][0] + BUFFER_SIZE , 0U); - // - // receiver.clear(); - // BD::callback_type callback = BD::callback_type::create(receiver); - // - // BD bd(&buffers[0][0], callback); - // - // for (size_t i = 0U; i < N_BUFFERS; ++i) - // { - // BD::descriptor desc = bd.allocate(); - // - // CHECK(desc.is_valid()); - // - // std::copy(test.begin(), test.begin() + DATA_COUNT, desc.data()); - // bd.notify(BD::notification(desc, DATA_COUNT)); - // desc.release(); - // - // CHECK_EQUAL(DATA_COUNT, receiver.count); - // CHECK_EQUAL(uintptr_t(&buffers[i][0]), uintptr_t(receiver.pbuffer)); - // CHECK_ARRAY_EQUAL(test.data(), desc.data(), BUFFER_SIZE); - // } - // } - // - // //************************************************************************* - // TEST(test_allocate_overflow) - // { - // BD bd(&buffers[0][0]); - // - // // Use up all of the descriptors. - // for (size_t i = 0U; i < N_BUFFERS; ++i) - // { - // BD::descriptor desc = bd.allocate(); - // CHECK(desc.is_valid()); - // } - // - // BD::descriptor desc = bd.allocate(); - // CHECK(!desc.is_valid()); - // } - // - // //************************************************************************* - // TEST(test_allocate_release_rollover) - // { - // std::queue desc_queue; - // - // BD bd(&buffers[0][0]); - // - // // Use up all of the descriptors, then release/allocate for the rest. - // for (size_t i = 0U; i < (N_BUFFERS * 2); ++i) - // { - // BD::descriptor desc = bd.allocate(); - // desc_queue.push(desc); - // - // CHECK(desc.is_valid()); - // - // if (i >= (N_BUFFERS - 1)) - // { - // desc_queue.front().release(); - // desc_queue.pop(); - // } - // } - // } - // - // //************************************************************************* - // TEST(test_descriptors) - // { - // BD bd(&buffers[0][0]); - // - // BD::descriptor desc1 = bd.allocate(); - // BD::descriptor desc2 = bd.allocate(); - // BD::descriptor desc3 = bd.allocate(); - // BD::descriptor desc4 = bd.allocate(); - // - // CHECK(desc1.is_allocated()); - // CHECK(desc2.is_allocated()); - // CHECK(desc3.is_allocated()); - // CHECK(desc4.is_allocated()); - // - // CHECK(desc1.data() == &buffers[0][0]); - // CHECK(desc2.data() == &buffers[1][0]); - // CHECK(desc3.data() == &buffers[2][0]); - // CHECK(desc4.data() == &buffers[3][0]); - // - // CHECK(desc1.max_size() == BUFFER_SIZE); - // CHECK(desc2.max_size() == BUFFER_SIZE); - // CHECK(desc3.max_size() == BUFFER_SIZE); - // CHECK(desc4.max_size() == BUFFER_SIZE); - // - // CHECK(desc1.MAX_SIZE == BUFFER_SIZE); - // CHECK(desc2.MAX_SIZE == BUFFER_SIZE); - // CHECK(desc3.MAX_SIZE == BUFFER_SIZE); - // CHECK(desc4.MAX_SIZE == BUFFER_SIZE); - // - // CHECK(desc1.is_valid()); - // CHECK(desc2.is_valid()); - // CHECK(desc3.is_valid()); - // CHECK(desc4.is_valid()); - // - // desc1.release(); - // desc2.release(); - // desc3.release(); - // desc4.release(); - // - // CHECK(desc1.is_released()); - // CHECK(desc2.is_released()); - // CHECK(desc3.is_released()); - // CHECK(desc4.is_released()); - // } - // - // //************************************************************************* - //#if REALTIME_TEST - // - //#if defined(ETL_TARGET_OS_WINDOWS) // Only Windows priority is currently supported - //#define RAISE_THREAD_PRIORITY SetThreadPriority(GetCurrentThread(), THREAD_PRIORITY_HIGHEST) - //#define FIX_PROCESSOR_AFFINITY1 SetThreadAffinityMask(GetCurrentThread(), 1) - //#define FIX_PROCESSOR_AFFINITY2 SetThreadAffinityMask(GetCurrentThread(), 2) - //#else - //#define RAISE_THREAD_PRIORITY - //#define FIX_PROCESSOR_AFFINITY1 - //#define FIX_PROCESSOR_AFFINITY2 - //#endif - // - // std::atomic_bool start = false; - // - // //********************************* - // struct Notification - // { - // BD::descriptor desc; - // BD::size_type count; - // }; - // - // constexpr int N_ITERATIONS = 1000000; - // - // etl::queue_spsc_atomic desc_queue; - // - // //********************************* - // void Callback(BD::notification n) - // { - // desc_queue.push(n); - // } - // - // //********************************* - // void Producer() - // { - // static char buffers[N_BUFFERS][BUFFER_SIZE]; - // - // BD bd(&buffers[0][0], BD::callback_type::create()); - // - // RAISE_THREAD_PRIORITY; - // FIX_PROCESSOR_AFFINITY1; - // - // // Wait for the start flag. - // while (!start); - // - // int errors = 0; - // - // for (int i = 0; i < N_ITERATIONS; ++i) - // { - // BD::descriptor desc; - // - // // Wait until we can allocate a descriptor. - // do - // { - // desc = bd.allocate(); - // } while (desc.is_valid() == false); - // - // if (!desc.is_allocated()) - // { - // ++errors; - // } - // - // // Send a notification to the callback function. - // bd.notify(BD::notification(desc, BUFFER_SIZE)); - // } - // - // CHECK_EQUAL(0, errors); - // } - // - // //********************************* - // void Consumer() - // { - // RAISE_THREAD_PRIORITY; - // FIX_PROCESSOR_AFFINITY2; - // - // // Wait for the start flag. - // while (!start); - // - // int errors = 0; - // - // for (int i = 0; i < N_ITERATIONS;) - // { - // BD::notification notification; - // - // // Try to get a notification from the queue. - // if (desc_queue.pop(notification)) - // { - // CHECK_EQUAL(BUFFER_SIZE, notification.get_count()); - // CHECK(notification.get_descriptor().is_allocated()); - // - // if (!notification.get_descriptor().is_allocated()) - // { - // ++errors; - // } - // - // notification.get_descriptor().release(); - // ++i; - // } - // - // CHECK_EQUAL(0, errors); - // } - // } - // - // //********************************* - // TEST(test_multi_thread) - // { - // std::thread t1(Producer); - // std::thread t2(Consumer); - // - // start = true; - // - // t1.join(); - // t2.join(); - // } - //#endif + + //************************************************************************* + TEST(test_buffers_with_allocate_fill) + { + std::array test = + { + char(0xFF), char(0xFF), char(0xFF), char(0xFF), char(0xFF), char(0xFF), char(0xFF), char(0xFF), + char(0xFF), char(0xFF), char(0xFF), char(0xFF), char(0xFF), char(0xFF), char(0xFF), char(0xFF) + }; + + BD bd(&buffers[0][0]); + + for (size_t i = 0U; i < N_BUFFERS; ++i) + { + BD::descriptor desc = bd.allocate(char(0xFF)); + + CHECK_EQUAL(BUFFER_SIZE, desc.max_size()); + CHECK_EQUAL(uintptr_t(&buffers[i][0]), uintptr_t(desc.data())); + CHECK_ARRAY_EQUAL(test.data(), desc.data(), BUFFER_SIZE); + } + } + + //************************************************************************* + TEST(test_notifications) + { + std::array test = { 0, 1, 2, 3, 4, 5, 6, 7, 0, 0, 0, 0, 0, 0, 0, 0 }; + + std::fill(&buffers[0][0], &buffers[N_BUFFERS - 1][0] + BUFFER_SIZE , 0U); + + receiver.clear(); + BD::callback_type callback = BD::callback_type::create(receiver); + + BD bd(&buffers[0][0], callback); + + for (size_t i = 0U; i < N_BUFFERS; ++i) + { + BD::descriptor desc = bd.allocate(); + + CHECK(desc.is_valid()); + + std::copy(test.begin(), test.begin() + DATA_COUNT, desc.data()); + bd.notify(BD::notification(desc, DATA_COUNT)); + desc.release(); + + CHECK_EQUAL(DATA_COUNT, receiver.count); + CHECK_EQUAL(uintptr_t(&buffers[i][0]), uintptr_t(receiver.pbuffer)); + CHECK_ARRAY_EQUAL(test.data(), desc.data(), BUFFER_SIZE); + } + } + + //************************************************************************* + TEST(test_allocate_overflow) + { + BD bd(&buffers[0][0]); + + // Use up all of the descriptors. + for (size_t i = 0U; i < N_BUFFERS; ++i) + { + BD::descriptor desc = bd.allocate(); + CHECK(desc.is_valid()); + } + + BD::descriptor desc = bd.allocate(); + CHECK(!desc.is_valid()); + } + + //************************************************************************* + TEST(test_allocate_release_rollover) + { + std::queue desc_queue; + + BD bd(&buffers[0][0]); + + // Use up all of the descriptors, then release/allocate for the rest. + for (size_t i = 0U; i < (N_BUFFERS * 2); ++i) + { + BD::descriptor desc = bd.allocate(); + desc_queue.push(desc); + + CHECK(desc.is_valid()); + + if (i >= (N_BUFFERS - 1)) + { + desc_queue.front().release(); + desc_queue.pop(); + } + } + } + + //************************************************************************* + TEST(test_descriptors) + { + BD bd(&buffers[0][0]); + + BD::descriptor desc1 = bd.allocate(); + BD::descriptor desc2 = bd.allocate(); + BD::descriptor desc3 = bd.allocate(); + BD::descriptor desc4 = bd.allocate(); + + CHECK(desc1.is_allocated()); + CHECK(desc2.is_allocated()); + CHECK(desc3.is_allocated()); + CHECK(desc4.is_allocated()); + + CHECK(desc1.data() == &buffers[0][0]); + CHECK(desc2.data() == &buffers[1][0]); + CHECK(desc3.data() == &buffers[2][0]); + CHECK(desc4.data() == &buffers[3][0]); + + CHECK(desc1.max_size() == BUFFER_SIZE); + CHECK(desc2.max_size() == BUFFER_SIZE); + CHECK(desc3.max_size() == BUFFER_SIZE); + CHECK(desc4.max_size() == BUFFER_SIZE); + + CHECK(desc1.MAX_SIZE == BUFFER_SIZE); + CHECK(desc2.MAX_SIZE == BUFFER_SIZE); + CHECK(desc3.MAX_SIZE == BUFFER_SIZE); + CHECK(desc4.MAX_SIZE == BUFFER_SIZE); + + CHECK(desc1.is_valid()); + CHECK(desc2.is_valid()); + CHECK(desc3.is_valid()); + CHECK(desc4.is_valid()); + + desc1.release(); + desc2.release(); + desc3.release(); + desc4.release(); + + CHECK(desc1.is_released()); + CHECK(desc2.is_released()); + CHECK(desc3.is_released()); + CHECK(desc4.is_released()); + } + + //************************************************************************* +#if REALTIME_TEST + +#if defined(ETL_TARGET_OS_WINDOWS) // Only Windows priority is currently supported +#define RAISE_THREAD_PRIORITY SetThreadPriority(GetCurrentThread(), THREAD_PRIORITY_HIGHEST) +#define FIX_PROCESSOR_AFFINITY1 SetThreadAffinityMask(GetCurrentThread(), 1) +#define FIX_PROCESSOR_AFFINITY2 SetThreadAffinityMask(GetCurrentThread(), 2) +#else +#define RAISE_THREAD_PRIORITY +#define FIX_PROCESSOR_AFFINITY1 +#define FIX_PROCESSOR_AFFINITY2 +#endif + + std::atomic_bool start = false; + + //********************************* + struct Notification + { + BD::descriptor desc; + BD::size_type count; + }; + + constexpr int N_ITERATIONS = 1000000; + + etl::queue_spsc_atomic desc_queue; + + //********************************* + void Callback(BD::notification n) + { + desc_queue.push(n); + } + + //********************************* + void Producer() + { + static char buffers[N_BUFFERS][BUFFER_SIZE]; + + BD bd(&buffers[0][0], BD::callback_type::create()); + + RAISE_THREAD_PRIORITY; + FIX_PROCESSOR_AFFINITY1; + + // Wait for the start flag. + while (!start); + + int errors = 0; + + for (int i = 0; i < N_ITERATIONS; ++i) + { + BD::descriptor desc; + + // Wait until we can allocate a descriptor. + do + { + desc = bd.allocate(); + } while (desc.is_valid() == false); + + if (!desc.is_allocated()) + { + ++errors; + } + + // Send a notification to the callback function. + bd.notify(BD::notification(desc, BUFFER_SIZE)); + } + + CHECK_EQUAL(0, errors); + } + + //********************************* + void Consumer() + { + RAISE_THREAD_PRIORITY; + FIX_PROCESSOR_AFFINITY2; + + // Wait for the start flag. + while (!start); + + int errors = 0; + + for (int i = 0; i < N_ITERATIONS;) + { + BD::notification notification; + + // Try to get a notification from the queue. + if (desc_queue.pop(notification)) + { + CHECK_EQUAL(BUFFER_SIZE, notification.get_count()); + CHECK(notification.get_descriptor().is_allocated()); + + if (!notification.get_descriptor().is_allocated()) + { + ++errors; + } + + notification.get_descriptor().release(); + ++i; + } + + CHECK_EQUAL(0, errors); + } + } + + //********************************* + TEST(test_multi_thread) + { + std::thread t1(Producer); + std::thread t2(Consumer); + + start = true; + + t1.join(); + t2.join(); + } +#endif }; }