Skip to content

Commit

Permalink
Merge pull request #2256 from jphickey/fix-2240-memaddress-handling
Browse files Browse the repository at this point in the history
Fix #2240, improve 64-bit memory address handling in CMD/TLM
  • Loading branch information
dzbaker committed Mar 16, 2023
2 parents e35c3da + 7fa0143 commit 45984f1
Show file tree
Hide file tree
Showing 19 changed files with 159 additions and 154 deletions.
15 changes: 0 additions & 15 deletions modules/cfe_assert/inc/cfe_assert.h
Original file line number Diff line number Diff line change
Expand Up @@ -110,21 +110,6 @@ typedef void (*CFE_Assert_StatusCallback_t)(uint8 MessageType, const char *Prefi
#define CFE_Assert_RESOURCEID_UNDEFINED(id) \
UtAssert_True(!CFE_RESOURCEID_TEST_DEFINED(id), "%s (0x%lx) not defined", #id, CFE_RESOURCEID_TO_ULONG(id))

/*****************************************************************************/
/**
** \brief Macro to check CFE memory size/offset for equality
**
** \par Description
** A macro that checks two memory offset/size values for equality.
**
** \par Assumptions, External Events, and Notes:
** This is a simple unsigned comparison which logs the values as hexadecimal
**
******************************************************************************/
#define CFE_Assert_MEMOFFSET_EQ(off1, off2) \
UtAssert_GenericUnsignedCompare(off1, UtAssert_Compare_EQ, off2, UtAssert_Radix_HEX, __FILE__, __LINE__, \
"Offset Check: ", #off1, #off2)

/*****************************************************************************/
/**
** \brief Macro to check CFE message ID for equality
Expand Down
46 changes: 18 additions & 28 deletions modules/cfe_testcase/src/es_info_test.c
Original file line number Diff line number Diff line change
Expand Up @@ -70,24 +70,17 @@ void TestGetAppInfo(void)
TestAppInfo.FileName);
UtAssert_True(strlen(ESAppInfo.FileName) == 0, "ES App Info -> FileName = %s", ESAppInfo.FileName);

UtAssert_True(TestAppInfo.StackSize > 0, "Test App Info -> StackSz = %d", (int)TestAppInfo.StackSize);
UtAssert_True(ESAppInfo.StackSize > 0, "ES App Info -> StackSz = %d", (int)ESAppInfo.StackSize);
UtAssert_NONZERO(CFE_ES_MEMOFFSET_TO_SIZET(TestAppInfo.StackSize));
UtAssert_NONZERO(CFE_ES_MEMOFFSET_TO_SIZET(ESAppInfo.StackSize));

if (TestAppInfo.AddressesAreValid)
{
UtAssert_True(TestAppInfo.AddressesAreValid > 0, "Test App Info -> AddrsValid? = %d",
(int)TestAppInfo.AddressesAreValid);
UtAssert_True(TestAppInfo.CodeAddress > 0, "Test App Info -> CodeAddress = %ld",
(unsigned long)TestAppInfo.CodeAddress);
UtAssert_True(TestAppInfo.CodeSize > 0, "Test App Info -> CodeSize = %ld",
(unsigned long)TestAppInfo.CodeSize);
UtAssert_True(TestAppInfo.DataAddress > 0, "Test App Info -> DataAddress = %ld",
(unsigned long)TestAppInfo.DataAddress);
UtAssert_True(TestAppInfo.DataSize > 0, "Test App Info -> DataSize = %ld",
(unsigned long)TestAppInfo.DataSize);
UtAssert_True(TestAppInfo.BSSAddress > 0, "Test App Info -> BSSAddress = %ld",
(unsigned long)TestAppInfo.BSSAddress);
UtAssert_True(TestAppInfo.BSSSize > 0, "Test App Info -> BSSSize = %ld", (unsigned long)TestAppInfo.BSSSize);
UtAssert_NOT_NULL(CFE_ES_MEMADDRESS_TO_PTR(TestAppInfo.CodeAddress));
UtAssert_NONZERO(CFE_ES_MEMOFFSET_TO_SIZET(TestAppInfo.CodeSize));
UtAssert_NOT_NULL(CFE_ES_MEMADDRESS_TO_PTR(TestAppInfo.DataAddress));
UtAssert_NONZERO(CFE_ES_MEMOFFSET_TO_SIZET(TestAppInfo.DataSize));
UtAssert_NOT_NULL(CFE_ES_MEMADDRESS_TO_PTR(TestAppInfo.BSSAddress));
UtAssert_NONZERO(CFE_ES_MEMOFFSET_TO_SIZET(TestAppInfo.BSSSize));
}
else
{
Expand All @@ -102,10 +95,8 @@ void TestGetAppInfo(void)
UtAssert_True(ESAppInfo.AddressesAreValid == 0, "ES App Info -> AddrsValid? = %d",
(int)ESAppInfo.AddressesAreValid);

UtAssert_True(TestAppInfo.StartAddress > 0, "Test App Info -> StartAddress = 0x%8lx",
(unsigned long)TestAppInfo.StartAddress);
UtAssert_True(ESAppInfo.StartAddress == 0, "ES App Info -> StartAddress = 0x%8lx",
(unsigned long)ESAppInfo.StartAddress);
UtAssert_NOT_NULL(CFE_ES_MEMADDRESS_TO_PTR(TestAppInfo.StartAddress));
UtAssert_NULL(CFE_ES_MEMADDRESS_TO_PTR(ESAppInfo.StartAddress));

UtAssert_INT32_EQ(TestAppInfo.ExceptionAction, 0);
UtAssert_INT32_EQ(ESAppInfo.ExceptionAction, 1);
Expand Down Expand Up @@ -180,18 +171,17 @@ void TestGetLibInfo(void)
UtAssert_StrCmp(LibInfo.EntryPoint, "CFE_Assert_LibInit", "Lib Info -> EntryPt = %s", LibInfo.EntryPoint);
UtAssert_True(strstr(LibInfo.FileName, FileName) != NULL, "Lib Info -> FileName = %s contains %s", LibInfo.FileName,
FileName);
UtAssert_True(LibInfo.StackSize == 0, "Lib Info -> StackSz = %d", (int)LibInfo.StackSize);

UtAssert_ZERO(CFE_ES_MEMOFFSET_TO_SIZET(LibInfo.StackSize));

if (LibInfo.AddressesAreValid)
{
UtAssert_True(LibInfo.AddressesAreValid > 0, "Lib Info -> AddrsValid? = %ld",
(unsigned long)LibInfo.AddressesAreValid);
UtAssert_True(LibInfo.CodeAddress > 0, "Lib Info -> CodeAddress = %ld", (unsigned long)LibInfo.CodeAddress);
UtAssert_True(LibInfo.CodeSize > 0, "Lib Info -> CodeSize = %ld", (unsigned long)LibInfo.CodeSize);
UtAssert_True(LibInfo.DataAddress > 0, "Lib Info -> DataAddress = %ld", (unsigned long)LibInfo.DataAddress);
UtAssert_True(LibInfo.DataSize > 0, "Lib Info -> DataSize = %ld", (unsigned long)LibInfo.DataSize);
UtAssert_True(LibInfo.BSSAddress > 0, "Lib Info -> BSSAddress = %ld", (unsigned long)LibInfo.BSSAddress);
UtAssert_True(LibInfo.BSSSize > 0, "Lib Info -> BSSSize = %ld", (unsigned long)LibInfo.BSSSize);
UtAssert_NOT_NULL(CFE_ES_MEMADDRESS_TO_PTR(LibInfo.CodeAddress));
UtAssert_NONZERO(CFE_ES_MEMOFFSET_TO_SIZET(LibInfo.CodeSize));
UtAssert_NOT_NULL(CFE_ES_MEMADDRESS_TO_PTR(LibInfo.DataAddress));
UtAssert_NONZERO(CFE_ES_MEMOFFSET_TO_SIZET(LibInfo.DataSize));
UtAssert_NOT_NULL(CFE_ES_MEMADDRESS_TO_PTR(LibInfo.BSSAddress));
UtAssert_NONZERO(CFE_ES_MEMOFFSET_TO_SIZET(LibInfo.BSSSize));
}
else
{
Expand Down
4 changes: 2 additions & 2 deletions modules/cfe_testcase/src/es_mempool_test.c
Original file line number Diff line number Diff line change
Expand Up @@ -185,10 +185,10 @@ void TestMemPoolDelete(void)
UtAssert_INT32_EQ(CFE_ES_PoolCreateEx(&PoolID, Buffer, sizeof(Buffer), 0, NULL, CFE_ES_NO_MUTEX), CFE_SUCCESS);
UtAssert_INT32_EQ(CFE_ES_GetMemPoolStats(&Stats, PoolID), CFE_SUCCESS);

UtAssert_UINT32_EQ(Stats.PoolSize, sizeof(Buffer));
UtAssert_EQ(size_t, CFE_ES_MEMOFFSET_TO_SIZET(Stats.PoolSize), sizeof(Buffer));
UtAssert_UINT32_EQ(Stats.NumBlocksRequested, 0);
UtAssert_UINT32_EQ(Stats.CheckErrCtr, 0);
UtAssert_UINT32_EQ(Stats.NumFreeBytes, sizeof(Buffer));
UtAssert_EQ(size_t, CFE_ES_MEMOFFSET_TO_SIZET(Stats.NumFreeBytes), sizeof(Buffer));

UtAssert_INT32_EQ(CFE_ES_GetMemPoolStats(NULL, PoolID), CFE_ES_BAD_ARGUMENT);
UtAssert_INT32_EQ(CFE_ES_GetMemPoolStats(&Stats, CFE_ES_MEMHANDLE_UNDEFINED), CFE_ES_ERR_RESOURCEID_NOT_VALID);
Expand Down
23 changes: 12 additions & 11 deletions modules/cfe_testcase/src/tbl_content_mang_test.c
Original file line number Diff line number Diff line change
Expand Up @@ -248,22 +248,23 @@ void TestModified(void)
UtAssert_INT32_EQ(CFE_TBL_Modified(CFE_TBL_BAD_TABLE_HANDLE), CFE_TBL_ERR_INVALID_HANDLE);
}

/* Helper function to set a CFE_ES_MemOffset_t value (must be big-endian) */
void TblTest_UpdateOffset(CFE_ES_MemOffset_t *TgtVal, CFE_ES_MemOffset_t SetVal)
/* Helper function to set a 32-bit table offset value (must be big-endian) */
void TblTest_UpdateOffset(uint32 *TgtVal, size_t SetVal)
{
size_t i;
union
{
CFE_ES_MemOffset_t offset;
uint8 bytes[sizeof(CFE_ES_MemOffset_t)];
uint32 offset;
uint8 bytes[sizeof(uint32)];
} offsetbuf;

offsetbuf.bytes[3] = SetVal & 0xFF;
SetVal >>= 8;
offsetbuf.bytes[2] = SetVal & 0xFF;
SetVal >>= 8;
offsetbuf.bytes[1] = SetVal & 0xFF;
SetVal >>= 8;
offsetbuf.bytes[0] = SetVal & 0xFF;
i = sizeof(offsetbuf.bytes);
while (i > 0)
{
--i;
offsetbuf.bytes[i] = SetVal & 0xFF;
SetVal >>= 8;
}

*TgtVal = offsetbuf.offset;
}
Expand Down
28 changes: 28 additions & 0 deletions modules/core_api/eds/base_types.xml
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,34 @@
<StringDataType name="ApiName" length="${CFE_MISSION/MAX_API_LEN}" />
<StringDataType name="PathName" length="${CFE_MISSION/MAX_PATH_LEN}" />

<!--
Memory addresses in CMD/TLM: These are integer types based on the
CFE_MISSION/MEM_ADDR_SIZE_BITS configuration setting. This allows
the user to select 32-bit (traditional) or 64-bit (modern) integer
values to be used in CMD/TLM fields that store a memory address.
Note that changing from 32 to 64 will extend all containers that
use/reference this type by a proportional amount, so traditional
non-EDS 32-bit CMD/TLM definitions will NOT match when this is 64 bits.
-->
<IntegerDataType name="MemReference" shortDescription="Integer type used for CPU memory addresses, sizes and offsets">
<LongDescription>
For backward compatibility with existing CFS code this should be uint32,
but all telemetry information will be limited to 4GB in size as a result.

On 64-bit platforms this can be expanded to 64 bits which will allow larger
memory objects, but this will break compatibility with existing control
systems, and may also change the alignment/padding of some messages.

In either case this must be an unsigned type, and should be large enough
to represent the largest memory address/size in use in the CFS system.
</LongDescription>
<IntegerDataEncoding sizeInBits="${CFE_MISSION/MEM_REFERENCE_SIZE_BITS}" encoding="unsigned" />
<Range>
<MinMaxRange max="2 ^ ${CFE_MISSION/MEM_REFERENCE_SIZE_BITS}" min="0" rangeType="inclusiveMinExclusiveMax"/>
</Range>
</IntegerDataType>

</DataTypeSet>

</Package>
Expand Down
30 changes: 23 additions & 7 deletions modules/core_api/fsw/inc/cfe_es_extern_typedefs.h
Original file line number Diff line number Diff line change
Expand Up @@ -381,11 +381,21 @@ typedef uint16 CFE_ES_TaskPriority_Atom_t;
*/
typedef uint32 CFE_ES_MemOffset_t;

/*
/**
* @brief Memory Offset initializer wrapper
*
* A converter macro to use when initializing a CFE_ES_MemOffset_t
* from an integer value of a different type.
*/
#define CFE_ES_MEMOFFSET_C(x) ((CFE_ES_MemOffset_t)(x))
#define CFE_ES_MEMOFFSET_C(x) ((CFE_ES_MemOffset_t)(x))

/**
* @brief Memory Offset to integer value (size_t) wrapper
*
* A converter macro to use when interpreting a CFE_ES_MemOffset_t
* value as a "size_t" type
*/
#define CFE_ES_MEMOFFSET_TO_SIZET(x) ((size_t)(x))

/**
* @brief Type used for memory addresses in command and telemetry messages
Expand All @@ -408,15 +418,21 @@ typedef uint32 CFE_ES_MemOffset_t;
*/
typedef uint32 CFE_ES_MemAddress_t;

/*
/**
* @brief Memory Address initializer wrapper
*
* A converter macro to use when initializing a CFE_ES_MemAddress_t
* from a pointer value of a different type.
*/
#define CFE_ES_MEMADDRESS_C(x) ((CFE_ES_MemAddress_t)((cpuaddr)(x)&0xFFFFFFFF))

/**
* @brief Memory Address to pointer wrapper
*
* @note on a 64 bit platform, this macro will truncate the address such
* that it will fit into a 32-bit telemetry field. Obviously, the resulting
* value is no longer usable as a memory address after this.
* A converter macro to use when interpreting a CFE_ES_MemAddress_t
* as a pointer value.
*/
#define CFE_ES_MEMADDRESS_C(x) ((CFE_ES_MemAddress_t)((cpuaddr)(x)&0xFFFFFFFF))
#define CFE_ES_MEMADDRESS_TO_PTR(x) ((void *)(cpuaddr)(x))

/*
* Data Structures shared between API and Message (CMD/TLM) interfaces
Expand Down
12 changes: 8 additions & 4 deletions modules/core_api/fsw/inc/cfe_tbl_extern_typedefs.h
Original file line number Diff line number Diff line change
Expand Up @@ -66,13 +66,17 @@ typedef uint16 CFE_TBL_BufferSelect_Enum_t;
* @brief The definition of the header fields that are included in CFE Table Data files.
*
* This header follows the CFE_FS header and precedes the actual table data.
*
* @note The Offset and NumBytes fields in the table header are to 32 bits for
* backward compatibility with existing CFE versions. This means that even on
* 64-bit CPUs, individual table files will be limited to 4GiB in size.
*/
typedef struct CFE_TBL_File_Hdr
{
uint32 Reserved; /**< Future Use: NumTblSegments in File? */
CFE_ES_MemOffset_t Offset; /**< Byte Offset at which load should commence */
CFE_ES_MemOffset_t NumBytes; /**< Number of bytes to load into table */
char TableName[CFE_MISSION_TBL_MAX_FULL_NAME_LEN]; /**< Fully qualified name of table to load */
uint32 Reserved; /**< Future Use: NumTblSegments in File? */
uint32 Offset; /**< Byte Offset at which load should commence */
uint32 NumBytes; /**< Number of bytes to load into table */
char TableName[CFE_MISSION_TBL_MAX_FULL_NAME_LEN]; /**< Fully qualified name of table to load */
} CFE_TBL_File_Hdr_t;

#endif /* CFE_EDS_ENABLED_BUILD */
Expand Down
15 changes: 0 additions & 15 deletions modules/core_private/ut-stubs/inc/ut_support.h
Original file line number Diff line number Diff line change
Expand Up @@ -753,21 +753,6 @@ bool CFE_UtAssert_MessageCheck_Impl(bool Status, const char *File, uint32 Line,
UtAssert_GenericUnsignedCompare(CFE_RESOURCEID_TO_ULONG(id1), UtAssert_Compare_EQ, CFE_RESOURCEID_TO_ULONG(id2), \
UtAssert_Radix_HEX, __FILE__, __LINE__, "Resource ID Check: ", #id1, #id2)

/*****************************************************************************/
/**
** \brief Macro to check CFE memory size/offset for equality
**
** \par Description
** A macro that checks two memory offset/size values for equality.
**
** \par Assumptions, External Events, and Notes:
** This is a simple unsigned comparison which logs the values as hexadecimal
**
******************************************************************************/
#define CFE_UtAssert_MEMOFFSET_EQ(off1, off2) \
UtAssert_GenericUnsignedCompare(off1, UtAssert_Compare_EQ, off2, UtAssert_Radix_HEX, __FILE__, __LINE__, \
"Offset Check: ", #off1, #off2)

/*****************************************************************************/
/**
** \brief Macro to check CFE message ID for equality
Expand Down
38 changes: 18 additions & 20 deletions modules/es/eds/cfe_es.xml
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@
</Range>
</IntegerDataType>

<IntegerDataType name="MemOffset" shortDescription="Type used for memory sizes and offsets in commands and telemetry">
<ContainerDataType name="MemOffset" shortDescription="Type used for memory sizes and offsets in commands and telemetry">
<LongDescription>
For backward compatibility with existing CFE code this should be uint32,
but all telemetry information will be limited to 4GB in size as a result.
Expand All @@ -176,13 +176,12 @@

In either case this must be an unsigned type.
</LongDescription>
<IntegerDataEncoding sizeInBits="${CFE_ES/MEM_OFFSET_SIZE_BITS}" encoding="unsigned" />
<Range>
<MinMaxRange max="2 ^ ${CFE_ES/MEM_OFFSET_SIZE_BITS}" min="0" rangeType="inclusiveMinExclusiveMax"/>
</Range>
</IntegerDataType>
<EntryList>
<Entry name="Offset" type="BASE_TYPES/MemReference" shortDescription="Memory Offset" />
</EntryList>
</ContainerDataType>

<IntegerDataType name="MemAddress" shortDescription="Type used for memory addresses in command and telemetry messages">
<ContainerDataType name="MemAddress" shortDescription="Type used for memory addresses in command and telemetry messages">
<LongDescription>
For backward compatibility with existing CFE code this should be uint32,
but if running on a 64-bit platform, addresses in telemetry will be
Expand All @@ -200,11 +199,10 @@
provides independence between the message representation and local
representation of a memory address.
</LongDescription>
<IntegerDataEncoding sizeInBits="${CFE_ES/MEM_OFFSET_SIZE_BITS}" encoding="unsigned" />
<Range>
<MinMaxRange max="2 ^ ${CFE_ES/MEM_OFFSET_SIZE_BITS}" min="0" rangeType="inclusiveMinExclusiveMax"/>
</Range>
</IntegerDataType>
<EntryList>
<Entry name="Addr" type="BASE_TYPES/MemReference" shortDescription="Memory Address" />
</EntryList>
</ContainerDataType>

<StringDataType name="char_x_CFE_ES_CDS_MAX_FULL_NAME_LEN" length="${CFE_MISSION/ES_CDS_MAX_FULL_NAME_LEN}" />

Expand Down Expand Up @@ -246,7 +244,7 @@
\cfetlmmnemonic \ES_APPFILENAME
</LongDescription>
</Entry>
<Entry name="StackSize" type="BASE_TYPES/uint32" shortDescription="The Stack Size of the Application">
<Entry name="StackSize" type="MemOffset" shortDescription="The Stack Size of the Application">
<LongDescription>
\cfetlmmnemonic \ES_STACKSIZE
</LongDescription>
Expand Down Expand Up @@ -353,7 +351,7 @@

<ContainerDataType name="BlockStats" shortDescription="Memory Pool Statistics data type">
<EntryList>
<Entry name="BlockSize" type="BASE_TYPES/uint32" shortDescription="Number of bytes in each of these blocks" />
<Entry name="BlockSize" type="MemOffset" shortDescription="Number of bytes in each of these blocks" />
<Entry name="NumCreated" type="BASE_TYPES/uint32" shortDescription="Number of Memory Blocks of this size created" />
<Entry name="NumFree" type="BASE_TYPES/uint32" shortDescription="Number of Memory Blocks of this size that are free" />
</EntryList>
Expand Down Expand Up @@ -432,7 +430,7 @@
<Entry name="Application" type="BASE_TYPES/ApiName" shortDescription="Name of Application to be started" />
<Entry name="AppEntryPoint" type="BASE_TYPES/ApiName" shortDescription="Symbolic name of Application's entry point" />
<Entry name="AppFileName" type="BASE_TYPES/PathName" shortDescription="Full path and filename of Application's executable image" />
<Entry name="StackSize" type="BASE_TYPES/uint32" shortDescription="Desired stack size for the new application" />
<Entry name="StackSize" type="MemOffset" shortDescription="Desired stack size for the new application" />
<Entry name="ExceptionAction" type="ExceptionAction">
<LongDescription>
\brief #CFE_ES_ExceptionAction_RESTART_APP=On exception, restart Application,
Expand Down Expand Up @@ -632,12 +630,12 @@
\cfetlmmnemonic \ES_PSPMISSIONREV
</LongDescription>
</Entry>
<Entry name="SysLogBytesUsed" type="BASE_TYPES/uint32" shortDescription="Total number of bytes used in system log">
<Entry name="SysLogBytesUsed" type="MemOffset" shortDescription="Total number of bytes used in system log">
<LongDescription>
\cfetlmmnemonic \ES_SYSLOGBYTEUSED
</LongDescription>
</Entry>
<Entry name="SysLogSize" type="BASE_TYPES/uint32" shortDescription="Total size of the system log">
<Entry name="SysLogSize" type="MemOffset" shortDescription="Total size of the system log">
<LongDescription>
\cfetlmmnemonic \ES_SYSLOGSIZE
</LongDescription>
Expand Down Expand Up @@ -752,17 +750,17 @@
\cfetlmmnemonic \ES_PERFDATA2WRITE
</LongDescription>
</Entry>
<Entry name="HeapBytesFree" type="BASE_TYPES/uint32" shortDescription="Number of free bytes remaining in the OS heap">
<Entry name="HeapBytesFree" type="MemOffset" shortDescription="Number of free bytes remaining in the OS heap">
<LongDescription>
\cfetlmmnemonic \ES_HEAPBYTESFREE
</LongDescription>
</Entry>
<Entry name="HeapBlocksFree" type="BASE_TYPES/uint32" shortDescription="Number of free blocks remaining in the OS heap">
<Entry name="HeapBlocksFree" type="MemOffset" shortDescription="Number of free blocks remaining in the OS heap">
<LongDescription>
\cfetlmmnemonic \ES_HEAPBLKSFREE
</LongDescription>
</Entry>
<Entry name="HeapMaxBlockSize" type="BASE_TYPES/uint32" shortDescription="Number of bytes in the largest free block">
<Entry name="HeapMaxBlockSize" type="MemOffset" shortDescription="Number of bytes in the largest free block">
<LongDescription>
\cfetlmmnemonic \ES_HEAPMAXBLK
</LongDescription>
Expand Down
Loading

0 comments on commit 45984f1

Please sign in to comment.