From ba6375cc6c156e9b45c5d89b89e16f952af5f99b Mon Sep 17 00:00:00 2001 From: Joseph Hickey Date: Mon, 2 Nov 2020 11:58:38 -0500 Subject: [PATCH 1/5] Fix #637, static module unload fix Mark static modules with a different type, and check that type at unload. Only call unload low level implementation if type is dynamic. --- src/os/shared/inc/os-shared-module.h | 16 ++++++++++++---- src/os/shared/src/osapi-module.c | 17 ++++++++++++++--- 2 files changed, 26 insertions(+), 7 deletions(-) diff --git a/src/os/shared/inc/os-shared-module.h b/src/os/shared/inc/os-shared-module.h index 3c93c2345..4ce5386d4 100644 --- a/src/os/shared/inc/os-shared-module.h +++ b/src/os/shared/inc/os-shared-module.h @@ -30,12 +30,20 @@ #include +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; /* diff --git a/src/os/shared/src/osapi-module.c b/src/os/shared/src/osapi-module.c index 8b690e7d4..3f4c2af1d 100644 --- a/src/os/shared/src/osapi-module.c +++ b/src/os/shared/src/osapi-module.c @@ -232,7 +232,12 @@ int32 OS_ModuleLoad(osal_id_t *module_id, const char *module_name, const char *f * returns OS_ERR_NAME_NOT_FOUND. */ return_code = OS_ModuleLoad_Static(module_name); - if (return_code != OS_SUCCESS) + if (return_code == OS_SUCCESS) + { + /* mark this as a statically loaded module */ + OS_module_table[local_id].module_type = OS_MODULE_TYPE_STATIC; + } + else { /* * If this is NOT a static module, then the module file must be loaded by normal @@ -248,6 +253,7 @@ int32 OS_ModuleLoad(osal_id_t *module_id, const char *module_name, const char *f { /* supplied filename was valid, so store a copy for future reference */ strncpy(OS_module_table[local_id].file_name, filename, OS_MAX_PATH_LEN); + OS_module_table[local_id].module_type = OS_MODULE_TYPE_DYNAMIC; /* Now call the OS-specific implementation. This reads info from the module table. */ return_code = OS_ModuleLoad_Impl(local_id, translated_path); @@ -280,9 +286,14 @@ int32 OS_ModuleUnload(osal_id_t module_id) if (return_code == OS_SUCCESS) { /* - * Only call the implementation if the loader is enabled + * Only call the implementation if the file was actually loaded. + * If this is a static module, then this is just a placeholder and + * it means there was no file actually loaded. */ - return_code = OS_ModuleUnload_Impl(local_id); + if (OS_module_table[local_id].module_type == OS_MODULE_TYPE_DYNAMIC) + { + return_code = OS_ModuleUnload_Impl(local_id); + } /* Complete the operation via the common routine */ return_code = OS_ObjectIdFinalizeDelete(return_code, record); From c70936eadfe1ff963a2d55d2d7d5e70e87207972 Mon Sep 17 00:00:00 2001 From: Joseph Hickey Date: Wed, 4 Nov 2020 10:06:25 -0500 Subject: [PATCH 2/5] Fix #641, add flags to OS_ModuleLoad Add flags to indicate symbol visibility for OS_ModuleLoad(). By default (flags=0) symbols will have global visibility, which should be the same as existing behavior. --- src/os/inc/osapi-os-loader.h | 59 ++++++++++++- src/os/portable/os-impl-no-symtab.c | 20 ++++- src/os/portable/os-impl-posix-dl-loader.c | 31 ++++++- src/os/portable/os-impl-posix-dl-symtab.c | 81 ++++++++++++++---- src/os/shared/inc/os-shared-module.h | 16 +++- src/os/shared/src/osapi-module.c | 80 +++++++++++++---- src/os/vxworks/src/os-impl-symtab.c | 44 +++++++++- .../shared/adaptors/inc/ut-adaptor-module.h | 2 +- .../shared/adaptors/src/ut-adaptor-module.c | 4 +- .../shared/src/coveragetest-module.c | 40 ++++++--- .../ut-stubs/src/osapi-loader-impl-stubs.c | 3 +- .../vxworks/src/coveragetest-symtab.c | 28 ++++-- .../osloader-test/ut_osloader_module_test.c | 22 ++--- .../osloader-test/ut_osloader_symtable_test.c | 85 ++++++++++++++++++- .../osloader-test/ut_osloader_symtable_test.h | 1 + .../osloader-test/ut_osloader_test.c | 1 + src/ut-stubs/osapi-utstub-module.c | 36 +++++++- 17 files changed, 473 insertions(+), 80 deletions(-) diff --git a/src/os/inc/osapi-os-loader.h b/src/os/inc/osapi-os-loader.h index 811bc4cef..e71801427 100644 --- a/src/os/inc/osapi-os-loader.h +++ b/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 *SymbolAddress, const char *SymbolName); + /*-------------------------------------------------------------------------------------*/ /** * @brief Dumps the system symbol table to a file @@ -138,7 +195,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); /*-------------------------------------------------------------------------------------*/ /** diff --git a/src/os/portable/os-impl-no-symtab.c b/src/os/portable/os-impl-no-symtab.c index fb602ecae..72fe4f1b2 100644 --- a/src/os/portable/os-impl-no-symtab.c +++ b/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 */ /*---------------------------------------------------------------- * diff --git a/src/os/portable/os-impl-posix-dl-loader.c b/src/os/portable/os-impl-posix-dl-loader.c index 55aa9ac3c..a946eb65a 100644 --- a/src/os/portable/os-impl-posix-dl-loader.c +++ b/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; diff --git a/src/os/portable/os-impl-posix-dl-symtab.c b/src/os/portable/os-impl-posix-dl-symtab.c index 98d861e55..8ed33bf53 100644 --- a/src/os/portable/os-impl-posix-dl-symtab.c +++ b/src/os/portable/os-impl-posix-dl-symtab.c @@ -60,16 +60,12 @@ * Otherwise, check if the C library provides an "RTLD_DEFAULT" symbol - * This symbol is not POSIX standard but many implementations do provide it. * - * Lastly, if nothing else works, use NULL. This is technically undefined - * behavior per POSIX, but most implementations do seem to interpret this - * as referring to the complete process (base executable + all loaded modules). + * Lastly, if nothing no special handle that indicates the global symbol + * table is defined, then OS_GlobalSymbolLookup_Impl() will return + * OS_ERR_NOT_IMPLEMENTED rather than relying on undefined behavior. */ -#ifndef OSAL_DLSYM_DEFAULT_HANDLE -#ifdef RTLD_DEFAULT -#define OSAL_DLSYM_DEFAULT_HANDLE RTLD_DEFAULT -#else -#define OSAL_DLSYM_DEFAULT_HANDLE NULL -#endif +#if !defined(OSAL_DLSYM_GLOBAL_HANDLE) && defined(RTLD_DEFAULT) +#define OSAL_DLSYM_GLOBAL_HANDLE RTLD_DEFAULT #endif /**************************************************************************************** @@ -78,23 +74,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 +108,65 @@ 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) { - *SymbolAddress = (cpuaddr)Function; - status = OS_SUCCESS; + /* technically not an error per POSIX, but in practice should not happen */ + OS_DEBUG("Error: %s: dlsym() returned NULL\n", SymbolName); + } + else + { + 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; + +#ifdef OSAL_DLSYM_DEFAULT_HANDLE + status = OS_GenericSymbolLookup_Impl(OSAL_DLSYM_DEFAULT_HANDLE, SymbolAddress, SymbolName); +#else + status = OS_ERR_NOT_IMPLEMENTED; +#endif + 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 diff --git a/src/os/shared/inc/os-shared-module.h b/src/os/shared/inc/os-shared-module.h index 3c93c2345..37b766e07 100644 --- a/src/os/shared/inc/os-shared-module.h +++ b/src/os/shared/inc/os-shared-module.h @@ -85,15 +85,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 +119,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_ */ diff --git a/src/os/shared/src/osapi-module.c b/src/os/shared/src/osapi-module.c index 8b690e7d4..0487fa5da 100644 --- a/src/os/shared/src/osapi-module.c +++ b/src/os/shared/src/osapi-module.c @@ -90,7 +90,7 @@ extern OS_static_symbol_record_t OS_STATIC_SYMTABLE_SOURCE[]; * Checks for a symbol name in the static symbol table * *-----------------------------------------------------------------*/ -int32 OS_SymbolLookup_Static(cpuaddr *SymbolAddress, const char *SymbolName) +int32 OS_SymbolLookup_Static(cpuaddr *SymbolAddress, const char *SymbolName, const char *ModuleName) { int32 return_code = OS_ERR_NOT_IMPLEMENTED; OS_static_symbol_record_t *StaticSym = OS_STATIC_SYMTABLE_SOURCE; @@ -105,7 +105,8 @@ int32 OS_SymbolLookup_Static(cpuaddr *SymbolAddress, const char *SymbolName) return_code = OS_ERROR; break; } - if (strcmp(StaticSym->Name, SymbolName) == 0) + if (strcmp(StaticSym->Name, SymbolName) == 0 && + (ModuleName == NULL || strcmp(StaticSym->Module, ModuleName) == 0)) { /* found matching symbol */ *SymbolAddress = (cpuaddr)StaticSym->Address; @@ -178,7 +179,7 @@ int32 OS_ModuleAPI_Init(void) * See description in API and header file for detail * *-----------------------------------------------------------------*/ -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) { char translated_path[OS_MAX_LOCAL_PATH_LEN]; int32 return_code; @@ -220,6 +221,7 @@ int32 OS_ModuleLoad(osal_id_t *module_id, const char *module_name, const char *f memset(&OS_module_table[local_id], 0, sizeof(OS_module_internal_record_t)); strncpy(OS_module_table[local_id].module_name, module_name, OS_MAX_API_NAME); record->name_entry = OS_module_table[local_id].module_name; + OS_module_table[local_id].flags = flags; /* save user-supplied flags */ /* * Check the statically-linked module list. @@ -339,7 +341,7 @@ int32 OS_ModuleInfo(osal_id_t module_id, OS_module_prop_t *module_prop) int32 OS_SymbolLookup(cpuaddr *SymbolAddress, const char *SymbolName) { int32 return_code; - int32 status; + int32 staticsym_status; /* ** Check parameters @@ -350,10 +352,9 @@ int32 OS_SymbolLookup(cpuaddr *SymbolAddress, const char *SymbolName) } /* - * if the module loader is included, then call the - * OS symbol lookup implementation function first. + * attempt to find the symbol in the global symbol table. */ - return_code = OS_SymbolLookup_Impl(SymbolAddress, SymbolName); + return_code = OS_GlobalSymbolLookup_Impl(SymbolAddress, SymbolName); /* * If the OS call did not find the symbol or the loader is @@ -361,20 +362,15 @@ int32 OS_SymbolLookup(cpuaddr *SymbolAddress, const char *SymbolName) */ if (return_code != OS_SUCCESS) { - status = OS_SymbolLookup_Static(SymbolAddress, SymbolName); + staticsym_status = OS_SymbolLookup_Static(SymbolAddress, SymbolName, NULL); /* - * NOTE: - * The OS_ERR_NOT_IMPLEMENTED code should only be returned - * if _neither_ the SymbolLookup_Impl _nor_ the static table - * lookup capabilities are implemented. - * - * If either of these are implemented then the returned - * value should be OS_ERROR for a not-found result. + * Only overwrite the return code if static lookup was successful. + * Otherwise keep the error code from the low level implementation. */ - if (status == OS_SUCCESS || return_code == OS_ERR_NOT_IMPLEMENTED) + if (staticsym_status == OS_SUCCESS) { - return_code = status; + return_code = staticsym_status; } } @@ -382,6 +378,56 @@ int32 OS_SymbolLookup(cpuaddr *SymbolAddress, const char *SymbolName) } /* end OS_SymbolLookup */ +/*---------------------------------------------------------------- + * + * Function: OS_ModuleSymbolLookup + * + * Purpose: Implemented per public OSAL API + * See description in API and header file for detail + * + *-----------------------------------------------------------------*/ +int32 OS_ModuleSymbolLookup(osal_id_t module_id, cpuaddr *SymbolAddress, const char *SymbolName) +{ + int32 return_code; + int32 staticsym_status; + OS_common_record_t *record; + uint32 local_id; + + /* + ** Check parameters + */ + if ((SymbolAddress == NULL) || (SymbolName == NULL)) + { + return (OS_INVALID_POINTER); + } + + return_code = OS_ObjectIdGetById(OS_LOCK_MODE_GLOBAL, LOCAL_OBJID_TYPE, module_id, &local_id, &record); + if (return_code == OS_SUCCESS) + { + return_code = OS_ModuleSymbolLookup_Impl(local_id, SymbolAddress, SymbolName); + if (return_code != OS_SUCCESS) + { + /* look for a static symbol that also matches this module name */ + staticsym_status = OS_SymbolLookup_Static(SymbolAddress, SymbolName, record->name_entry); + + /* + * Only overwrite the return code if static lookup was successful. + * Otherwise keep the error code from the low level implementation. + */ + if (staticsym_status == OS_SUCCESS) + { + return_code = staticsym_status; + } + } + OS_Unlock_Global(LOCAL_OBJID_TYPE); + } + + return (return_code); + +} /* end OS_ModuleSymbolLookup */ + + + /*---------------------------------------------------------------- * * Function: OS_SymbolTableDump diff --git a/src/os/vxworks/src/os-impl-symtab.c b/src/os/vxworks/src/os-impl-symtab.c index d3ebaf85a..31b2db5c4 100644 --- a/src/os/vxworks/src/os-impl-symtab.c +++ b/src/os/vxworks/src/os-impl-symtab.c @@ -64,13 +64,13 @@ extern SYMTAB_ID sysSymTbl; /*---------------------------------------------------------------- * - * 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(SYMTAB_ID SymTab, cpuaddr *SymbolAddress, const char *SymbolName) { STATUS vxStatus; SYMBOL_DESC SymDesc; @@ -94,7 +94,7 @@ int32 OS_SymbolLookup_Impl(cpuaddr *SymbolAddress, const char *SymbolName) SymDesc.mask = SYM_FIND_BY_NAME; SymDesc.name = (char *)SymbolName; - vxStatus = symFind(sysSymTbl, &SymDesc); + vxStatus = symFind(SymTab, &SymDesc); *SymbolAddress = (cpuaddr)SymDesc.value; if (vxStatus == ERROR) @@ -104,7 +104,43 @@ int32 OS_SymbolLookup_Impl(cpuaddr *SymbolAddress, const char *SymbolName) return (OS_SUCCESS); -} /* end OS_SymbolLookup_Impl */ +} /* end OS_GenericSymbolLookup_Impl */ + +/*---------------------------------------------------------------- + * + * 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) +{ + return OS_GenericSymbolLookup_Impl(sysSymTbl, SymbolAddress, SymbolName); +} /* 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) +{ + /* + * NOTE: this is currently exactly the same as OS_GlobalSymbolLookup_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 + * offers this capability. + */ + return OS_GenericSymbolLookup_Impl(sysSymTbl, SymbolAddress, SymbolName); +} /* end OS_ModuleSymbolLookup_Impl */ + + /*---------------------------------------------------------------- * diff --git a/src/unit-test-coverage/shared/adaptors/inc/ut-adaptor-module.h b/src/unit-test-coverage/shared/adaptors/inc/ut-adaptor-module.h index 354027f89..d65a2b2b3 100644 --- a/src/unit-test-coverage/shared/adaptors/inc/ut-adaptor-module.h +++ b/src/unit-test-coverage/shared/adaptors/inc/ut-adaptor-module.h @@ -49,7 +49,7 @@ void Osapi_Internal_ResetState(void); /* A dummy function for the static symbol lookup test. Not called */ void Test_DummyFunc(void); -int32 Osapi_Call_SymbolLookup_Static(cpuaddr *SymbolAddress, const char *SymbolName); +int32 Osapi_Call_SymbolLookup_Static(cpuaddr *SymbolAddress, const char *SymbolName, const char *ModuleName); int32 Osapi_Call_ModuleLoad_Static(const char *ModuleName); #endif /* INCLUDE_UT_ADAPTOR_MODULE_H_ */ diff --git a/src/unit-test-coverage/shared/adaptors/src/ut-adaptor-module.c b/src/unit-test-coverage/shared/adaptors/src/ut-adaptor-module.c index 8963abdc7..1be553798 100644 --- a/src/unit-test-coverage/shared/adaptors/src/ut-adaptor-module.c +++ b/src/unit-test-coverage/shared/adaptors/src/ut-adaptor-module.c @@ -28,9 +28,9 @@ #include "ut-adaptor-module.h" #include "os-shared-module.h" -int32 Osapi_Call_SymbolLookup_Static(cpuaddr *SymbolAddress, const char *SymbolName) +int32 Osapi_Call_SymbolLookup_Static(cpuaddr *SymbolAddress, const char *SymbolName, const char *ModuleName) { - return OS_SymbolLookup_Static(SymbolAddress, SymbolName); + return OS_SymbolLookup_Static(SymbolAddress, SymbolName, ModuleName); } int32 Osapi_Call_ModuleLoad_Static(const char *ModuleName) diff --git a/src/unit-test-coverage/shared/src/coveragetest-module.c b/src/unit-test-coverage/shared/src/coveragetest-module.c index 7cd0146b3..69d71290e 100644 --- a/src/unit-test-coverage/shared/src/coveragetest-module.c +++ b/src/unit-test-coverage/shared/src/coveragetest-module.c @@ -68,7 +68,7 @@ void Test_OS_ModuleLoad(void) */ int32 expected = OS_SUCCESS; osal_id_t objid; - int32 actual = OS_ModuleLoad(&objid, "UT", "File"); + int32 actual = OS_ModuleLoad(&objid, "UT", "File", OS_MODULE_FLAG_GLOBAL_SYMBOLS); UtAssert_True(actual == expected, "OS_ModuleLoad() (%ld) == OS_SUCCESS", (long)actual); actual = UT_GetStubCount(UT_KEY(OS_ModuleLoad_Impl)); @@ -76,25 +76,32 @@ void Test_OS_ModuleLoad(void) OSAPI_TEST_OBJID(objid, !=, OS_OBJECT_ID_UNDEFINED); /* for a static module, it should also return a valid objid, but should NOT invoke OS_ModuleLoad_Impl */ - actual = OS_ModuleLoad(&objid, "UTS", "File2"); + actual = OS_ModuleLoad(&objid, "UTS", "File2", OS_MODULE_FLAG_GLOBAL_SYMBOLS); UtAssert_True(actual == expected, "OS_ModuleLoad() (%ld) == OS_SUCCESS", (long)actual); actual = UT_GetStubCount(UT_KEY(OS_ModuleLoad_Impl)); UtAssert_True(actual == 1, "OS_ModuleLoad_Impl() called (%ld) == 1", (long)actual); OSAPI_TEST_OBJID(objid, !=, OS_OBJECT_ID_UNDEFINED); + /* a dynamic module with local symbols */ + actual = OS_ModuleLoad(&objid, "UT", "File3", OS_MODULE_FLAG_LOCAL_SYMBOLS); + UtAssert_True(actual == expected, "OS_ModuleLoad() (%ld) == OS_SUCCESS", (long)actual); + actual = UT_GetStubCount(UT_KEY(OS_ModuleLoad_Impl)); + UtAssert_True(actual == 2, "OS_ModuleLoad_Impl() called (%ld) == 2", (long)actual); + OSAPI_TEST_OBJID(objid, !=, OS_OBJECT_ID_UNDEFINED); + /* error cases */ - actual = OS_ModuleLoad(NULL, NULL, NULL); + actual = OS_ModuleLoad(NULL, NULL, NULL, OS_MODULE_FLAG_GLOBAL_SYMBOLS); expected = OS_INVALID_POINTER; UtAssert_True(actual == expected, "OS_ModuleLoad() (%ld) == OS_INVALID_POINTER", (long)actual); UT_SetForceFail(UT_KEY(OCS_strlen), 2 + OS_MAX_API_NAME); - actual = OS_ModuleLoad(&objid, "UTS", "File2"); + actual = OS_ModuleLoad(&objid, "UTS", "File2", OS_MODULE_FLAG_GLOBAL_SYMBOLS); expected = OS_ERR_NAME_TOO_LONG; UtAssert_True(actual == expected, "OS_ModuleLoad() (%ld) == OS_ERR_NAME_TOO_LONG", (long)actual); UT_ResetState(UT_KEY(OCS_strlen)); UT_SetForceFail(UT_KEY(OS_TranslatePath), OS_ERROR); - actual = OS_ModuleLoad(&objid, "UT", "FileBad"); + actual = OS_ModuleLoad(&objid, "UT", "FileBad", OS_MODULE_FLAG_GLOBAL_SYMBOLS); expected = OS_ERROR; UtAssert_True(actual == expected, "OS_ModuleLoad() (%ld) == OS_ERROR", (long)actual); } @@ -127,8 +134,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_SymbolLookup_Impl)); - UT_SetForceFail(UT_KEY(OS_SymbolLookup_Impl), OS_ERROR); + UT_ResetState(UT_KEY(OS_GlobalSymbolLookup_Impl)); + UT_SetForceFail(UT_KEY(OS_GlobalSymbolLookup_Impl), OS_ERROR); /* this lookup should always fail */ symaddr = 0; @@ -167,8 +174,14 @@ void Test_OS_StaticSymbolLookup(void) cpuaddr addr; /* nominal */ - actual = OS_SymbolLookup_Static(&addr, "UT_staticsym"); - UtAssert_True(actual == expected, "OS_SymbolLookup_Static(name=%s) (%ld) == OS_SUCCESS", "Test_Func1", + actual = OS_SymbolLookup_Static(&addr, "UT_staticsym", NULL); + UtAssert_True(actual == expected, "OS_SymbolLookup_Static(name=%s, NULL) (%ld) == OS_SUCCESS", "Test_Func1", + (long)actual); + UtAssert_True(addr == (cpuaddr)&Test_DummyFunc, "OS_SymbolLookup_Static(address=%lx) == %lx", (unsigned long)addr, + (unsigned long)&Test_DummyFunc); + + actual = OS_SymbolLookup_Static(&addr, "UT_staticsym", "UTS"); + UtAssert_True(actual == expected, "OS_SymbolLookup_Static(name=%s, UTS) (%ld) == OS_SUCCESS", "Test_Func1", (long)actual); UtAssert_True(addr == (cpuaddr)&Test_DummyFunc, "OS_SymbolLookup_Static(address=%lx) == %lx", (unsigned long)addr, (unsigned long)&Test_DummyFunc); @@ -177,7 +190,14 @@ void Test_OS_StaticSymbolLookup(void) UtAssert_True(actual == expected, "OS_ModuleLoad_Static(name=%s) (%ld) == OS_SUCCESS", "UT", (long)actual); expected = OS_ERROR; - actual = OS_SymbolLookup_Static(&addr, "Invalid"); + actual = OS_SymbolLookup_Static(&addr, "UT_staticsym", "NoModuleMatch"); + UtAssert_True(actual == expected, "OS_SymbolLookup_Static(name=%s, NoModuleMatch) (%ld) == OS_ERROR", "Test_Func1", + (long)actual); + UtAssert_True(addr == (cpuaddr)&Test_DummyFunc, "OS_SymbolLookup_Static(address=%lx) == %lx", (unsigned long)addr, + (unsigned long)&Test_DummyFunc); + + expected = OS_ERROR; + actual = OS_SymbolLookup_Static(&addr, "Invalid", NULL); UtAssert_True(actual == expected, "OS_SymbolLookup_Static(name=%s) (%ld) == OS_ERROR", "Invalid", (long)actual); expected = OS_ERR_NAME_NOT_FOUND; diff --git a/src/unit-test-coverage/ut-stubs/src/osapi-loader-impl-stubs.c b/src/unit-test-coverage/ut-stubs/src/osapi-loader-impl-stubs.c index 8c5d7aa77..621bcab7c 100644 --- a/src/unit-test-coverage/ut-stubs/src/osapi-loader-impl-stubs.c +++ b/src/unit-test-coverage/ut-stubs/src/osapi-loader-impl-stubs.c @@ -39,5 +39,6 @@ UT_DEFAULT_STUB(OS_ModuleLoad_Impl, (uint32 module_id, const char *translated_path)) UT_DEFAULT_STUB(OS_ModuleUnload_Impl, (uint32 module_id)) UT_DEFAULT_STUB(OS_ModuleGetInfo_Impl, (uint32 module_id, OS_module_prop_t *module_prop)) -UT_DEFAULT_STUB(OS_SymbolLookup_Impl, (cpuaddr * SymbolAddress, const char *SymbolName)) +UT_DEFAULT_STUB(OS_GlobalSymbolLookup_Impl, (cpuaddr * SymbolAddress, const char *SymbolName)) +UT_DEFAULT_STUB(OS_ModuleSymbolLookup_Impl, (uint32 module_id, cpuaddr * SymbolAddress, const char *SymbolName)) UT_DEFAULT_STUB(OS_SymbolTableDump_Impl, (const char *filename, uint32 size_limit)) diff --git a/src/unit-test-coverage/vxworks/src/coveragetest-symtab.c b/src/unit-test-coverage/vxworks/src/coveragetest-symtab.c index 890fd480d..b2f107f08 100644 --- a/src/unit-test-coverage/vxworks/src/coveragetest-symtab.c +++ b/src/unit-test-coverage/vxworks/src/coveragetest-symtab.c @@ -33,16 +33,31 @@ #include #include -void Test_OS_SymbolLookup_Impl(void) +void Test_OS_GlobalSymbolLookup_Impl(void) { /* Test Case For: - * int32 OS_SymbolLookup_Impl( cpuaddr *SymbolAddress, const char *SymbolName ) + * int32 OS_GlobalSymbolLookup_Impl( cpuaddr *SymbolAddress, const char *SymbolName ) */ cpuaddr SymAddr; - OSAPI_TEST_FUNCTION_RC(OS_SymbolLookup_Impl(&SymAddr, "symname"), OS_SUCCESS); - OSAPI_TEST_FUNCTION_RC(OS_SymbolLookup_Impl(NULL, NULL), OS_INVALID_POINTER); + + OSAPI_TEST_FUNCTION_RC(OS_GlobalSymbolLookup_Impl(&SymAddr, "symname"), OS_SUCCESS); + OSAPI_TEST_FUNCTION_RC(OS_GlobalSymbolLookup_Impl(NULL, NULL), OS_INVALID_POINTER); + UT_SetForceFail(UT_KEY(OCS_symFind), OCS_ERROR); + OSAPI_TEST_FUNCTION_RC(OS_GlobalSymbolLookup_Impl(&SymAddr, "symname"), OS_ERROR); + +} + +void Test_OS_ModuleSymbolLookup_Impl(void) +{ + /* Test Case For: + * int32 OS_ModuleSymbolLookup_Impl( uint32 local_id, cpuaddr *SymbolAddress, const char *SymbolName ) + */ + cpuaddr SymAddr; + + OSAPI_TEST_FUNCTION_RC(OS_ModuleSymbolLookup_Impl(0, &SymAddr, "symname"), OS_SUCCESS); + OSAPI_TEST_FUNCTION_RC(OS_ModuleSymbolLookup_Impl(0, NULL, NULL), OS_INVALID_POINTER); UT_SetForceFail(UT_KEY(OCS_symFind), OCS_ERROR); - OSAPI_TEST_FUNCTION_RC(OS_SymbolLookup_Impl(&SymAddr, "symname"), OS_ERROR); + OSAPI_TEST_FUNCTION_RC(OS_ModuleSymbolLookup_Impl(0, &SymAddr, "symname"), OS_ERROR); } void Test_OS_SymTableIterator_Impl(void) @@ -101,6 +116,7 @@ void Osapi_Test_Teardown(void) {} void UtTest_Setup(void) { ADD_TEST(OS_SymTableIterator_Impl); - ADD_TEST(OS_SymbolLookup_Impl); + ADD_TEST(OS_GlobalSymbolLookup_Impl); + ADD_TEST(OS_ModuleSymbolLookup_Impl); ADD_TEST(OS_SymbolTableDump_Impl); } diff --git a/src/unit-tests/osloader-test/ut_osloader_module_test.c b/src/unit-tests/osloader-test/ut_osloader_module_test.c index ba8d6ffcc..9c5489097 100644 --- a/src/unit-tests/osloader-test/ut_osloader_module_test.c +++ b/src/unit-tests/osloader-test/ut_osloader_module_test.c @@ -79,7 +79,7 @@ void UT_os_module_load_test() /*-----------------------------------------------------*/ testDesc = "API Not implemented"; - res = OS_ModuleLoad(0, "TestModule", UT_OS_GENERIC_MODULE_NAME1); + res = OS_ModuleLoad(0, "TestModule", UT_OS_GENERIC_MODULE_NAME1, OS_MODULE_FLAG_LOCAL_SYMBOLS); if (res == OS_ERR_NOT_IMPLEMENTED) { UT_OS_TEST_RESULT(testDesc, UTASSERT_CASETYPE_NA); @@ -89,7 +89,7 @@ void UT_os_module_load_test() /*-----------------------------------------------------*/ testDesc = "#1 Null-pointer-arg-1"; - res = OS_ModuleLoad(0, "TestModule", UT_OS_GENERIC_MODULE_NAME1); + res = OS_ModuleLoad(0, "TestModule", UT_OS_GENERIC_MODULE_NAME1, OS_MODULE_FLAG_LOCAL_SYMBOLS); if (res == OS_INVALID_POINTER) UT_OS_TEST_RESULT(testDesc, UTASSERT_CASETYPE_PASS); else @@ -98,7 +98,7 @@ void UT_os_module_load_test() /*-----------------------------------------------------*/ testDesc = "#2 Null-pointer-arg-2"; - res = OS_ModuleLoad(&module_id, 0, UT_OS_GENERIC_MODULE_NAME1); + res = OS_ModuleLoad(&module_id, 0, UT_OS_GENERIC_MODULE_NAME1, OS_MODULE_FLAG_LOCAL_SYMBOLS); if (res == OS_INVALID_POINTER) UT_OS_TEST_RESULT(testDesc, UTASSERT_CASETYPE_PASS); else @@ -107,7 +107,7 @@ void UT_os_module_load_test() /*-----------------------------------------------------*/ testDesc = "#3 Null-pointer-arg-3"; - res = OS_ModuleLoad(&module_id, "TestModule", 0); + res = OS_ModuleLoad(&module_id, "TestModule", 0, OS_MODULE_FLAG_LOCAL_SYMBOLS); if (res == OS_INVALID_POINTER) UT_OS_TEST_RESULT(testDesc, UTASSERT_CASETYPE_PASS); else @@ -121,7 +121,7 @@ void UT_os_module_load_test() { snprintf(module_name, sizeof(module_name), UT_OS_GENERIC_MODULE_NAME_TEMPLATE, i); snprintf(module_file_name, sizeof(module_file_name), UT_OS_GENERIC_MODULE_FILE_TEMPLATE, i); - res = OS_ModuleLoad(&module_id, module_name, module_file_name); + res = OS_ModuleLoad(&module_id, module_name, module_file_name, OS_MODULE_FLAG_LOCAL_SYMBOLS); if (res != OS_SUCCESS) { testDesc = "#4 No-free-IDs - Module Load failed"; @@ -133,7 +133,7 @@ void UT_os_module_load_test() if (test_setup_invalid == 0) { - res = OS_ModuleLoad(&module_id, "OneTooMany", UT_OS_GENERIC_MODULE_NAME2); + res = OS_ModuleLoad(&module_id, "OneTooMany", UT_OS_GENERIC_MODULE_NAME2, OS_MODULE_FLAG_LOCAL_SYMBOLS); if (res == OS_ERR_NO_FREE_IDS) UT_OS_TEST_RESULT(testDesc, UTASSERT_CASETYPE_PASS); else @@ -146,7 +146,7 @@ void UT_os_module_load_test() testDesc = "#5 Duplicate-name"; /* Setup */ - res = OS_ModuleLoad(&module_id2, "DUPLICATE", UT_OS_GENERIC_MODULE_NAME2); + res = OS_ModuleLoad(&module_id2, "DUPLICATE", UT_OS_GENERIC_MODULE_NAME2, OS_MODULE_FLAG_LOCAL_SYMBOLS); if (res != OS_SUCCESS) { testDesc = "#5 Duplicate-name - Module Load failed"; @@ -154,7 +154,7 @@ void UT_os_module_load_test() } else { - res = OS_ModuleLoad(&module_id, "DUPLICATE", UT_OS_GENERIC_MODULE_NAME2); + res = OS_ModuleLoad(&module_id, "DUPLICATE", UT_OS_GENERIC_MODULE_NAME2, OS_MODULE_FLAG_LOCAL_SYMBOLS); if (res == OS_ERR_NAME_TAKEN) UT_OS_TEST_RESULT(testDesc, UTASSERT_CASETYPE_PASS); else @@ -172,7 +172,7 @@ void UT_os_module_load_test() /*-----------------------------------------------------*/ testDesc = "#7 Nominal"; - res = OS_ModuleLoad(&module_id, "Good", UT_OS_GENERIC_MODULE_NAME2); + res = OS_ModuleLoad(&module_id, "Good", UT_OS_GENERIC_MODULE_NAME2, OS_MODULE_FLAG_LOCAL_SYMBOLS); if (res == OS_SUCCESS) UT_OS_TEST_RESULT(testDesc, UTASSERT_CASETYPE_PASS); else @@ -227,7 +227,7 @@ void UT_os_module_unload_test() testDesc = "#3 Nominal"; /* Setup */ - res = OS_ModuleLoad(&module_id, "Good", UT_OS_GENERIC_MODULE_NAME2); + res = OS_ModuleLoad(&module_id, "Good", UT_OS_GENERIC_MODULE_NAME2, OS_MODULE_FLAG_LOCAL_SYMBOLS); if (res != OS_SUCCESS) { testDesc = "#3 Nominal - Module Load failed"; @@ -293,7 +293,7 @@ void UT_os_module_info_test() testDesc = "#3 Nominal"; /* Setup */ - res = OS_ModuleLoad(&module_id, "Good", UT_OS_GENERIC_MODULE_NAME2); + res = OS_ModuleLoad(&module_id, "Good", UT_OS_GENERIC_MODULE_NAME2, OS_MODULE_FLAG_LOCAL_SYMBOLS); if (res != OS_SUCCESS) { testDesc = "#3 Nominal - Module Load failed"; 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 aa951f616..1ccf5a5c4 100644 --- a/src/unit-tests/osloader-test/ut_osloader_symtable_test.c +++ b/src/unit-tests/osloader-test/ut_osloader_symtable_test.c @@ -109,10 +109,10 @@ void UT_os_symbol_lookup_test() UT_OS_TEST_RESULT(testDesc, UTASSERT_CASETYPE_FAILURE); /*-----------------------------------------------------*/ - testDesc = "#4 Nominal"; + testDesc = "#4 Nominal, Global Symbols"; /* Setup */ - res = OS_ModuleLoad(&module_id, "Mod1", UT_OS_GENERIC_MODULE_NAME2); + res = OS_ModuleLoad(&module_id, "Mod1", UT_OS_GENERIC_MODULE_NAME2, OS_MODULE_FLAG_GLOBAL_SYMBOLS); if (res != OS_SUCCESS) { UT_OS_TEST_RESULT("#4 Nominal - Module Load failed", UTASSERT_CASETYPE_TSF); @@ -122,6 +122,8 @@ void UT_os_symbol_lookup_test() res = OS_SymbolLookup(&symbol_addr, "module1"); if (res == OS_SUCCESS) UT_OS_TEST_RESULT(testDesc, UTASSERT_CASETYPE_PASS); + else if (res == OS_ERR_NOT_IMPLEMENTED) + UT_OS_TEST_RESULT(testDesc, UTASSERT_CASETYPE_NA); else UT_OS_TEST_RESULT(testDesc, UTASSERT_CASETYPE_FAILURE); @@ -133,6 +135,85 @@ void UT_os_symbol_lookup_test() return; } +/*--------------------------------------------------------------------------------* +** Syntax: OS_ModuleSymbolLookup +** Purpose: Returns the memory address of a symbol +** Parameters: To-be-filled-in +** Returns: OS_INVALID_POINTER if any of the pointers passed in is null +** OS_ERROR if the symbol name is not found +** OS_SUCCESS if succeeded +**--------------------------------------------------------------------------------*/ + +void UT_os_module_symbol_lookup_test() +{ + int32 res = 0; + const char *testDesc; + cpuaddr symbol_addr; + osal_id_t module_id; + + /*-----------------------------------------------------*/ + testDesc = "API Not implemented"; + + res = OS_ModuleSymbolLookup(OS_OBJECT_ID_UNDEFINED, &symbol_addr, "main"); + if (res == OS_ERR_NOT_IMPLEMENTED) + { + UT_OS_TEST_RESULT(testDesc, UTASSERT_CASETYPE_NA); + goto UT_os_module_symbol_lookup_test_exit_tag; + } + + /*-----------------------------------------------------*/ + testDesc = "#1 Invalid-pointer-arg-1"; + + res = OS_ModuleSymbolLookup(OS_OBJECT_ID_UNDEFINED, 0, "main"); + if (res == OS_INVALID_POINTER) + UT_OS_TEST_RESULT(testDesc, UTASSERT_CASETYPE_PASS); + else + UT_OS_TEST_RESULT(testDesc, UTASSERT_CASETYPE_FAILURE); + + /*-----------------------------------------------------*/ + testDesc = "#2 Invalid-pointer-arg-2"; + + res = OS_ModuleSymbolLookup(OS_OBJECT_ID_UNDEFINED, &symbol_addr, 0); + if (res == OS_INVALID_POINTER) + UT_OS_TEST_RESULT(testDesc, UTASSERT_CASETYPE_PASS); + else + UT_OS_TEST_RESULT(testDesc, UTASSERT_CASETYPE_FAILURE); + + /*-----------------------------------------------------*/ + /* Setup for remainder of tests */ + res = OS_ModuleLoad(&module_id, "Mod1", UT_OS_GENERIC_MODULE_NAME2, OS_MODULE_FLAG_LOCAL_SYMBOLS); + if (res != OS_SUCCESS) + { + UT_OS_TEST_RESULT("Module Load failed", UTASSERT_CASETYPE_TSF); + goto UT_os_module_symbol_lookup_test_exit_tag; + } + + /*-----------------------------------------------------*/ + testDesc = "#3 Symbol-not-found"; + + res = OS_ModuleSymbolLookup(module_id, &symbol_addr, "ThisSymbolIsNotFound"); + if (res == OS_ERROR) + UT_OS_TEST_RESULT(testDesc, UTASSERT_CASETYPE_PASS); + else + UT_OS_TEST_RESULT(testDesc, UTASSERT_CASETYPE_FAILURE); + + /*-----------------------------------------------------*/ + testDesc = "#4 Nominal, Local Symbols"; + + res = OS_ModuleSymbolLookup(module_id, &symbol_addr, "module1"); + if (res == OS_SUCCESS) + UT_OS_TEST_RESULT(testDesc, UTASSERT_CASETYPE_PASS); + else + UT_OS_TEST_RESULT(testDesc, UTASSERT_CASETYPE_FAILURE); + + /* Reset test environment */ + res = OS_ModuleUnload(module_id); + +UT_os_module_symbol_lookup_test_exit_tag: + return; +} + + /*--------------------------------------------------------------------------------* ** Syntax: OS_SymbolTableDump ** Purpose: Dumps the system symbol table to the given filename diff --git a/src/unit-tests/osloader-test/ut_osloader_symtable_test.h b/src/unit-tests/osloader-test/ut_osloader_symtable_test.h index eae16753a..a98972cac 100644 --- a/src/unit-tests/osloader-test/ut_osloader_symtable_test.h +++ b/src/unit-tests/osloader-test/ut_osloader_symtable_test.h @@ -54,6 +54,7 @@ **--------------------------------------------------------------------------------*/ void UT_os_symbol_lookup_test(void); +void UT_os_module_symbol_lookup_test(void); void UT_os_symbol_table_dump_test(void); /*--------------------------------------------------------------------------------*/ diff --git a/src/unit-tests/osloader-test/ut_osloader_test.c b/src/unit-tests/osloader-test/ut_osloader_test.c index f3821e748..7dc7d9c6c 100644 --- a/src/unit-tests/osloader-test/ut_osloader_test.c +++ b/src/unit-tests/osloader-test/ut_osloader_test.c @@ -82,6 +82,7 @@ void UtTest_Setup(void) UtTest_Add(UT_os_module_unload_test, NULL, NULL, "OS_ModuleUnload"); UtTest_Add(UT_os_module_info_test, NULL, NULL, "OS_ModuleInfo"); + UtTest_Add(UT_os_module_symbol_lookup_test, NULL, NULL, "OS_ModuleSymbolLookup"); UtTest_Add(UT_os_symbol_lookup_test, NULL, NULL, "OS_SymbolLookup"); UtTest_Add(UT_os_symbol_table_dump_test, NULL, NULL, "OS_SymbolTableDump"); } diff --git a/src/ut-stubs/osapi-utstub-module.c b/src/ut-stubs/osapi-utstub-module.c index e5d4e4054..1b2ddc30b 100644 --- a/src/ut-stubs/osapi-utstub-module.c +++ b/src/ut-stubs/osapi-utstub-module.c @@ -80,11 +80,12 @@ int32 dummy_function(void) ** Returns either a user-defined status flag or OS_SUCCESS. ** ******************************************************************************/ -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) { UT_Stub_RegisterContext(UT_KEY(OS_ModuleLoad), module_id); UT_Stub_RegisterContext(UT_KEY(OS_ModuleLoad), module_name); UT_Stub_RegisterContext(UT_KEY(OS_ModuleLoad), filename); + UT_Stub_RegisterContextGenericArg(UT_KEY(OS_ModuleLoad), flags); int32 status; @@ -223,6 +224,39 @@ int32 OS_SymbolLookup(cpuaddr *symbol_address, const char *symbol_name) return status; } +/***************************************************************************** + * + * Stub function for OS_SymbolTableDump() + * + *****************************************************************************/ +int32 OS_ModuleSymbolLookup(osal_id_t module_id, cpuaddr *symbol_address, const char *symbol_name) +{ + UT_Stub_RegisterContextGenericArg(UT_KEY(OS_ModuleSymbolLookup), module_id); + UT_Stub_RegisterContext(UT_KEY(OS_ModuleSymbolLookup), symbol_address); + UT_Stub_RegisterContext(UT_KEY(OS_ModuleSymbolLookup), symbol_name); + + int32 status; + + /* + * Register the context so a hook can do something with the parameters + */ + + status = UT_DEFAULT_IMPL(OS_ModuleSymbolLookup); + + if (status != OS_SUCCESS) + { + *symbol_address = 0xDEADBEEFU; + } + else if (UT_Stub_CopyToLocal(UT_KEY(OS_ModuleSymbolLookup), symbol_address, sizeof(*symbol_address)) < + sizeof(*symbol_address)) + { + /* return the dummy function when test didn't register anything else */ + *symbol_address = (cpuaddr)&dummy_function; + } + + return status; +} + /***************************************************************************** * * Stub function for OS_SymbolTableDump() From a32392a1ce1bd66877727fe114a3428e4bdd43bc Mon Sep 17 00:00:00 2001 From: Joseph Hickey Date: Wed, 4 Nov 2020 13:34:29 -0500 Subject: [PATCH 3/5] Update #641, revert change to OS_SymbolLookup() This restores the original behavior when using OS_SymbolLookup on POSIX, so it doesn't return OS_ERR_NOT_IMPLEMENTED. This is required for transitional purposes. Will submit a separate bug about use platform-dependent logic here. --- src/os/portable/os-impl-posix-dl-symtab.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/os/portable/os-impl-posix-dl-symtab.c b/src/os/portable/os-impl-posix-dl-symtab.c index 8ed33bf53..2ce8e1acc 100644 --- a/src/os/portable/os-impl-posix-dl-symtab.c +++ b/src/os/portable/os-impl-posix-dl-symtab.c @@ -60,12 +60,16 @@ * Otherwise, check if the C library provides an "RTLD_DEFAULT" symbol - * This symbol is not POSIX standard but many implementations do provide it. * - * Lastly, if nothing no special handle that indicates the global symbol - * table is defined, then OS_GlobalSymbolLookup_Impl() will return - * OS_ERR_NOT_IMPLEMENTED rather than relying on undefined behavior. + * Lastly, if nothing else works, use NULL. This is technically undefined + * behavior per POSIX, but most implementations do seem to interpret this + * as referring to the complete process (base executable + all loaded modules). */ -#if !defined(OSAL_DLSYM_GLOBAL_HANDLE) && defined(RTLD_DEFAULT) -#define OSAL_DLSYM_GLOBAL_HANDLE RTLD_DEFAULT +#ifndef OSAL_DLSYM_DEFAULT_HANDLE +#ifdef RTLD_DEFAULT +#define OSAL_DLSYM_DEFAULT_HANDLE RTLD_DEFAULT +#else +#define OSAL_DLSYM_DEFAULT_HANDLE NULL +#endif #endif /**************************************************************************************** @@ -139,11 +143,7 @@ int32 OS_GlobalSymbolLookup_Impl(cpuaddr *SymbolAddress, const char *SymbolName) { int32 status; -#ifdef OSAL_DLSYM_DEFAULT_HANDLE status = OS_GenericSymbolLookup_Impl(OSAL_DLSYM_DEFAULT_HANDLE, SymbolAddress, SymbolName); -#else - status = OS_ERR_NOT_IMPLEMENTED; -#endif return status; From 3f5e8f31d7ecdbbcee17f8c4ef7f305a1cb66463 Mon Sep 17 00:00:00 2001 From: Joseph Hickey Date: Thu, 12 Nov 2020 17:23:22 -0500 Subject: [PATCH 4/5] Update #641, Doxygen comment corrections Add doxygen comment for flags parameter on OS_ModuleLoad Also correct the capitalization of parameters to the OS_ModuleSymbolLookup which caused them to be reported as undocumented. --- src/os/inc/osapi-os-loader.h | 7 ++++++- src/os/shared/src/osapi-module.c | 8 ++++---- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/src/os/inc/osapi-os-loader.h b/src/os/inc/osapi-os-loader.h index e71801427..c5a49053e 100644 --- a/src/os/inc/osapi-os-loader.h +++ b/src/os/inc/osapi-os-loader.h @@ -160,7 +160,7 @@ int32 OS_SymbolLookup(cpuaddr *symbol_address, const char *symbol_name); * @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 *SymbolAddress, const char *SymbolName); +int32 OS_ModuleSymbolLookup(osal_id_t module_id, cpuaddr *symbol_address, const char *symbol_name); /*-------------------------------------------------------------------------------------*/ /** @@ -184,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 diff --git a/src/os/shared/src/osapi-module.c b/src/os/shared/src/osapi-module.c index 1c325d85e..96bbe4abb 100644 --- a/src/os/shared/src/osapi-module.c +++ b/src/os/shared/src/osapi-module.c @@ -397,7 +397,7 @@ int32 OS_SymbolLookup(cpuaddr *SymbolAddress, const char *SymbolName) * See description in API and header file for detail * *-----------------------------------------------------------------*/ -int32 OS_ModuleSymbolLookup(osal_id_t module_id, cpuaddr *SymbolAddress, const char *SymbolName) +int32 OS_ModuleSymbolLookup(osal_id_t module_id, cpuaddr *symbol_address, const char *symbol_name) { int32 return_code; int32 staticsym_status; @@ -407,7 +407,7 @@ int32 OS_ModuleSymbolLookup(osal_id_t module_id, cpuaddr *SymbolAddress, const c /* ** Check parameters */ - if ((SymbolAddress == NULL) || (SymbolName == NULL)) + if ((symbol_address == NULL) || (symbol_name == NULL)) { return (OS_INVALID_POINTER); } @@ -415,11 +415,11 @@ int32 OS_ModuleSymbolLookup(osal_id_t module_id, cpuaddr *SymbolAddress, const c return_code = OS_ObjectIdGetById(OS_LOCK_MODE_GLOBAL, LOCAL_OBJID_TYPE, module_id, &local_id, &record); if (return_code == OS_SUCCESS) { - return_code = OS_ModuleSymbolLookup_Impl(local_id, SymbolAddress, SymbolName); + return_code = OS_ModuleSymbolLookup_Impl(local_id, symbol_address, symbol_name); if (return_code != OS_SUCCESS) { /* look for a static symbol that also matches this module name */ - staticsym_status = OS_SymbolLookup_Static(SymbolAddress, SymbolName, record->name_entry); + staticsym_status = OS_SymbolLookup_Static(symbol_address, symbol_name, record->name_entry); /* * Only overwrite the return code if static lookup was successful. From 2b5b4b18c9acc89512687718e41ab42cacbba532 Mon Sep 17 00:00:00 2001 From: astrogeco <59618057+astrogeco@users.noreply.github.com> Date: Fri, 13 Nov 2020 15:49:07 -0500 Subject: [PATCH 5/5] Bump to v5.1.0-rc1+dev75 and update ReadMe --- 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 69ce4e7ed..b8741f519 100644 --- a/README.md +++ b/README.md @@ -10,6 +10,15 @@ The autogenerated OSAL user's guide can be viewed at + ### 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: diff --git a/src/os/inc/osapi-version.h b/src/os/inc/osapi-version.h index 98d95a4a8..f8df9294b 100644 --- a/src/os/inc/osapi-version.h +++ b/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" /*