From 2abd8e69ee434752cf7e5f3c3e8f290a97168c2a Mon Sep 17 00:00:00 2001 From: ArielSAdamsNASA Date: Wed, 27 Oct 2021 12:27:52 -0500 Subject: [PATCH 1/8] Fix #1188, Reuse CodeQL, Static Analysis, Format Check --- .github/workflows/codeql-cfe-build.yml | 110 ++-------------------- .github/workflows/codeql-osal-default.yml | 84 ++--------------- .github/workflows/format-check.yml | 46 +-------- .github/workflows/static-analysis.yml | 50 +--------- 4 files changed, 21 insertions(+), 269 deletions(-) diff --git a/.github/workflows/codeql-cfe-build.yml b/.github/workflows/codeql-cfe-build.yml index 62b6e6a0b..a0d6db0f2 100644 --- a/.github/workflows/codeql-cfe-build.yml +++ b/.github/workflows/codeql-cfe-build.yml @@ -4,107 +4,11 @@ on: push: pull_request: -env: - SIMULATION: native - ENABLE_UNIT_TESTS: true - OMIT_DEPRECATED: true - BUILDTYPE: release - jobs: - #Checks for duplicate actions. Skips push actions if there is a matching or duplicate pull-request action. - check-for-duplicates: - runs-on: ubuntu-latest - # Map a step output to a job output - outputs: - should_skip: ${{ steps.skip_check.outputs.should_skip }} - steps: - - id: skip_check - uses: fkirc/skip-duplicate-actions@master - with: - concurrent_skipping: 'same_content' - skip_after_successful_duplicate: 'true' - do_not_skip: '["pull_request", "workflow_dispatch", "schedule"]' - - CodeQL-Security-Build: - needs: check-for-duplicates - if: ${{ needs.check-for-duplicates.outputs.should_skip != 'true' }} - runs-on: ubuntu-18.04 - timeout-minutes: 15 - - steps: - - name: Checkout bundle - uses: actions/checkout@v2 - with: - repository: nasa/cFS - submodules: true - - - name: Checkout submodule - uses: actions/checkout@v2 - with: - path: osal - - - name: Check versions - run: git submodule - - - name: Initialize CodeQL - uses: github/codeql-action/init@v1 - with: - languages: c - config-file: nasa/cFS/.github/codeql/codeql-security.yml@main - - - name: Set up for build - run: | - cp ./cfe/cmake/Makefile.sample Makefile - cp -r ./cfe/cmake/sample_defs sample_defs - make prep - - - name: Build - run: make -j native/default_cpu1/osal/ - - - name: Run tests - run: (cd build/native/default_cpu1/osal && make test) - - - name: Perform CodeQL Analysis - uses: github/codeql-action/analyze@v1 - - CodeQL-Coding-Standard-Build: - needs: check-for-duplicates - if: ${{ needs.check-for-duplicates.outputs.should_skip != 'true' }} - runs-on: ubuntu-18.04 - timeout-minutes: 15 - - steps: - - name: Checkout bundle - uses: actions/checkout@v2 - with: - repository: nasa/cFS - submodules: true - - - name: Checkout submodule - uses: actions/checkout@v2 - with: - path: osal - - - name: Check versions - run: git submodule - - - name: Initialize CodeQL - uses: github/codeql-action/init@v1 - with: - languages: c - config-file: nasa/cFS/.github/codeql/codeql-coding-standard.yml@main - - - name: Set up for build - run: | - cp ./cfe/cmake/Makefile.sample Makefile - cp -r ./cfe/cmake/sample_defs sample_defs - make prep - - - name: Build - run: make -j native/default_cpu1/osal/ - - - name: Run tests - run: (cd build/native/default_cpu1/osal && make test) - - - name: Perform CodeQL Analysis - uses: github/codeql-action/analyze@v1 + codeql: + name: CodeQl Analysis + uses: nasa/cFS/.github/workflows/codeql-build.yml@main + with: + make-prep: 'make prep' + make: 'make -j native/default_cpu1/osal/' + tests: '(cd build/native/default_cpu1/osal && make test)' \ No newline at end of file diff --git a/.github/workflows/codeql-osal-default.yml b/.github/workflows/codeql-osal-default.yml index 3cb8146e3..0509f7280 100644 --- a/.github/workflows/codeql-osal-default.yml +++ b/.github/workflows/codeql-osal-default.yml @@ -4,81 +4,11 @@ on: push: pull_request: -env: - SIMULATION: native - ENABLE_UNIT_TESTS: true - OMIT_DEPRECATED: true - BUILDTYPE: release - PERMISSIVE_MODE: true - jobs: - - #Checks for duplicate actions. Skips push actions if there is a matching or duplicate pull-request action. - check-for-duplicates: - runs-on: ubuntu-latest - # Map a step output to a job output - outputs: - should_skip: ${{ steps.skip_check.outputs.should_skip }} - steps: - - id: skip_check - uses: fkirc/skip-duplicate-actions@master - with: - concurrent_skipping: 'same_content' - skip_after_successful_duplicate: 'true' - do_not_skip: '["pull_request", "workflow_dispatch", "schedule"]' - - CodeQL-Security-Build: - #Continue if check-for-duplicates found no duplicates. Always runs for pull-requests. - needs: check-for-duplicates - if: ${{ needs.check-for-duplicates.outputs.should_skip != 'true' }} - runs-on: ubuntu-18.04 - timeout-minutes: 15 - - steps: - - name: Checkout submodule - uses: actions/checkout@v2 - - - name: Initialize CodeQL - uses: github/codeql-action/init@v1 - with: - languages: c - config-file: nasa/cFS/.github/codeql/codeql-security.yml@main - - - name: Set up for build - run: | - cp Makefile.sample Makefile - make prep - - - name: Build - run: make -j - - - name: Perform CodeQL Analysis - uses: github/codeql-action/analyze@v1 - - CodeQL-Coding-Standard-Build: - #Continue if check-for-duplicates found no duplicates. Always runs for pull-requests. - needs: check-for-duplicates - if: ${{ needs.check-for-duplicates.outputs.should_skip != 'true' }} - runs-on: ubuntu-18.04 - timeout-minutes: 15 - - steps: - - name: Checkout submodule - uses: actions/checkout@v2 - - - name: Initialize CodeQL - uses: github/codeql-action/init@v1 - with: - languages: c - config-file: nasa/cFS/.github/codeql/codeql-coding-standard.yml@main - - - name: Set up for build - run: | - cp Makefile.sample Makefile - make prep - - - name: Build - run: make -j - - - name: Perform CodeQL Analysis - uses: github/codeql-action/analyze@v1 \ No newline at end of file + codeql: + name: CodeQl Analysis + uses: nasa/cFS/.github/workflows/codeql-build.yml@main + with: + setup: 'cd osal && cp Makefile.sample Makefile' + make-prep: 'cd osal && make prep' + make: 'cd osal && make -j' \ No newline at end of file diff --git a/.github/workflows/format-check.yml b/.github/workflows/format-check.yml index 0a998eccb..d5fd74250 100644 --- a/.github/workflows/format-check.yml +++ b/.github/workflows/format-check.yml @@ -1,6 +1,6 @@ name: Format Check -# Run on main push and pull requests +# Run on all push and pull requests on: push: branches: @@ -8,46 +8,6 @@ on: pull_request: jobs: - - static-analysis: + format-check: name: Run format check - runs-on: ubuntu-18.04 - timeout-minutes: 15 - - steps: - - - name: Install format checker - run: | - wget -O - https://apt.llvm.org/llvm-snapshot.gpg.key | sudo apt-key add - - sudo add-apt-repository 'deb http://apt.llvm.org/bionic/ llvm-toolchain-bionic-10 main' - sudo apt-get update && sudo apt-get install clang-format-10 - - - name: Checkout bundle - uses: actions/checkout@v2 - with: - repository: nasa/cFS - - - name: Checkout - uses: actions/checkout@v2 - with: - path: repo - - - name: Generate format differences - run: | - cd repo - find . -name "*.[ch]" -exec clang-format-10 -i -style=file {} + - git diff > $GITHUB_WORKSPACE/style_differences.txt - - - name: Archive Static Analysis Artifacts - uses: actions/upload-artifact@v2 - with: - name: style_differences - path: style_differences.txt - - - name: Error on differences - run: | - if [[ -s style_differences.txt ]]; - then - cat style_differences.txt - exit -1 - fi + uses: nasa/cFS/.github/workflows/format-check.yml@main \ No newline at end of file diff --git a/.github/workflows/static-analysis.yml b/.github/workflows/static-analysis.yml index 5016ac33f..ac905d695 100644 --- a/.github/workflows/static-analysis.yml +++ b/.github/workflows/static-analysis.yml @@ -1,55 +1,13 @@ name: Static Analysis -# Run this workflow every time a new commit pushed to your repository +# Run on all push and pull requests on: push: - branches: - - main pull_request: jobs: - static-analysis: name: Run cppcheck - runs-on: ubuntu-18.04 - timeout-minutes: 15 - - strategy: - fail-fast: false - matrix: - cppcheck: [all, osal] - - steps: - - - name: Install cppcheck - run: sudo apt-get install cppcheck -y - - # Checks out a copy of the cfs bundle - - name: Checkout code - uses: actions/checkout@v2 - with: - submodules: true - - - name: Run bundle cppcheck - if: ${{matrix.cppcheck =='all'}} - run: cppcheck --force --inline-suppr . 2> ${{matrix.cppcheck}}_cppcheck_err.txt - - # Run strict static analysis for embedded portions of osal - - name: osal strict cppcheck - if: ${{matrix.cppcheck =='osal'}} - run: | - cppcheck --force --inline-suppr --std=c99 --language=c --enable=warning,performance,portability,style --suppress=variableScope --inconclusive ./src/bsp ./src/os 2> ./${{matrix.cppcheck}}_cppcheck_err.txt - - - name: Archive Static Analysis Artifacts - uses: actions/upload-artifact@v2 - with: - name: ${{matrix.cppcheck}}-cppcheck-err - path: ./*cppcheck_err.txt - - - name: Check for errors - run: | - if [[ -s ${{matrix.cppcheck}}_cppcheck_err.txt ]]; - then - cat ${{matrix.cppcheck}}_cppcheck_err.txt - exit -1 - fi + uses: nasa/cFS/.github/workflows/static-analysis.yml@main + with: + strict-dir-list: './src/bsp ./src/os' From 09032eea305ed0c5c3c782c46b0f9d8350b26100 Mon Sep 17 00:00:00 2001 From: KurtJD Date: Fri, 7 Jan 2022 07:27:58 -0800 Subject: [PATCH 2/8] Fix #1200, Add missing space to UtAssert_STUB_COUNT --- ut_assert/inc/utassert.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ut_assert/inc/utassert.h b/ut_assert/inc/utassert.h index 35861ca13..cca0469a3 100644 --- a/ut_assert/inc/utassert.h +++ b/ut_assert/inc/utassert.h @@ -498,7 +498,7 @@ typedef struct */ #define UtAssert_STUB_COUNT(stub, expected) \ UtAssert_GenericSignedCompare(UT_GetStubCount(UT_KEY(stub)), UtAssert_Compare_EQ, expected, \ - UtAssert_Radix_DECIMAL, __FILE__, __LINE__, "CallCount", #stub "()", #expected) + UtAssert_Radix_DECIMAL, __FILE__, __LINE__, "CallCount ", #stub "()", #expected) /* * Exported Functions From 7aa674ce760e1bccf77c173240f5b3d4995734c6 Mon Sep 17 00:00:00 2001 From: KurtJD Date: Sat, 8 Jan 2022 18:04:07 -0800 Subject: [PATCH 3/8] Fix #1196, Add UINT16 equivalents for UtAssert_UINT32_ macros --- ut_assert/inc/utassert.h | 78 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 78 insertions(+) diff --git a/ut_assert/inc/utassert.h b/ut_assert/inc/utassert.h index 35861ca13..fba1e6c0e 100644 --- a/ut_assert/inc/utassert.h +++ b/ut_assert/inc/utassert.h @@ -406,6 +406,84 @@ typedef struct UtAssert_GenericUnsignedCompare((uint32)(expr), UtAssert_Compare_GT, (uint32)(ref), UtAssert_Radix_DECIMAL, \ __FILE__, __LINE__, "", #expr, #ref) +/** + * \brief Compare two 16-bit values for equality with an auto-generated description message + * + * This macro confirms that the given expression is equal to the reference value + * The generated log message will include the actual and reference values in decimal notation + * Values will be compared in an "uint16" type context. + */ +#define UtAssert_UINT16_EQ(actual, ref) \ + UtAssert_GenericUnsignedCompare((uint16)(actual), UtAssert_Compare_EQ, (uint16)(ref), UtAssert_Radix_DECIMAL, \ + __FILE__, __LINE__, "Compare UINT16: ", #actual, #ref) + +/** + * \brief Compare two 16-bit values for inequality with an auto-generated description message + * + * This macro confirms that the given expression is _not_ equal to the reference value + * The generated log message will include the actual and reference values in decimal notation + * Values will be compared in an "uint16" type context. + */ +#define UtAssert_UINT16_NEQ(actual, ref) \ + UtAssert_GenericUnsignedCompare((uint16)(actual), UtAssert_Compare_NEQ, (uint16)(ref), UtAssert_Radix_DECIMAL, \ + __FILE__, __LINE__, "Compare UINT16: ", #actual, #ref) + +/** + * \brief Asserts the minimum 16-bit value of a given function or expression + * + * \par Description + * This macro confirms that the given expression is at least the minimum 16-bit value (inclusive) + * + * \par Assumptions, External Events, and Notes: + * None + * + */ +#define UtAssert_UINT16_GTEQ(expr, ref) \ + UtAssert_GenericUnsignedCompare((uint16)(expr), UtAssert_Compare_GTEQ, (uint16)(ref), UtAssert_Radix_DECIMAL, \ + __FILE__, __LINE__, "Compare UINT16: ", #expr, #ref) + +/** + * \brief Asserts the maximum 16-bit value of a given function or expression + * + * \par Description + * This macro confirms that the given expression is at most the maximum 16-bit value (inclusive) + * + * \par Assumptions, External Events, and Notes: + * None + * + */ +#define UtAssert_UINT16_LTEQ(expr, ref) \ + UtAssert_GenericUnsignedCompare((uint16)(expr), UtAssert_Compare_LTEQ, (uint16)(ref), UtAssert_Radix_DECIMAL, \ + __FILE__, __LINE__, "Compare UINT16: ", #expr, #ref) + +/** + * \brief Asserts the 16-bit value of a given function or expression is less than the 16-bit reference value + * + * \par Description + * This macro confirms that the given expression is less than the maximum 16-bit value (exclusive) + * + * \par Assumptions, External Events, and Notes: + * None + * + */ +#define UtAssert_UINT16_LT(expr, ref) \ + UtAssert_GenericUnsignedCompare((uint16)(expr), UtAssert_Compare_LT, (uint16)(ref), UtAssert_Radix_DECIMAL, \ + __FILE__, __LINE__, "Compare UINT16: ", #expr, #ref) + +/** + * \brief Asserts the 16-bit value of a given function or expression is greater than the 16-bit reference value + * + * \par Description + * This macro confirms that the given expression is greater than the minimum 16-bit value (exclusive) + * + * \par Assumptions, External Events, and Notes: + * None + * + */ +#define UtAssert_UINT16_GT(expr, ref) \ + UtAssert_GenericUnsignedCompare((uint16)(expr), UtAssert_Compare_GT, (uint16)(ref), UtAssert_Radix_DECIMAL, \ + __FILE__, __LINE__, "Compare UINT16: ", #expr, #ref) + /** * \brief Macro for checking that bits in a bit field are set * From a3bf24592347510972a25285f60db7396a85cd66 Mon Sep 17 00:00:00 2001 From: KurtJD Date: Sat, 8 Jan 2022 18:12:46 -0800 Subject: [PATCH 4/8] Fix #1196, Add UINT8 equivalents for UtAssert_UINT32_ macros --- ut_assert/inc/utassert.h | 78 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 78 insertions(+) diff --git a/ut_assert/inc/utassert.h b/ut_assert/inc/utassert.h index fba1e6c0e..b40c884f3 100644 --- a/ut_assert/inc/utassert.h +++ b/ut_assert/inc/utassert.h @@ -484,6 +484,84 @@ typedef struct UtAssert_GenericUnsignedCompare((uint16)(expr), UtAssert_Compare_GT, (uint16)(ref), UtAssert_Radix_DECIMAL, \ __FILE__, __LINE__, "Compare UINT16: ", #expr, #ref) +/** + * \brief Compare two 8-bit values for equality with an auto-generated description message + * + * This macro confirms that the given expression is equal to the reference value + * The generated log message will include the actual and reference values in decimal notation + * Values will be compared in an "uint8" type context. + */ +#define UtAssert_UINT8_EQ(actual, ref) \ + UtAssert_GenericUnsignedCompare((uint8)(actual), UtAssert_Compare_EQ, (uint8)(ref), UtAssert_Radix_DECIMAL, \ + __FILE__, __LINE__, "Compare UINT8: ", #actual, #ref) + +/** + * \brief Compare two 8-bit values for inequality with an auto-generated description message + * + * This macro confirms that the given expression is _not_ equal to the reference value + * The generated log message will include the actual and reference values in decimal notation + * Values will be compared in an "uint8" type context. + */ +#define UtAssert_UINT8_NEQ(actual, ref) \ + UtAssert_GenericUnsignedCompare((uint8)(actual), UtAssert_Compare_NEQ, (uint8)(ref), UtAssert_Radix_DECIMAL, \ + __FILE__, __LINE__, "Compare UINT8: ", #actual, #ref) + +/** + * \brief Asserts the minimum 8-bit value of a given function or expression + * + * \par Description + * This macro confirms that the given expression is at least the minimum 8-bit value (inclusive) + * + * \par Assumptions, External Events, and Notes: + * None + * + */ +#define UtAssert_UINT8_GTEQ(expr, ref) \ + UtAssert_GenericUnsignedCompare((uint8)(expr), UtAssert_Compare_GTEQ, (uint8)(ref), UtAssert_Radix_DECIMAL, \ + __FILE__, __LINE__, "Compare UINT8: ", #expr, #ref) + +/** + * \brief Asserts the maximum 8-bit value of a given function or expression + * + * \par Description + * This macro confirms that the given expression is at most the maximum 8-bit value (inclusive) + * + * \par Assumptions, External Events, and Notes: + * None + * + */ +#define UtAssert_UINT8_LTEQ(expr, ref) \ + UtAssert_GenericUnsignedCompare((uint8)(expr), UtAssert_Compare_LTEQ, (uint8)(ref), UtAssert_Radix_DECIMAL, \ + __FILE__, __LINE__, "Compare UINT8: ", #expr, #ref) + +/** + * \brief Asserts the 8-bit value of a given function or expression is less than the 8-bit reference value + * + * \par Description + * This macro confirms that the given expression is less than the maximum 8-bit value (exclusive) + * + * \par Assumptions, External Events, and Notes: + * None + * + */ +#define UtAssert_UINT8_LT(expr, ref) \ + UtAssert_GenericUnsignedCompare((uint8)(expr), UtAssert_Compare_LT, (uint8)(ref), UtAssert_Radix_DECIMAL, \ + __FILE__, __LINE__, "Compare UINT8: ", #expr, #ref) + +/** + * \brief Asserts the 8-bit value of a given function or expression is greater than the 8-bit reference value + * + * \par Description + * This macro confirms that the given expression is greater than the minimum 8-bit value (exclusive) + * + * \par Assumptions, External Events, and Notes: + * None + * + */ +#define UtAssert_UINT8_GT(expr, ref) \ + UtAssert_GenericUnsignedCompare((uint8)(expr), UtAssert_Compare_GT, (uint8)(ref), UtAssert_Radix_DECIMAL, \ + __FILE__, __LINE__, "Compare UINT8: ", #expr, #ref) + /** * \brief Macro for checking that bits in a bit field are set * From e51cbe33794e5245df0567ba472444e9883f6fda Mon Sep 17 00:00:00 2001 From: Joseph Hickey Date: Mon, 10 Jan 2022 11:16:07 -0500 Subject: [PATCH 5/8] Fix #1199, correct warnings on gcc11 These new warnings are detected by the new compiler. They are all places in unit test where an uninitialized value is passed via "const" pointer into a unit under test. By definition a "const" pointer is always an input. While the warning is pedantically true - should not pass an uninitialized struct as an input - these were all unit tests where the object value is a don't-care value, so it does not matter. But to fix the warning, simply need to initialize a value before making the call. --- .../portable/src/coveragetest-bsd-sockets.c | 14 +++++++------- .../shared/src/coveragetest-clock.c | 6 +++--- .../vxworks/src/coveragetest-tasks.c | 2 +- src/unit-tests/oscore-test/ut_oscore_task_test.c | 2 +- 4 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/unit-test-coverage/portable/src/coveragetest-bsd-sockets.c b/src/unit-test-coverage/portable/src/coveragetest-bsd-sockets.c index 9322fa2bc..e3f2ac551 100644 --- a/src/unit-test-coverage/portable/src/coveragetest-bsd-sockets.c +++ b/src/unit-test-coverage/portable/src/coveragetest-bsd-sockets.c @@ -316,10 +316,10 @@ void Test_OS_SocketRecvFrom_Impl(void) void Test_OS_SocketSendTo_Impl(void) { - OS_object_token_t token = {0}; - uint8 buffer[UT_BUFFER_SIZE]; - OS_SockAddr_t addr = {0}; - struct OCS_sockaddr *sa = (struct OCS_sockaddr *)&addr.AddrData; + OS_object_token_t token = {0}; + const uint8 buffer[UT_BUFFER_SIZE] = {0}; + OS_SockAddr_t addr = {0}; + struct OCS_sockaddr *sa = (struct OCS_sockaddr *)&addr.AddrData; /* Set up token */ token.obj_idx = UT_INDEX_0; @@ -393,9 +393,9 @@ void Test_OS_SocketAddrToString_Impl(void) void Test_OS_SocketAddrFromString_Impl(void) { - char buffer[UT_BUFFER_SIZE]; - OS_SockAddr_t addr = {0}; - struct OCS_sockaddr *sa = (struct OCS_sockaddr *)&addr.AddrData; + const char buffer[UT_BUFFER_SIZE] = "UT"; + OS_SockAddr_t addr = {0}; + struct OCS_sockaddr *sa = (struct OCS_sockaddr *)&addr.AddrData; /* Bad family */ sa->sa_family = -1; diff --git a/src/unit-test-coverage/shared/src/coveragetest-clock.c b/src/unit-test-coverage/shared/src/coveragetest-clock.c index 244f8b2e1..2490ddbdb 100644 --- a/src/unit-test-coverage/shared/src/coveragetest-clock.c +++ b/src/unit-test-coverage/shared/src/coveragetest-clock.c @@ -50,9 +50,9 @@ void Test_OS_SetLocalTime(void) * Test Case For: * int32 OS_SetLocalTime(OS_time_t *time_struct) */ - OS_time_t time_struct; - int32 expected = OS_SUCCESS; - int32 actual = OS_SetLocalTime(&time_struct); + OS_time_t time_struct = OS_TimeAssembleFromMicroseconds(5, 12345); + int32 expected = OS_SUCCESS; + int32 actual = OS_SetLocalTime(&time_struct); UtAssert_True(actual == expected, "OS_SetLocalTime() (%ld) == OS_SUCCESS", (long)actual); diff --git a/src/unit-test-coverage/vxworks/src/coveragetest-tasks.c b/src/unit-test-coverage/vxworks/src/coveragetest-tasks.c index afd7c3184..c99965219 100644 --- a/src/unit-test-coverage/vxworks/src/coveragetest-tasks.c +++ b/src/unit-test-coverage/vxworks/src/coveragetest-tasks.c @@ -145,7 +145,7 @@ void Test_OS_TaskDetach_Impl(void) * Test Case For: * int32 OS_TaskDetach_Impl(const OS_object_token_t *token) */ - OS_object_token_t token; + OS_object_token_t token = UT_TOKEN_0; /* no-op on VxWorks - always returns success */ OSAPI_TEST_FUNCTION_RC(OS_TaskDetach_Impl(&token), OS_SUCCESS); diff --git a/src/unit-tests/oscore-test/ut_oscore_task_test.c b/src/unit-tests/oscore-test/ut_oscore_task_test.c index 3c4fb1e2e..879cd7093 100644 --- a/src/unit-tests/oscore-test/ut_oscore_task_test.c +++ b/src/unit-tests/oscore-test/ut_oscore_task_test.c @@ -605,7 +605,7 @@ void UT_os_task_get_info_test() **--------------------------------------------------------------------------------*/ void UT_os_task_getid_by_sysdata_test() { - uint8 sysdata; + uint8 sysdata = 0; osal_id_t task_id; /* From 82487f9441abc3ccfd5f6d36bdd27ef85e7c4a4f Mon Sep 17 00:00:00 2001 From: Jacob Hageman Date: Wed, 12 Jan 2022 14:29:42 -0700 Subject: [PATCH 6/8] Fix #1204, Search global and local symbol tables - Refactors symbol table searching to include both local and global symbol tables for POSIX - Renamed global search to generic since there isn't currently a use case for global only search --- src/os/portable/os-impl-no-symtab.c | 2 +- src/os/portable/os-impl-posix-dl-symtab.c | 25 ++++++++++++++++--- src/os/shared/inc/os-shared-module.h | 7 +++--- src/os/shared/src/osapi-module.c | 4 +-- src/os/vxworks/src/os-impl-symtab.c | 8 +++--- .../portable/src/coveragetest-no-symtab.c | 2 +- .../shared/src/coveragetest-module.c | 4 +-- .../src/os-shared-module-impl-stubs.c | 14 +++++------ .../vxworks/src/coveragetest-symtab.c | 14 +++++------ .../osloader-test/ut_osloader_symtable_test.c | 15 ++++++++++- 10 files changed, 64 insertions(+), 31 deletions(-) diff --git a/src/os/portable/os-impl-no-symtab.c b/src/os/portable/os-impl-no-symtab.c index 270fd598d..5a36ecee9 100644 --- a/src/os/portable/os-impl-no-symtab.c +++ b/src/os/portable/os-impl-no-symtab.c @@ -35,7 +35,7 @@ * * See prototype for argument/return detail *-----------------------------------------------------------------*/ -int32 OS_GlobalSymbolLookup_Impl(cpuaddr *SymbolAddress, const char *SymbolName) +int32 OS_SymbolLookup_Impl(cpuaddr *SymbolAddress, const char *SymbolName) { return OS_ERR_NOT_IMPLEMENTED; } diff --git a/src/os/portable/os-impl-posix-dl-symtab.c b/src/os/portable/os-impl-posix-dl-symtab.c index 629ba8d8e..81d578ad3 100644 --- a/src/os/portable/os-impl-posix-dl-symtab.c +++ b/src/os/portable/os-impl-posix-dl-symtab.c @@ -134,18 +134,37 @@ int32 OS_GenericSymbolLookup_Impl(void *dl_handle, cpuaddr *SymbolAddress, const /*---------------------------------------------------------------- * - * Function: OS_GlobalSymbolLookup_Impl + * Function: OS_SymbolLookup_Impl * * Purpose: Implemented per internal OSAL API * See prototype for argument/return detail * *-----------------------------------------------------------------*/ -int32 OS_GlobalSymbolLookup_Impl(cpuaddr *SymbolAddress, const char *SymbolName) +int32 OS_SymbolLookup_Impl(cpuaddr *SymbolAddress, const char *SymbolName) { - int32 status; + int32 status; + int32 local_status = OS_ERROR; + OS_object_iter_t iter; + /* First search global table */ status = OS_GenericSymbolLookup_Impl(OSAL_DLSYM_DEFAULT_HANDLE, SymbolAddress, SymbolName); + /* If not found iterate through module local symbols and break if found */ + if (status != OS_SUCCESS) + { + OS_ObjectIdIterateActive(OS_OBJECT_TYPE_OS_MODULE, &iter); + while (OS_ObjectIdIteratorGetNext(&iter)) + { + local_status = OS_ModuleSymbolLookup_Impl(&iter.token, SymbolAddress, SymbolName); + if (local_status == OS_SUCCESS) + { + status = local_status; + break; + } + } + OS_ObjectIdIteratorDestroy(&iter); + } + return status; } /* end OS_SymbolLookup_Impl */ diff --git a/src/os/shared/inc/os-shared-module.h b/src/os/shared/inc/os-shared-module.h index 86811e8e7..b43ded2a5 100644 --- a/src/os/shared/inc/os-shared-module.h +++ b/src/os/shared/inc/os-shared-module.h @@ -95,14 +95,15 @@ int32 OS_ModuleUnload_Impl(const OS_object_token_t *token); int32 OS_ModuleGetInfo_Impl(const OS_object_token_t *token, OS_module_prop_t *module_prop); /*---------------------------------------------------------------- - Function: OS_GlobalSymbolLookup_Impl + Function: OS_SymbolLookup_Impl - Purpose: Find the Address of a Symbol in the global symbol table. + Purpose: Find the Address of a Symbol in the symbol table. If global and + local tables exist all are checked. The address of the symbol will be stored in the pointer that is passed in. Returns: OS_SUCCESS on success, or relevant error code ------------------------------------------------------------------*/ -int32 OS_GlobalSymbolLookup_Impl(cpuaddr *SymbolAddress, const char *SymbolName); +int32 OS_SymbolLookup_Impl(cpuaddr *SymbolAddress, const char *SymbolName); /*---------------------------------------------------------------- Function: OS_SymbolLookup_Impl diff --git a/src/os/shared/src/osapi-module.c b/src/os/shared/src/osapi-module.c index 720da42ed..236006c24 100644 --- a/src/os/shared/src/osapi-module.c +++ b/src/os/shared/src/osapi-module.c @@ -361,9 +361,9 @@ int32 OS_SymbolLookup(cpuaddr *SymbolAddress, const char *SymbolName) OS_CHECK_POINTER(SymbolName); /* - * attempt to find the symbol in the global symbol table. + * attempt to find the symbol in the symbol table */ - return_code = OS_GlobalSymbolLookup_Impl(SymbolAddress, SymbolName); + return_code = OS_SymbolLookup_Impl(SymbolAddress, SymbolName); /* * If the OS call did not find the symbol or the loader is diff --git a/src/os/vxworks/src/os-impl-symtab.c b/src/os/vxworks/src/os-impl-symtab.c index 3956f0541..35ed770d9 100644 --- a/src/os/vxworks/src/os-impl-symtab.c +++ b/src/os/vxworks/src/os-impl-symtab.c @@ -108,16 +108,16 @@ int32 OS_GenericSymbolLookup_Impl(SYMTAB_ID SymTab, cpuaddr *SymbolAddress, cons /*---------------------------------------------------------------- * - * Function: OS_GlobalSymbolLookup_Impl + * Function: OS_SymbolLookup_Impl * * Purpose: Implemented per internal OSAL API * See prototype for argument/return detail * *-----------------------------------------------------------------*/ -int32 OS_GlobalSymbolLookup_Impl(cpuaddr *SymbolAddress, const char *SymbolName) +int32 OS_SymbolLookup_Impl(cpuaddr *SymbolAddress, const char *SymbolName) { return OS_GenericSymbolLookup_Impl(sysSymTbl, SymbolAddress, SymbolName); -} /* end OS_GlobalSymbolLookup_Impl */ +} /* end OS_SymbolLookup_Impl */ /*---------------------------------------------------------------- * @@ -130,7 +130,7 @@ int32 OS_GlobalSymbolLookup_Impl(cpuaddr *SymbolAddress, const char *SymbolName) int32 OS_ModuleSymbolLookup_Impl(const OS_object_token_t *token, cpuaddr *SymbolAddress, const char *SymbolName) { /* - * NOTE: this is currently exactly the same as OS_GlobalSymbolLookup_Impl(). + * NOTE: this is currently exactly the same as OS_SymbolLookup_Impl(). * * Ideally this should get a SYMTAB_ID from the MODULE_ID and search only * for the symbols provided by that module - but it is not clear if vxWorks diff --git a/src/unit-test-coverage/portable/src/coveragetest-no-symtab.c b/src/unit-test-coverage/portable/src/coveragetest-no-symtab.c index ad7009970..f08ff8c9b 100644 --- a/src/unit-test-coverage/portable/src/coveragetest-no-symtab.c +++ b/src/unit-test-coverage/portable/src/coveragetest-no-symtab.c @@ -28,7 +28,7 @@ void Test_No_Symtab(void) { - OSAPI_TEST_FUNCTION_RC(OS_GlobalSymbolLookup_Impl, (NULL, NULL), OS_ERR_NOT_IMPLEMENTED); + OSAPI_TEST_FUNCTION_RC(OS_SymbolLookup_Impl, (NULL, NULL), OS_ERR_NOT_IMPLEMENTED); OSAPI_TEST_FUNCTION_RC(OS_ModuleSymbolLookup_Impl, (NULL, NULL, NULL), OS_ERR_NOT_IMPLEMENTED); OSAPI_TEST_FUNCTION_RC(OS_SymbolTableDump_Impl, (NULL, 0), OS_ERR_NOT_IMPLEMENTED); } diff --git a/src/unit-test-coverage/shared/src/coveragetest-module.c b/src/unit-test-coverage/shared/src/coveragetest-module.c index 4eace72c0..63644fc7e 100644 --- a/src/unit-test-coverage/shared/src/coveragetest-module.c +++ b/src/unit-test-coverage/shared/src/coveragetest-module.c @@ -136,8 +136,8 @@ void Test_OS_SymbolLookup(void) actual = OS_SymbolLookup(&symaddr, "uttestsym0"); UtAssert_True(actual == expected, "OS_SymbolLookup(name=%s) (%ld) == OS_SUCCESS", "uttestsym0", (long)actual); - UT_ResetState(UT_KEY(OS_GlobalSymbolLookup_Impl)); - UT_SetDefaultReturnValue(UT_KEY(OS_GlobalSymbolLookup_Impl), OS_ERROR); + UT_ResetState(UT_KEY(OS_SymbolLookup_Impl)); + UT_SetDefaultReturnValue(UT_KEY(OS_SymbolLookup_Impl), OS_ERROR); /* this lookup should always fail */ symaddr = 0; diff --git a/src/unit-test-coverage/ut-stubs/src/os-shared-module-impl-stubs.c b/src/unit-test-coverage/ut-stubs/src/os-shared-module-impl-stubs.c index 6413b0b05..bc7bcf2aa 100644 --- a/src/unit-test-coverage/ut-stubs/src/os-shared-module-impl-stubs.c +++ b/src/unit-test-coverage/ut-stubs/src/os-shared-module-impl-stubs.c @@ -29,19 +29,19 @@ /* * ---------------------------------------------------- - * Generated stub function for OS_GlobalSymbolLookup_Impl() + * Generated stub function for OS_SymbolLookup_Impl() * ---------------------------------------------------- */ -int32 OS_GlobalSymbolLookup_Impl(cpuaddr *SymbolAddress, const char *SymbolName) +int32 OS_SymbolLookup_Impl(cpuaddr *SymbolAddress, const char *SymbolName) { - UT_GenStub_SetupReturnBuffer(OS_GlobalSymbolLookup_Impl, int32); + UT_GenStub_SetupReturnBuffer(OS_SymbolLookup_Impl, int32); - UT_GenStub_AddParam(OS_GlobalSymbolLookup_Impl, cpuaddr *, SymbolAddress); - UT_GenStub_AddParam(OS_GlobalSymbolLookup_Impl, const char *, SymbolName); + UT_GenStub_AddParam(OS_SymbolLookup_Impl, cpuaddr *, SymbolAddress); + UT_GenStub_AddParam(OS_SymbolLookup_Impl, const char *, SymbolName); - UT_GenStub_Execute(OS_GlobalSymbolLookup_Impl, Basic, NULL); + UT_GenStub_Execute(OS_SymbolLookup_Impl, Basic, NULL); - return UT_GenStub_GetReturnValue(OS_GlobalSymbolLookup_Impl, int32); + return UT_GenStub_GetReturnValue(OS_SymbolLookup_Impl, int32); } /* diff --git a/src/unit-test-coverage/vxworks/src/coveragetest-symtab.c b/src/unit-test-coverage/vxworks/src/coveragetest-symtab.c index 9d2e84397..ea5fca3cb 100644 --- a/src/unit-test-coverage/vxworks/src/coveragetest-symtab.c +++ b/src/unit-test-coverage/vxworks/src/coveragetest-symtab.c @@ -33,18 +33,18 @@ #include "OCS_fcntl.h" #include "OCS_symLib.h" -void Test_OS_GlobalSymbolLookup_Impl(void) +void Test_OS_SymbolLookup_Impl(void) { /* Test Case For: - * int32 OS_GlobalSymbolLookup_Impl( cpuaddr *SymbolAddress, const char *SymbolName ) + * int32 OS_SymbolLookup_Impl( cpuaddr *SymbolAddress, const char *SymbolName ) */ cpuaddr SymAddr; - OSAPI_TEST_FUNCTION_RC(OS_GlobalSymbolLookup_Impl(&SymAddr, "symname"), OS_SUCCESS); - OSAPI_TEST_FUNCTION_RC(OS_GlobalSymbolLookup_Impl(NULL, "symname"), OS_INVALID_POINTER); - OSAPI_TEST_FUNCTION_RC(OS_GlobalSymbolLookup_Impl(&SymAddr, NULL), OS_INVALID_POINTER); + OSAPI_TEST_FUNCTION_RC(OS_SymbolLookup_Impl(&SymAddr, "symname"), OS_SUCCESS); + OSAPI_TEST_FUNCTION_RC(OS_SymbolLookup_Impl(NULL, "symname"), OS_INVALID_POINTER); + OSAPI_TEST_FUNCTION_RC(OS_SymbolLookup_Impl(&SymAddr, NULL), OS_INVALID_POINTER); UT_SetDefaultReturnValue(UT_KEY(OCS_symFind), OCS_ERROR); - OSAPI_TEST_FUNCTION_RC(OS_GlobalSymbolLookup_Impl(&SymAddr, "symname"), OS_ERROR); + OSAPI_TEST_FUNCTION_RC(OS_SymbolLookup_Impl(&SymAddr, "symname"), OS_ERROR); } void Test_OS_ModuleSymbolLookup_Impl(void) @@ -144,7 +144,7 @@ void Osapi_Test_Teardown(void) {} void UtTest_Setup(void) { ADD_TEST(OS_SymTableIterator_Impl); - ADD_TEST(OS_GlobalSymbolLookup_Impl); + ADD_TEST(OS_SymbolLookup_Impl); ADD_TEST(OS_ModuleSymbolLookup_Impl); ADD_TEST(OS_SymbolTableDump_Impl); } diff --git a/src/unit-tests/osloader-test/ut_osloader_symtable_test.c b/src/unit-tests/osloader-test/ut_osloader_symtable_test.c index d96d9fe1a..5cbe943f1 100644 --- a/src/unit-tests/osloader-test/ut_osloader_symtable_test.c +++ b/src/unit-tests/osloader-test/ut_osloader_symtable_test.c @@ -96,7 +96,7 @@ void UT_os_symbol_lookup_test() UT_RETVAL(OS_SymbolLookup(&symbol_addr, 0), OS_INVALID_POINTER); /*-----------------------------------------------------*/ - /* Setup for remainder of tests */ + /* Setup for global symbol test */ if (UT_SETUP(OS_ModuleLoad(&module_id, "Mod1", UT_OS_GENERIC_MODULE_NAME2, OS_MODULE_FLAG_GLOBAL_SYMBOLS))) { /*-----------------------------------------------------*/ @@ -112,6 +112,19 @@ void UT_os_symbol_lookup_test() /* Reset test environment */ UT_TEARDOWN(OS_ModuleUnload(module_id)); } + + /*-----------------------------------------------------*/ + /* Setup for local symbol test */ + if (UT_SETUP(OS_ModuleLoad(&module_id, "Mod1", UT_OS_GENERIC_MODULE_NAME2, OS_MODULE_FLAG_LOCAL_SYMBOLS))) + { + /*-----------------------------------------------------*/ + /* #5 Nominal, Local Symbols */ + + UT_NOMINAL(OS_SymbolLookup(&symbol_addr, "module1")); + + /* Reset test environment */ + UT_TEARDOWN(OS_ModuleUnload(module_id)); + } } /*--------------------------------------------------------------------------------* From 45ab4e140953e56f77a460aee7d1ec7a20433e81 Mon Sep 17 00:00:00 2001 From: Jacob Hageman Date: Tue, 18 Jan 2022 15:34:58 -0700 Subject: [PATCH 7/8] Fix #1210, Set output in OS_stat handler --- src/ut-stubs/osapi-file-handlers.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/ut-stubs/osapi-file-handlers.c b/src/ut-stubs/osapi-file-handlers.c index ac200dbcf..b62428f65 100644 --- a/src/ut-stubs/osapi-file-handlers.c +++ b/src/ut-stubs/osapi-file-handlers.c @@ -192,13 +192,20 @@ void UT_DefaultHandler_OS_TimedWrite(void *UserObj, UT_EntryKey_t FuncKey, const void UT_DefaultHandler_OS_stat(void *UserObj, UT_EntryKey_t FuncKey, const UT_StubContext_t *Context) { os_fstat_t *filestats = UT_Hook_GetArgValueByName(Context, "filestats", os_fstat_t *); + size_t CopySize; int32 Status; UT_Stub_GetInt32StatusCode(Context, &Status); if (Status == OS_SUCCESS) { - UT_Stub_CopyToLocal(UT_KEY(OS_stat), filestats, sizeof(*filestats)); + CopySize = UT_Stub_CopyToLocal(UT_KEY(OS_stat), filestats, sizeof(*filestats)); + + /* Ensure memory is set if not provided by test */ + if (CopySize < sizeof(*filestats)) + { + memset(filestats, 0, sizeof(*filestats)); + } } } From 6375bbf608c21bc1153afbe2cb6238ccceb2de2b Mon Sep 17 00:00:00 2001 From: "Gerardo E. Cruz-Ortiz" <59618057+astrogeco@users.noreply.github.com> Date: Fri, 21 Jan 2022 14:41:43 -0500 Subject: [PATCH 8/8] Bump to v6.0.0-rc4+dev29 --- README.md | 9 +++++++++ src/os/inc/osapi-version.h | 2 +- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index cbbb80a25..b8fd9d657 100644 --- a/README.md +++ b/README.md @@ -11,6 +11,15 @@ The autogenerated OSAL user's guide can be viewed at and ### Development Build: v6.0.0-rc4+dev15 - Add Duplicate Check to Local Unit Test diff --git a/src/os/inc/osapi-version.h b/src/os/inc/osapi-version.h index 6e4ce779a..8b9fd2e14 100644 --- a/src/os/inc/osapi-version.h +++ b/src/os/inc/osapi-version.h @@ -36,7 +36,7 @@ /* * Development Build Macro Definitions */ -#define OS_BUILD_NUMBER 15 +#define OS_BUILD_NUMBER 29 #define OS_BUILD_BASELINE "v6.0.0-rc4" /*