Merge pull request #652 from nasa/integration-candidate

Integration Candidate: 2020-11-10

README.md

@@ -10,6 +10,15 @@ The autogenerated OSAL user's guide can be viewed at <https://github.com/nasa/cF
 
 ## Version History
 
+### Development Build: 5.1.0-rc1+dev75
+
+- Ensure that the handle is not NULL before invoking dlclose(). In particular the handle will be NULL for static modules. Shutdown after CTRL+C occurs normally (no segfault).
+- Add a "flags" parameter to OS_ModuleLoad() to indicate the desired symbol visibility:
+    - GLOBAL (0, the default, and matches current behavior)
+    - LOCAL which hides from other modules and prevents other modules from binding to symbols in this module, thereby ensuring/preserving the ability to unload in the future
+  - CFE should use LOCAL flag for apps, and GLOBAL flags for libraries.
+- See <https://github.com/nasa/osal/pull/652>
+
 ### Development Build: 5.1.0-rc1+dev68
 
 - When `OS_DEBUG` is enabled, this adds a message if mutex give/take actions occur outside the expected sequence. This informs the user (via the debug console) if a lock is taken more than once or if a lock is given by a different task than the one that originally took it:

src/os/inc/osapi-os-loader.h

@@ -34,6 +34,44 @@
 ** Defines
 */
 
+/**
+ * @brief Requests OS_ModuleLoad() to add the symbols to the global symbol table
+ *
+ * When supplied as the "flags" argument to OS_ModuleLoad(), this indicates
+ * that the symbols in the loaded module should be added to the global symbol
+ * table.  This will make symbols in this library available for use when
+ * resolving symbols in future module loads.
+ *
+ * This is the default mode of operation for OS_ModuleLoad().
+ *
+ * @note On some operating systems, use of this option may make it difficult
+ * to unload the module in the future, if the symbols are in use by other entities.
+ *
+ */
+#define OS_MODULE_FLAG_GLOBAL_SYMBOLS       0x00
+
+/**
+ * @brief Requests OS_ModuleLoad() to keep the symbols local/private to this module
+ *
+ * When supplied as the "flags" argument to OS_ModuleLoad(), this indicates
+ * that the symbols in the loaded module should NOT be added to the global
+ * symbol table.  This means the symbols in the loaded library will not available
+ * to for use by other modules.
+ *
+ * Use this option is recommended for cases where no other entities will need
+ * to reference symbols within this module.  This helps ensure that the module
+ * can be more safely unloaded in the future, by preventing other modules from
+ * binding to it.  It also helps reduce the likelihood of symbol name conflicts
+ * among modules.
+ *
+ * @note To look up symbols within a module loaded with this flag, use
+ * OS_SymbolLookupInModule() instead of OS_SymbolLookup().  Also note that
+ * references obtained using this method are not tracked by the OS; the
+ * application must ensure that all references obtained in this manner have
+ * been cleaned up/released before unloading the module.
+ */
+#define OS_MODULE_FLAG_LOCAL_SYMBOLS        0x01
+
 /*
 ** Typedefs
 */
@@ -105,6 +143,25 @@ typedef const struct
  */
 int32 OS_SymbolLookup(cpuaddr *symbol_address, const char *symbol_name);
 
+/*-------------------------------------------------------------------------------------*/
+/**
+ * @brief Find the Address of a Symbol within a module
+ *
+ * This is similar to OS_SymbolLookup() but for a specific module ID.
+ * This should be used to look up a symbol in a module that has been
+ * loaded with the #OS_MODULE_FLAG_LOCAL_SYMBOLS flag.
+ *
+ * @param[in]  module_id      Module ID that should contain the symbol
+ * @param[out] symbol_address Set to the address of the symbol
+ * @param[in]  symbol_name    Name of the symbol to look up
+ *
+ * @return Execution status, see @ref OSReturnCodes
+ * @retval #OS_SUCCESS @copybrief OS_SUCCESS
+ * @retval #OS_ERROR if the symbol could not be found
+ * @retval #OS_INVALID_POINTER if one of the pointers passed in are NULL
+ */
+int32 OS_ModuleSymbolLookup(osal_id_t module_id, cpuaddr *symbol_address, const char *symbol_name);
+
 /*-------------------------------------------------------------------------------------*/
 /**
  * @brief Dumps the system symbol table to a file
@@ -127,9 +184,14 @@ int32 OS_SymbolTableDump(const char *filename, uint32 size_limit);
  *
  * Loads an object file into the running operating system
  *
+ * The "flags" parameter may influence how the loaded module symbols are made
+ * available for use in the application.  See #OS_MODULE_FLAG_LOCAL_SYMBOLS
+ * and #OS_MODULE_FLAG_GLOBAL_SYMBOLS for descriptions.
+ *
  * @param[out] module_id    Non-zero OSAL ID corresponding to the loaded module
  * @param[in]  module_name  Name of module
  * @param[in]  filename     File containing the object code to load
+ * @param[in]  flags        Options for the loaded module
  *
  * @return Execution status, see @ref OSReturnCodes
  * @retval #OS_SUCCESS @copybrief OS_SUCCESS
@@ -138,7 +200,7 @@ int32 OS_SymbolTableDump(const char *filename, uint32 size_limit);
  * @retval #OS_ERR_NO_FREE_IDS if the module table is full
  * @retval #OS_ERR_NAME_TAKEN if the name is in use
  */
