From 5ff05e356157fbf86e4b5d9a3605b1fe695cc488 Mon Sep 17 00:00:00 2001 From: Joseph Hickey Date: Fri, 7 May 2021 13:05:37 -0400 Subject: [PATCH] Fix #989, add local mutex to BSP console Adds a real mutex for use with low level BSP console output. This needs to be actually implemented in the BSP layer, so it will be used by UtAssert outputs (pass/fail message) in addition to OS_printf(). This also converts OS_DEBUG to use the same console output rather than calling fprintf directly. The combination of a mutex and all common print outputs (UtAssert, OS_printf, OS_DEBUG) going through the same path makes the test output on VxWorks much more coherent. --- src/bsp/generic-linux/src/bsp_start.c | 55 ++++++++++++++++++- .../src/generic_linux_bsp_internal.h | 5 +- src/bsp/generic-vxworks/src/bsp_start.c | 47 ++++++++++++++++ .../src/generic_vxworks_bsp_internal.h | 17 ++++++ src/bsp/pc-rtems/src/bsp_start.c | 39 +++++++++++++ src/bsp/pc-rtems/src/pcrtems_bsp_internal.h | 7 ++- src/bsp/shared/inc/bsp-impl.h | 22 ++++++++ src/os/portable/os-impl-console-bsp.c | 5 ++ src/os/shared/src/osapi-debug.c | 19 ++++++- .../ut-stubs/inc/OCS_bsp-impl.h | 6 ++ .../ut-stubs/override_inc/bsp-impl.h | 2 + .../ut-stubs/src/bsp-console-impl-stubs.c | 16 ++++++ ut_assert/src/utbsp.c | 7 +++ 13 files changed, 239 insertions(+), 8 deletions(-) diff --git a/src/bsp/generic-linux/src/bsp_start.c b/src/bsp/generic-linux/src/bsp_start.c index 71b6afeb3..7ebea804d 100644 --- a/src/bsp/generic-linux/src/bsp_start.c +++ b/src/bsp/generic-linux/src/bsp_start.c @@ -49,8 +49,10 @@ OS_BSP_GenericLinuxGlobalData_t OS_BSP_GenericLinuxGlobal; --------------------------------------------------------- */ void OS_BSP_Initialize(void) { - FILE *fp; - char buffer[32]; + FILE * fp; + char buffer[32]; + pthread_mutexattr_t mutex_attr; + int status; /* * If not running as root, check /proc/sys/fs/mqueue/msg_max @@ -76,6 +78,53 @@ void OS_BSP_Initialize(void) fclose(fp); } } + + /* Initialize the low level access mutex (w/priority inheritance) */ + status = pthread_mutexattr_init(&mutex_attr); + if (status < 0) + { + BSP_DEBUG("pthread_mutexattr_init: %s\n", strerror(status)); + } + status = pthread_mutexattr_setprotocol(&mutex_attr, PTHREAD_PRIO_INHERIT); + if (status < 0) + { + BSP_DEBUG("pthread_mutexattr_setprotocol: %s\n", strerror(status)); + } + status = pthread_mutex_init(&OS_BSP_GenericLinuxGlobal.ConsoleMutex, &mutex_attr); + if (status < 0) + { + BSP_DEBUG("pthread_mutex_init: %s\n", strerror(status)); + } +} + +/*---------------------------------------------------------------- + OS_BSP_Lock_Impl + See full description in header + ------------------------------------------------------------------*/ +void OS_BSP_Lock_Impl(void) +{ + int status; + + status = pthread_mutex_lock(&OS_BSP_GenericLinuxGlobal.ConsoleMutex); + if (status < 0) + { + BSP_DEBUG("pthread_mutex_lock: %s\n", strerror(status)); + } +} + +/*---------------------------------------------------------------- + OS_BSP_Unlock_Impl + See full description in header + ------------------------------------------------------------------*/ +void OS_BSP_Unlock_Impl(void) +{ + int status; + + status = pthread_mutex_unlock(&OS_BSP_GenericLinuxGlobal.ConsoleMutex); + if (status < 0) + { + BSP_DEBUG("pthread_mutex_unlock: %s\n", strerror(status)); + } } /* --------------------------------------------------------- @@ -166,7 +215,7 @@ int main(int argc, char *argv[]) } /* - * Auto-Create any missing FS_BASED mount points specified in OS_VolumeTable + * Perform any other BSP-specific initialization */ OS_BSP_Initialize(); diff --git a/src/bsp/generic-linux/src/generic_linux_bsp_internal.h b/src/bsp/generic-linux/src/generic_linux_bsp_internal.h index 716c09b50..14dd18e17 100644 --- a/src/bsp/generic-linux/src/generic_linux_bsp_internal.h +++ b/src/bsp/generic-linux/src/generic_linux_bsp_internal.h @@ -33,12 +33,15 @@ #include "osapi-error.h" #include "bsp-impl.h" +#include + /* ** BSP types */ typedef struct { - bool EnableTermControl; /**< Will be set "true" when invoked from a TTY device, false otherwise */ + bool EnableTermControl; /**< Will be set "true" when invoked from a TTY device, false otherwise */ + pthread_mutex_t ConsoleMutex; } OS_BSP_GenericLinuxGlobalData_t; /* diff --git a/src/bsp/generic-vxworks/src/bsp_start.c b/src/bsp/generic-vxworks/src/bsp_start.c index 55d56c4fc..caa33a333 100644 --- a/src/bsp/generic-vxworks/src/bsp_start.c +++ b/src/bsp/generic-vxworks/src/bsp_start.c @@ -30,9 +30,44 @@ */ #include #include +#include #include "generic_vxworks_bsp_internal.h" +OS_BSP_GenericVxWorksGlobalData_t OS_BSP_GenericVxWorksGlobal; + +/* --------------------------------------------------------- + OS_BSP_Lock_Impl() + + Helper function to get exclusive access to BSP + --------------------------------------------------------- */ +void OS_BSP_Lock_Impl(void) +{ + int status; + + status = semTake(OS_BSP_GenericVxWorksGlobal.AccessMutex, WAIT_FOREVER); + if (status != OK) + { + BSP_DEBUG("semTake: errno=%d\n", errno); + } +} + +/* --------------------------------------------------------- + OS_BSP_Unlock_Impl() + + Helper function to release exclusive access to BSP + --------------------------------------------------------- */ +void OS_BSP_Unlock_Impl(void) +{ + int status; + + status = semGive(OS_BSP_GenericVxWorksGlobal.AccessMutex); + if (status != OK) + { + BSP_DEBUG("semGive: errno=%d\n", errno); + } +} + /* --------------------------------------------------------- OS_BSP_Shutdown_Impl() @@ -63,6 +98,18 @@ int OS_BSPMain(void) * Initially clear the global object (this contains return code) */ memset(&OS_BSP_Global, 0, sizeof(OS_BSP_Global)); + memset(&OS_BSP_GenericVxWorksGlobal, 0, sizeof(OS_BSP_GenericVxWorksGlobal)); + + /* + * Initialize the low level access sem + */ + OS_BSP_GenericVxWorksGlobal.AccessMutex = + semMInitialize(OS_BSP_GenericVxWorksGlobal.AccessMutexMem, SEM_Q_PRIORITY | SEM_INVERSION_SAFE); + + if (OS_BSP_GenericVxWorksGlobal.AccessMutex == (SEM_ID)0) + { + BSP_DEBUG("semMInitalize: errno=%d\n", errno); + } /* * Call application specific entry point. diff --git a/src/bsp/generic-vxworks/src/generic_vxworks_bsp_internal.h b/src/bsp/generic-vxworks/src/generic_vxworks_bsp_internal.h index 084c0345e..d6d5e6a50 100644 --- a/src/bsp/generic-vxworks/src/generic_vxworks_bsp_internal.h +++ b/src/bsp/generic-vxworks/src/generic_vxworks_bsp_internal.h @@ -33,4 +33,21 @@ */ #include "bsp-impl.h" +#include + +/* +** BSP types +*/ +typedef struct +{ + SEM_ID AccessMutex; + VX_MUTEX_SEMAPHORE(AccessMutexMem); + +} OS_BSP_GenericVxWorksGlobalData_t; + +/* + * Global Data object + */ +extern OS_BSP_GenericVxWorksGlobalData_t OS_BSP_GenericVxWorksGlobal; + #endif /* GENERIC_VXWORKS_BSP_INTERNAL_H */ diff --git a/src/bsp/pc-rtems/src/bsp_start.c b/src/bsp/pc-rtems/src/bsp_start.c index cb2e43542..1d0ef9789 100644 --- a/src/bsp/pc-rtems/src/bsp_start.c +++ b/src/bsp/pc-rtems/src/bsp_start.c @@ -169,6 +169,17 @@ void OS_BSP_Setup(void) } } + /* + * Initialize the low level access sem + */ + status = rtems_semaphore_create(rtems_build_name('B', 'S', 'P', '\0'), 1, + RTEMS_PRIORITY | RTEMS_BINARY_SEMAPHORE | RTEMS_INHERIT_PRIORITY, 0, + &OS_BSP_PcRtemsGlobal.AccessMutex); + if (status != RTEMS_SUCCESSFUL) + { + BSP_DEBUG("rtems_semaphore_create: %s\n", rtems_status_text(status)); + } + /* ** Create the RTEMS Root file system */ @@ -248,6 +259,34 @@ void OS_BSP_Setup(void) printf("\n\n"); } +/*---------------------------------------------------------------- + OS_BSP_Lock_Impl + See full description in header + ------------------------------------------------------------------*/ +void OS_BSP_Lock_Impl(void) +{ + rtems_status_code status; + status = rtems_semaphore_obtain(OS_BSP_PcRtemsGlobal.AccessMutex, RTEMS_WAIT, RTEMS_NO_TIMEOUT); + if (status != RTEMS_SUCCESSFUL) + { + BSP_DEBUG("rtems_semaphore_obtain: %s\n", rtems_status_text(status)); + } +} + +/*---------------------------------------------------------------- + OS_BSP_Unlock_Impl + See full description in header + ------------------------------------------------------------------*/ +void OS_BSP_Unlock_Impl(void) +{ + rtems_status_code status; + status = rtems_semaphore_release(OS_BSP_PcRtemsGlobal.AccessMutex); + if (status != RTEMS_SUCCESSFUL) + { + BSP_DEBUG("rtems_semaphore_release: %s\n", rtems_status_text(status)); + } +} + /* --------------------------------------------------------- OS_BSP_GetReturnStatus() diff --git a/src/bsp/pc-rtems/src/pcrtems_bsp_internal.h b/src/bsp/pc-rtems/src/pcrtems_bsp_internal.h index 653c62fb6..90669c809 100644 --- a/src/bsp/pc-rtems/src/pcrtems_bsp_internal.h +++ b/src/bsp/pc-rtems/src/pcrtems_bsp_internal.h @@ -33,6 +33,8 @@ */ #include "bsp-impl.h" +#include + /* * BSP compile-time tuning */ @@ -64,8 +66,9 @@ */ typedef struct { - char UserArgBuffer[RTEMS_MAX_CMDLINE]; - bool BatchMode; + char UserArgBuffer[RTEMS_MAX_CMDLINE]; + bool BatchMode; + rtems_id AccessMutex; } OS_BSP_PcRtemsGlobalData_t; /* diff --git a/src/bsp/shared/inc/bsp-impl.h b/src/bsp/shared/inc/bsp-impl.h index d6aa87da5..d2e7cfcbc 100644 --- a/src/bsp/shared/inc/bsp-impl.h +++ b/src/bsp/shared/inc/bsp-impl.h @@ -108,6 +108,28 @@ extern OS_BSP_GlobalData_t OS_BSP_Global; /* INTERNAL BSP IMPLEMENTATION FUNCTIONS */ /********************************************************************/ +/*---------------------------------------------------------------- + Function: OS_BSP_Lock_Impl + + Purpose: Get exclusive access to a BSP-provided service or object + + Useful in conjuction with console output functions to avoid strings + from multiple tasks getting mixed together in the final output. + + ------------------------------------------------------------------*/ +void OS_BSP_Lock_Impl(void); + +/*---------------------------------------------------------------- + Function: OS_BSP_Unlock_Impl + + Purpose: Release exclusive access to a BSP-provided service or object + + This must be called after a call to OS_BSP_Lock_Impl() once + access is complete, to allow other tasks to use the resource. + + ------------------------------------------------------------------*/ +void OS_BSP_Unlock_Impl(void); + /*---------------------------------------------------------------- Function: OS_BSP_ConsoleOutput_Impl diff --git a/src/os/portable/os-impl-console-bsp.c b/src/os/portable/os-impl-console-bsp.c index c0ba15325..cf628950c 100644 --- a/src/os/portable/os-impl-console-bsp.c +++ b/src/os/portable/os-impl-console-bsp.c @@ -65,6 +65,9 @@ void OS_ConsoleOutput_Impl(const OS_object_token_t *token) console = OS_OBJECT_TABLE_GET(OS_console_table, *token); StartPos = console->ReadPos; EndPos = console->WritePos; + + OS_BSP_Lock_Impl(); + while (StartPos != EndPos) { if (StartPos > EndPos) @@ -87,6 +90,8 @@ void OS_ConsoleOutput_Impl(const OS_object_token_t *token) } } + OS_BSP_Unlock_Impl(); + /* Update the global with the new read location */ console->ReadPos = StartPos; } /* end OS_ConsoleOutput_Impl */ diff --git a/src/os/shared/src/osapi-debug.c b/src/os/shared/src/osapi-debug.c index f6eef77ac..5a3e2fa5f 100644 --- a/src/os/shared/src/osapi-debug.c +++ b/src/os/shared/src/osapi-debug.c @@ -41,8 +41,10 @@ */ #include "os-shared-globaldefs.h" #include "os-shared-common.h" +#include "bsp-impl.h" #define OS_DEBUG_OUTPUT_STREAM stdout +#define OS_DEBUG_MAX_LINE_LEN 132 /*---------------------------------------------------------------- * @@ -53,14 +55,27 @@ *-----------------------------------------------------------------*/ void OS_DebugPrintf(uint32 Level, const char *Func, uint32 Line, const char *Format, ...) { + char buffer[OS_DEBUG_MAX_LINE_LEN]; va_list va; if (OS_SharedGlobalVars.DebugLevel >= Level) { + /* + * Lock the console so this appears coherently, + * not mixed with other chars from other tasks + */ + OS_BSP_Lock_Impl(); + + snprintf(buffer, sizeof(buffer), "%s():%lu:", Func, (unsigned long)Line); + OS_BSP_ConsoleOutput_Impl(buffer, strlen(buffer)); + va_start(va, Format); - fprintf(OS_DEBUG_OUTPUT_STREAM, "%s():%lu:", Func, (unsigned long)Line); - vfprintf(OS_DEBUG_OUTPUT_STREAM, Format, va); + vsnprintf(buffer, sizeof(buffer), Format, va); va_end(va); + + OS_BSP_ConsoleOutput_Impl(buffer, strlen(buffer)); + + OS_BSP_Unlock_Impl(); } } /* end OS_DebugPrintf */ diff --git a/src/unit-test-coverage/ut-stubs/inc/OCS_bsp-impl.h b/src/unit-test-coverage/ut-stubs/inc/OCS_bsp-impl.h index d76e3fc75..b6eaa255f 100644 --- a/src/unit-test-coverage/ut-stubs/inc/OCS_bsp-impl.h +++ b/src/unit-test-coverage/ut-stubs/inc/OCS_bsp-impl.h @@ -55,6 +55,12 @@ /* INTERNAL BSP IMPLEMENTATION FUNCTIONS */ /********************************************************************/ +/* + * Lock and unlock stubs + */ +void OCS_OS_BSP_Lock_Impl(void); +void OCS_OS_BSP_Unlock_Impl(void); + /*---------------------------------------------------------------- Function: OS_BSP_ConsoleOutput_Impl diff --git a/src/unit-test-coverage/ut-stubs/override_inc/bsp-impl.h b/src/unit-test-coverage/ut-stubs/override_inc/bsp-impl.h index 74d7d6039..ed77d95d1 100644 --- a/src/unit-test-coverage/ut-stubs/override_inc/bsp-impl.h +++ b/src/unit-test-coverage/ut-stubs/override_inc/bsp-impl.h @@ -36,8 +36,10 @@ #define OS_BSP_CONSOLEMODE_BLUE OCS_OS_BSP_CONSOLEMODE_BLUE #define OS_BSP_CONSOLEMODE_HIGHLIGHT OCS_OS_BSP_CONSOLEMODE_HIGHLIGHT +#define OS_BSP_Lock_Impl OCS_OS_BSP_Lock_Impl #define OS_BSP_ConsoleOutput_Impl OCS_OS_BSP_ConsoleOutput_Impl #define OS_BSP_ConsoleSetMode_Impl OCS_OS_BSP_ConsoleSetMode_Impl +#define OS_BSP_Unlock_Impl OCS_OS_BSP_Unlock_Impl /********************* END bsp-impl.h diff --git a/src/unit-test-coverage/ut-stubs/src/bsp-console-impl-stubs.c b/src/unit-test-coverage/ut-stubs/src/bsp-console-impl-stubs.c index d3ed86ea3..23f369b20 100644 --- a/src/unit-test-coverage/ut-stubs/src/bsp-console-impl-stubs.c +++ b/src/unit-test-coverage/ut-stubs/src/bsp-console-impl-stubs.c @@ -30,6 +30,22 @@ #include "OCS_bsp-impl.h" +/*---------------------------------------------------------------- + Stub for OS_BSP_Lock_Impl + ------------------------------------------------------------------*/ +void OCS_OS_BSP_Lock_Impl(void) +{ + UT_DEFAULT_IMPL(OCS_OS_BSP_Lock_Impl); +} + +/*---------------------------------------------------------------- + Stub for OS_BSP_Unlock_Impl + ------------------------------------------------------------------*/ +void OCS_OS_BSP_Unlock_Impl(void) +{ + UT_DEFAULT_IMPL(OCS_OS_BSP_Unlock_Impl); +} + /*---------------------------------------------------------------- Function: OS_BSP_ConsoleOutput_Impl diff --git a/ut_assert/src/utbsp.c b/ut_assert/src/utbsp.c index 3de0efae4..d5f1b1d1b 100644 --- a/ut_assert/src/utbsp.c +++ b/ut_assert/src/utbsp.c @@ -113,6 +113,8 @@ void UT_BSP_DoText(uint8 MessageType, const char *OutputMessage) if (MsgEnabled & 1) { + OS_BSP_Lock_Impl(); + switch (MessageType) { case UTASSERT_CASETYPE_ABORT: @@ -182,6 +184,8 @@ void UT_BSP_DoText(uint8 MessageType, const char *OutputMessage) OS_BSP_ConsoleOutput_Impl(" ", 1); OS_BSP_ConsoleOutput_Impl(OutputMessage, strlen(OutputMessage)); OS_BSP_ConsoleOutput_Impl("\n", 1); + + OS_BSP_Unlock_Impl(); } /* @@ -209,7 +213,10 @@ void UT_BSP_EndTest(const UtAssert_TestCounter_t *TestCounters) snprintf(Message, sizeof(Message), "COMPLETE: %u tests Segment(s) executed\n\n", (unsigned int)TestCounters->TestSegmentCount); + + OS_BSP_Lock_Impl(); OS_BSP_ConsoleOutput_Impl(Message, strlen(Message)); + OS_BSP_Unlock_Impl(); if ((TestCounters->CaseCount[UTASSERT_CASETYPE_FAILURE] > 0) || (TestCounters->CaseCount[UTASSERT_CASETYPE_TSF] > 0) || (TestCounters->CaseCount[UTASSERT_CASETYPE_TTF] > 0))