Merge pull request nasa#1038 from jphickey/fix-952-app-reload

Fix nasa#952, OSAL module flags to permit app reload
jphickey · Dec 18, 2020 · f650f2d · f650f2d
2 parents ff66d77 + 231a0cb
commit f650f2d
Show file tree

Hide file tree

Showing 5 changed files with 62 additions and 15 deletions.
diff --git a/fsw/cfe-core/src/es/cfe_es_api.c b/fsw/cfe-core/src/es/cfe_es_api.c
@@ -1117,12 +1117,9 @@ int32 CFE_ES_GetLibInfo(CFE_ES_AppInfo_t *LibInfo, CFE_ES_ResourceID_t LibId)
 */
 int32 CFE_ES_GetModuleInfo(CFE_ES_AppInfo_t *ModuleInfo, CFE_ES_ResourceID_t ResourceId)
 {
-    uint32 ResourceType;
     int32 Status;
 
-    ResourceType = CFE_ES_ResourceID_ToInteger(ResourceId);
-    ResourceType -= ResourceType & CFE_ES_RESOURCEID_MAX;
-    switch(ResourceType)
+    switch(CFE_ES_ResourceID_GetBase(ResourceId))
     {
     case CFE_ES_APPID_BASE:
         Status = CFE_ES_GetAppInfo(ModuleInfo, ResourceId);

diff --git a/fsw/cfe-core/src/es/cfe_es_apps.c b/fsw/cfe-core/src/es/cfe_es_apps.c
@@ -369,25 +369,53 @@ int32 CFE_ES_ParseFileEntry(const char **TokenList, uint32 NumTokens)
 **
 **-------------------------------------------------------------------------------------
 */
-int32 CFE_ES_LoadModule(const CFE_ES_ModuleLoadParams_t* LoadParams, CFE_ES_ModuleLoadStatus_t *LoadStatus)
+int32 CFE_ES_LoadModule(CFE_ES_ResourceID_t ResourceId, const CFE_ES_ModuleLoadParams_t* LoadParams, CFE_ES_ModuleLoadStatus_t *LoadStatus)
 {
     osal_id_t ModuleId;
     cpuaddr StartAddr;
     int32 ReturnCode;
     int32 StatusCode;
+    uint32 LoadFlags;
 
+    LoadFlags = 0;
     StartAddr = 0;
     ReturnCode = CFE_SUCCESS;
 
     if (LoadParams->FileName[0] != 0)
     {
+        switch(CFE_ES_ResourceID_GetBase(ResourceId))
+        {
+        case CFE_ES_APPID_BASE:
+            /* 
+             * Apps should not typically have symbols exposed to other apps.
+             * 
+             * Keeping symbols local/private may help ensure this module is unloadable
+             * in the future, depending on underlying OS/loader implementation.
+             */
+            LoadFlags |= OS_MODULE_FLAG_LOCAL_SYMBOLS;
+            break;
+        case CFE_ES_LIBID_BASE:
+            /* 
+             * Libraries need to have their symbols exposed to other apps.
+             * 
+             * Note on some OS/loader implementations this may make it so the module
+             * cannot be unloaded, if there is no way to ensure that symbols
+             * are not being referenced.  CFE does not currently support unloading
+             * of libraries for this reason, among others.
+             */
+            LoadFlags |= OS_MODULE_FLAG_GLOBAL_SYMBOLS;
+            break;
+        default:
+            break;
+        }
+
         /*
-         ** Load the module via OSAL.
+         * Load the module via OSAL.
          */
         StatusCode = OS_ModuleLoad ( &ModuleId,
                 LoadParams->Name,
                 LoadParams->FileName,
-                OS_MODULE_FLAG_GLOBAL_SYMBOLS );
+                LoadFlags );
 
         if (StatusCode != OS_SUCCESS)
         {
@@ -403,11 +431,11 @@ int32 CFE_ES_LoadModule(const CFE_ES_ModuleLoadParams_t* LoadParams, CFE_ES_Modu
     }
 
     /*
-     ** If the Load was OK, then lookup the address of the entry point
+     * If the Load was OK, then lookup the address of the entry point
      */
     if (ReturnCode == CFE_SUCCESS && LoadParams->EntryPoint[0] != 0)
     {
-        StatusCode = OS_SymbolLookup(&StartAddr, LoadParams->EntryPoint);
+        StatusCode = OS_ModuleSymbolLookup(ModuleId, &StartAddr, LoadParams->EntryPoint);
         if (StatusCode != OS_SUCCESS)
         {
             CFE_ES_WriteToSysLog("ES Startup: Could not find symbol:%s. EC = 0x%08X\n",
@@ -717,7 +745,7 @@ int32 CFE_ES_AppCreate(CFE_ES_ResourceID_t *ApplicationIdPtr,
    /*
     * Load the module based on StartParams configured above.
     */
-   Status = CFE_ES_LoadModule(&AppRecPtr->StartParams.BasicInfo, &AppRecPtr->ModuleInfo);
+   Status = CFE_ES_LoadModule(PendingAppId, &AppRecPtr->StartParams.BasicInfo, &AppRecPtr->ModuleInfo);
 
    /*
     * If the Load was OK, then complete the initialization
@@ -883,7 +911,7 @@ int32 CFE_ES_LoadLibrary(CFE_ES_ResourceID_t       *LibraryIdPtr,
    /*
     * Load the module based on StartParams configured above.
     */
-   Status = CFE_ES_LoadModule(&LibSlotPtr->BasicInfo, &LibSlotPtr->ModuleInfo);
+   Status = CFE_ES_LoadModule(PendingLibId, &LibSlotPtr->BasicInfo, &LibSlotPtr->ModuleInfo);
    if (Status == CFE_SUCCESS)
    {
       FunctionPointer = (CFE_ES_LibraryEntryFuncPtr_t)LibSlotPtr->ModuleInfo.EntryAddress;

diff --git a/fsw/cfe-core/src/es/cfe_es_apps.h b/fsw/cfe-core/src/es/cfe_es_apps.h
@@ -198,7 +198,7 @@ int32 CFE_ES_ParseFileEntry(const char **TokenList, uint32 NumTokens);
 ** This only loads the code and looks up relevent runtime information.
 ** It does not start any tasks.
 */
-int32 CFE_ES_LoadModule(const CFE_ES_ModuleLoadParams_t* LoadParams, CFE_ES_ModuleLoadStatus_t *LoadStatus);
+int32 CFE_ES_LoadModule(CFE_ES_ResourceID_t ResourceId, const CFE_ES_ModuleLoadParams_t* LoadParams, CFE_ES_ModuleLoadStatus_t *LoadStatus);
 
 /*
 ** Internal function to determine the entry point of an app.

diff --git a/fsw/cfe-core/src/es/cfe_es_resource.h b/fsw/cfe-core/src/es/cfe_es_resource.h
@@ -55,12 +55,34 @@
 #define CFE_ES_RESOURCEID_MAX   ((1 << CFE_ES_RESOURCEID_SHIFT)-1)
 #define CFE_ES_RESOURCEID_MARK  (0x02000000)
 
+/** 
+ * @defgroup CFEESResourceIDBase ES Resource ID base values 
+ * @{
+ */
 #define CFE_ES_APPID_BASE       (CFE_ES_RESOURCEID_MARK | ((OS_OBJECT_TYPE_USER+1) << CFE_ES_RESOURCEID_SHIFT))
 #define CFE_ES_LIBID_BASE       (CFE_ES_RESOURCEID_MARK | ((OS_OBJECT_TYPE_USER+2) << CFE_ES_RESOURCEID_SHIFT))
 #define CFE_ES_COUNTID_BASE     (CFE_ES_RESOURCEID_MARK | ((OS_OBJECT_TYPE_USER+3) << CFE_ES_RESOURCEID_SHIFT))
 #define CFE_ES_POOLID_BASE      (CFE_ES_RESOURCEID_MARK | ((OS_OBJECT_TYPE_USER+4) << CFE_ES_RESOURCEID_SHIFT))
 #define CFE_ES_CDSBLOCKID_BASE  (CFE_ES_RESOURCEID_MARK | ((OS_OBJECT_TYPE_USER+5) << CFE_ES_RESOURCEID_SHIFT))
+/** @} */
 
+/**
+ * @brief Get the Base value (type/category) from a resource ID value
+ *
+ * This masks out the ID serial number to obtain the base value, which is different
+ * for each resource type.
+ * 
+ * @note The value is NOT shifted or otherwise adjusted.  It should match one of the
+ * defined base values in @ref CFEESResourceIDBase.
+ *
+ * @param[in]   ResourceId   the resource ID to decode
+ * @returns     The base value associated with that ID
+ */
+static inline uint32 CFE_ES_ResourceID_GetBase(CFE_ES_ResourceID_t ResourceId)
+{
+    uint32 ResourceType = CFE_ES_ResourceID_ToInteger(ResourceId);
+    return (ResourceType - (ResourceType & CFE_ES_RESOURCEID_MAX));
+}
 
 /**
  * @brief Locate the next resource ID which does not map to an in-use table entry

diff --git a/fsw/cfe-core/unit-test/es_UT.c b/fsw/cfe-core/unit-test/es_UT.c
@@ -1389,7 +1389,7 @@ void TestApps(void)
      * cannot be found
      */
     ES_ResetUnitTest();
-    UT_SetDeferredRetcode(UT_KEY(OS_SymbolLookup), 1, -1);
+    UT_SetDeferredRetcode(UT_KEY(OS_ModuleSymbolLookup), 1, -1);
     Return = CFE_ES_AppCreate(&Id,
                               "ut/filename.x",
                               "EntryPoint",
@@ -1405,7 +1405,7 @@ void TestApps(void)
      * cannot be found and module unload fails
      */
     ES_ResetUnitTest();
-    UT_SetDeferredRetcode(UT_KEY(OS_SymbolLookup), 1, -1);
+    UT_SetDeferredRetcode(UT_KEY(OS_ModuleSymbolLookup), 1, -1);
     UT_SetDeferredRetcode(UT_KEY(OS_ModuleUnload), 1, -1);
     Return = CFE_ES_AppCreate(&Id,
                               "ut/filename.x",
@@ -2272,7 +2272,7 @@ void TestLibs(void)
      * entry point symbol cannot be found
      */
     ES_ResetUnitTest();
-    UT_SetDeferredRetcode(UT_KEY(OS_SymbolLookup), 1, -1);
+    UT_SetDeferredRetcode(UT_KEY(OS_ModuleSymbolLookup), 1, -1);
     Return = CFE_ES_LoadLibrary(&Id,
                                 "/cf/apps/tst_lib.bundle",
                                 "TST_LIB_Init",