-int32 OS_ModuleLoad(osal_id_t *module_id, const char *module_name, const char *filename);
+int32 OS_ModuleLoad(osal_id_t *module_id, const char *module_name, const char *filename, uint32 flags);
 
 /*-------------------------------------------------------------------------------------*/
 /**

src/os/inc/osapi-version.h

@@ -30,7 +30,7 @@
 /*
  * Development Build Macro Definitions
  */
-#define OS_BUILD_NUMBER   67
+#define OS_BUILD_NUMBER   75
 #define OS_BUILD_BASELINE "v5.1.0-rc1"
 
 /*

src/os/portable/os-impl-no-symtab.c

@@ -32,17 +32,31 @@
 
 /*----------------------------------------------------------------
  *
- * Function: OS_SymbolLookup_Impl
+ * Function: OS_GlobalSymbolLookup_Impl
  *
  *  Purpose: Implemented per internal OSAL API
  *           See prototype for argument/return detail
  *
  *-----------------------------------------------------------------*/
-int32 OS_SymbolLookup_Impl(cpuaddr *SymbolAddress, const char *SymbolName)
+int32 OS_GlobalSymbolLookup_Impl(cpuaddr *SymbolAddress, const char *SymbolName)
 {
     return OS_ERR_NOT_IMPLEMENTED;
 
-} /* end OS_SymbolLookup_Impl */
+} /* end OS_GlobalSymbolLookup_Impl */
+
+/*----------------------------------------------------------------
+ *
+ * Function: OS_ModuleSymbolLookup_Impl
+ *
+ *  Purpose: Implemented per internal OSAL API
+ *           See prototype for argument/return detail
+ *
+ *-----------------------------------------------------------------*/
+int32 OS_ModuleSymbolLookup_Impl(uint32 local_id, cpuaddr *SymbolAddress, const char *SymbolName)
+{
+    return OS_ERR_NOT_IMPLEMENTED;
+
+} /* end OS_ModuleSymbolLookup_Impl */
 
 /*----------------------------------------------------------------
  *

src/os/portable/os-impl-posix-dl-loader.c

@@ -64,9 +64,38 @@
 int32 OS_ModuleLoad_Impl(uint32 module_id, const char *translated_path)
 {
     int32 status = OS_ERROR;
+    int dl_mode;
+
+    /*
+     * RTLD_NOW should instruct dlopen() to resolve all the symbols in the
+     * module immediately, as opposed to waiting until they are used.
+     * The latter (lazy mode) is non-deterministic - a resolution error on
+     * a rarely-used symbol could cause a random failure far in the future.
+     */
+    dl_mode = RTLD_NOW;
+
+    if ((OS_module_table[module_id].flags & OS_MODULE_FLAG_LOCAL_SYMBOLS) != 0)
+    {
+        /*
+         * Do not add the symbols in this module to the global symbol table.
+         * This mode helps prevent any unanticipated references into this
+         * module, which can in turn prevent unloading via dlclose().
+         */
+        dl_mode |= RTLD_LOCAL;
+    }
+    else
+    {
+        /*
+         * Default mode - add symbols to the global symbol table, so they
+         * will be available to resolve symbols in future module loads.
+         * However, any such references will prevent unloading of this
+         * module via dlclose().
+         */
+        dl_mode |= RTLD_GLOBAL;
+    }
 
     dlerror();
-    OS_impl_module_table[module_id].dl_handle = dlopen(translated_path, RTLD_NOW | RTLD_GLOBAL);
+    OS_impl_module_table[module_id].dl_handle = dlopen(translated_path, dl_mode);
     if (OS_impl_module_table[module_id].dl_handle != NULL)
     {
         status = OS_SUCCESS;

src/os/portable/os-impl-posix-dl-symtab.c

@@ -78,23 +78,25 @@
 
 /*----------------------------------------------------------------
  *
- * Function: OS_SymbolLookup_Impl
+ * Function: OS_GenericSymbolLookup_Impl
  *
  *  Purpose: Implemented per internal OSAL API
  *           See prototype for argument/return detail
  *
  *-----------------------------------------------------------------*/
-int32 OS_SymbolLookup_Impl(cpuaddr *SymbolAddress, const char *SymbolName)
+int32 OS_GenericSymbolLookup_Impl(void *dl_handle, cpuaddr *SymbolAddress, const char *SymbolName)
 {
-    int32       status = OS_ERROR;
     const char *dlError; /*  Pointer to error string   */
-    void *      Function;
+    void       *Function;
+    int32       status;
+
+    status = OS_ERROR;
 
     /*
      * call dlerror() to clear any prior error that might have occurred.
      */
     dlerror();
-    Function = dlsym(OSAL_DLSYM_DEFAULT_HANDLE, SymbolName);
+    Function = dlsym(dl_handle, SymbolName);
     dlError  = dlerror();
 
     /*
@@ -110,16 +112,61 @@ int32 OS_SymbolLookup_Impl(cpuaddr *SymbolAddress, const char *SymbolName)
      * and as such all valid symbols should be non-NULL, so NULL is considered
      * an error even if the C library doesn't consider this an error.
      */
-    if (dlError == NULL && Function != NULL)
+    if (dlError != NULL)
+    {
+        OS_DEBUG("Error: %s: %s\n", SymbolName, dlError);
+    }
+    else if (Function == NULL)
+    {
+        /* technically not an error per POSIX, but in practice should not happen */
+        OS_DEBUG("Error: %s: dlsym() returned NULL\n", SymbolName);
+    }
+    else
     {
-        *SymbolAddress = (cpuaddr)Function;
-        status         = OS_SUCCESS;
+        status = OS_SUCCESS;
     }
 
+    *SymbolAddress = (cpuaddr)Function;
+
+    return status;
+}
+
+/*----------------------------------------------------------------
+ *
+ * Function: OS_GlobalSymbolLookup_Impl
+ *
+ *  Purpose: Implemented per internal OSAL API
+ *           See prototype for argument/return detail
+ *
+ *-----------------------------------------------------------------*/
+int32 OS_GlobalSymbolLookup_Impl(cpuaddr *SymbolAddress, const char *SymbolName)
+{
+    int32 status;
+
+    status = OS_GenericSymbolLookup_Impl(OSAL_DLSYM_DEFAULT_HANDLE, SymbolAddress, SymbolName);
+
     return status;
 
 } /* end OS_SymbolLookup_Impl */
 
+/*----------------------------------------------------------------
+ *
+ * Function: OS_ModuleSymbolLookup_Impl
+ *
+ *  Purpose: Implemented per internal OSAL API
+ *           See prototype for argument/return detail
+ *
+ *-----------------------------------------------------------------*/
+int32 OS_ModuleSymbolLookup_Impl(uint32 local_id, cpuaddr *SymbolAddress, const char *SymbolName)
+{
+    int32 status;
+
+    status = OS_GenericSymbolLookup_Impl(OS_impl_module_table[local_id].dl_handle, SymbolAddress, SymbolName);
+
+    return status;
+
+} /* end OS_ModuleSymbolLookup_Impl */
+
 /*----------------------------------------------------------------
  *
  * Function: OS_SymbolTableDump_Impl

src/os/shared/inc/os-shared-module.h

@@ -30,12 +30,20 @@
 
 #include <os-shared-globaldefs.h>
 
+typedef enum
+{
+    OS_MODULE_TYPE_UNKNOWN = 0, /**< Default/unspecified (reserved value) */
+    OS_MODULE_TYPE_DYNAMIC = 1, /**< Module is dynamically loaded via the OS loader */
+    OS_MODULE_TYPE_STATIC  = 2  /**< Module is statically linked and is a placeholder */
+} OS_module_type_t;
+
 typedef struct
 {
-    char    module_name[OS_MAX_API_NAME];
-    char    file_name[OS_MAX_PATH_LEN];
-    uint32  flags;
-    cpuaddr entry_point;
+    char             module_name[OS_MAX_API_NAME];
+    char             file_name[OS_MAX_PATH_LEN];
+    OS_module_type_t module_type;
+    uint32           flags;
+    cpuaddr          entry_point;
 } OS_module_internal_record_t;
 
 /*
@@ -85,15 +93,25 @@ int32 OS_ModuleUnload_Impl(uint32 module_id);
  ------------------------------------------------------------------*/
 int32 OS_ModuleGetInfo_Impl(uint32 module_id, OS_module_prop_t *module_prop);
 
+/*----------------------------------------------------------------
+   Function: OS_GlobalSymbolLookup_Impl
+
+    Purpose: Find the Address of a Symbol in the global symbol table.
+             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);
+
 /*----------------------------------------------------------------
    Function: OS_SymbolLookup_Impl
 
-    Purpose: Find the Address of a Symbol
+    Purpose: Find the Address of a Symbol within a specific module.
              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_SymbolLookup_Impl(cpuaddr *SymbolAddress, const char *SymbolName);
+int32 OS_ModuleSymbolLookup_Impl(uint32 local_id, cpuaddr *SymbolAddress, const char *SymbolName);
 
 /*----------------------------------------------------------------
    Function: OS_SymbolTableDump_Impl
@@ -109,6 +127,6 @@ int32 OS_SymbolTableDump_Impl(const char *filename, uint32 size_limit);
  * These need to be exposed for unit testing
  */
 int32 OS_ModuleLoad_Static(const char *ModuleName);
-int32 OS_SymbolLookup_Static(cpuaddr *SymbolAddress, const char *SymbolName);
+int32 OS_SymbolLookup_Static(cpuaddr *SymbolAddress, const char *SymbolName, const char *ModuleName);
 
 #endif /* INCLUDE_OS_SHARED_MODULE_H_ */