Check for add overflow only once (#467)
Update the size calculations such that we only need to check for add
overflow only once. Also, change the way we detect add overflow so that
we do not need to cause an overflow to detect an overflow.
Signed-off-by: Gaurav Aggarwal <aggarg@amazon.com>
diff --git a/portable/MemMang/heap_2.c b/portable/MemMang/heap_2.c
index f50965e..163a12e 100644
--- a/portable/MemMang/heap_2.c
+++ b/portable/MemMang/heap_2.c
@@ -62,8 +62,14 @@
/* Assumes 8bit bytes! */
#define heapBITS_PER_BYTE ( ( size_t ) 8 )
+/* Max value that fits in a size_t type. */
+#define heapSIZE_MAX ( ~( ( size_t ) 0 ) )
+
/* Check if multiplying a and b will result in overflow. */
-#define heapMULTIPLY_WILL_OVERFLOW( a, b, max ) ( ( ( a ) > 0 ) && ( ( b ) > ( ( max ) / ( a ) ) ) )
+#define heapMULTIPLY_WILL_OVERFLOW( a, b ) ( ( ( a ) > 0 ) && ( ( b ) > ( heapSIZE_MAX / ( a ) ) ) )
+
+/* Check if adding a and b will result in overflow. */
+#define heapADD_WILL_OVERFLOW( a, b ) ( ( a ) > ( heapSIZE_MAX - ( b ) ) )
/* MSB of the xBlockSize member of an BlockLink_t structure is used to track
* the allocation status of a block. When MSB of the xBlockSize member of
@@ -97,7 +103,7 @@
} BlockLink_t;
-static const uint16_t heapSTRUCT_SIZE = ( ( sizeof( BlockLink_t ) + ( portBYTE_ALIGNMENT - 1 ) ) & ~portBYTE_ALIGNMENT_MASK );
+static const uint16_t heapSTRUCT_SIZE = ( ( sizeof( BlockLink_t ) + ( portBYTE_ALIGNMENT - 1 ) ) & ~( ( size_t ) portBYTE_ALIGNMENT_MASK ) );
#define heapMINIMUM_BLOCK_SIZE ( ( size_t ) ( heapSTRUCT_SIZE * 2 ) )
/* Create a couple of list links to mark the start and end of the list. */
@@ -149,6 +155,7 @@
BlockLink_t * pxBlock, * pxPreviousBlock, * pxNewBlockLink;
PRIVILEGED_DATA static BaseType_t xHeapHasBeenInitialised = pdFALSE;
void * pvReturn = NULL;
+ size_t xAdditionalRequiredSize;
vTaskSuspendAll();
{
@@ -160,29 +167,22 @@
xHeapHasBeenInitialised = pdTRUE;
}
- /* The wanted size must be increased so it can contain a BlockLink_t
- * structure in addition to the requested amount of bytes. */
- if( ( xWantedSize > 0 ) &&
- ( ( xWantedSize + heapSTRUCT_SIZE ) > xWantedSize ) ) /* Overflow check. */
+ if( xWantedSize > 0 )
{
- xWantedSize += heapSTRUCT_SIZE;
+ /* The wanted size must be increased so it can contain a BlockLink_t
+ * structure in addition to the requested amount of bytes. Some
+ * additional increment may also be needed for alignment. */
+ xAdditionalRequiredSize = heapSTRUCT_SIZE + portBYTE_ALIGNMENT - ( xWantedSize & portBYTE_ALIGNMENT_MASK );
- /* Byte alignment required. Check for overflow. */
- if( ( xWantedSize + ( portBYTE_ALIGNMENT - ( xWantedSize & portBYTE_ALIGNMENT_MASK ) ) )
- > xWantedSize )
+ if( heapADD_WILL_OVERFLOW( xWantedSize, xAdditionalRequiredSize ) == 0 )
{
- xWantedSize += ( portBYTE_ALIGNMENT - ( xWantedSize & portBYTE_ALIGNMENT_MASK ) );
- configASSERT( ( xWantedSize & portBYTE_ALIGNMENT_MASK ) == 0 );
+ xWantedSize += xAdditionalRequiredSize;
}
else
{
xWantedSize = 0;
}
}
- else
- {
- xWantedSize = 0;
- }
/* Check the block size we are trying to allocate is not so large that the
* top bit is set. The top bit of the block size member of the BlockLink_t
@@ -320,9 +320,8 @@
size_t xSize )
{
void * pv = NULL;
- const size_t xSizeMaxValue = ~( ( size_t ) 0 );
- if( !heapMULTIPLY_WILL_OVERFLOW( xNum, xSize, xSizeMaxValue ) )
+ if( heapMULTIPLY_WILL_OVERFLOW( xNum, xSize ) == 0 )
{
pv = pvPortMalloc( xNum * xSize );
diff --git a/portable/MemMang/heap_4.c b/portable/MemMang/heap_4.c
index 28efcbb..33bb818 100644
--- a/portable/MemMang/heap_4.c
+++ b/portable/MemMang/heap_4.c
@@ -61,8 +61,14 @@
/* Assumes 8bit bytes! */
#define heapBITS_PER_BYTE ( ( size_t ) 8 )
+/* Max value that fits in a size_t type. */
+#define heapSIZE_MAX ( ~( ( size_t ) 0 ) )
+
/* Check if multiplying a and b will result in overflow. */
-#define heapMULTIPLY_WILL_OVERFLOW( a, b, max ) ( ( ( a ) > 0 ) && ( ( b ) > ( ( max ) / ( a ) ) ) )
+#define heapMULTIPLY_WILL_OVERFLOW( a, b ) ( ( ( a ) > 0 ) && ( ( b ) > ( heapSIZE_MAX / ( a ) ) ) )
+
+/* Check if adding a and b will result in overflow. */
+#define heapADD_WILL_OVERFLOW( a, b ) ( ( a ) > ( heapSIZE_MAX - ( b ) ) )
/* MSB of the xBlockSize member of an BlockLink_t structure is used to track
* the allocation status of a block. When MSB of the xBlockSize member of
@@ -132,6 +138,7 @@
{
BlockLink_t * pxBlock, * pxPreviousBlock, * pxNewBlockLink;
void * pvReturn = NULL;
+ size_t xAdditionalRequiredSize;
vTaskSuspendAll();
{
@@ -146,35 +153,25 @@
mtCOVERAGE_TEST_MARKER();
}
- /* The wanted size must be increased so it can contain a BlockLink_t
- * structure in addition to the requested amount of bytes. */
- if( ( xWantedSize > 0 ) &&
- ( ( xWantedSize + xHeapStructSize ) > xWantedSize ) ) /* Overflow check. */
+ if( xWantedSize > 0 )
{
- xWantedSize += xHeapStructSize;
+ /* The wanted size must be increased so it can contain a BlockLink_t
+ * structure in addition to the requested amount of bytes. Some
+ * additional increment may also be needed for alignment. */
+ xAdditionalRequiredSize = xHeapStructSize + portBYTE_ALIGNMENT - ( xWantedSize & portBYTE_ALIGNMENT_MASK );
- /* Ensure that blocks are always aligned. */
- if( ( xWantedSize & portBYTE_ALIGNMENT_MASK ) != 0x00 )
+ if( heapADD_WILL_OVERFLOW( xWantedSize, xAdditionalRequiredSize ) == 0 )
{
- /* Byte alignment required. Check for overflow. */
- if( ( xWantedSize + ( portBYTE_ALIGNMENT - ( xWantedSize & portBYTE_ALIGNMENT_MASK ) ) ) > xWantedSize )
- {
- xWantedSize += ( portBYTE_ALIGNMENT - ( xWantedSize & portBYTE_ALIGNMENT_MASK ) );
- configASSERT( ( xWantedSize & portBYTE_ALIGNMENT_MASK ) == 0 );
- }
- else
- {
- xWantedSize = 0;
- }
+ xWantedSize += xAdditionalRequiredSize;
}
else
{
- mtCOVERAGE_TEST_MARKER();
+ xWantedSize = 0;
}
}
else
{
- xWantedSize = 0;
+ mtCOVERAGE_TEST_MARKER();
}
/* Check the block size we are trying to allocate is not so large that the
@@ -362,9 +359,8 @@
size_t xSize )
{
void * pv = NULL;
- const size_t xSizeMaxValue = ~( ( size_t ) 0 );
- if( !heapMULTIPLY_WILL_OVERFLOW( xNum, xSize, xSizeMaxValue ) )
+ if( heapMULTIPLY_WILL_OVERFLOW( xNum, xSize ) == 0 )
{
pv = pvPortMalloc( xNum * xSize );
diff --git a/portable/MemMang/heap_5.c b/portable/MemMang/heap_5.c
index c0c4ea4..b330439 100644
--- a/portable/MemMang/heap_5.c
+++ b/portable/MemMang/heap_5.c
@@ -95,8 +95,14 @@
/* Assumes 8bit bytes! */
#define heapBITS_PER_BYTE ( ( size_t ) 8 )
+/* Max value that fits in a size_t type. */
+#define heapSIZE_MAX ( ~( ( size_t ) 0 ) )
+
/* Check if multiplying a and b will result in overflow. */
-#define heapMULTIPLY_WILL_OVERFLOW( a, b, max ) ( ( ( a ) > 0 ) && ( ( b ) > ( ( max ) / ( a ) ) ) )
+#define heapMULTIPLY_WILL_OVERFLOW( a, b ) ( ( ( a ) > 0 ) && ( ( b ) > ( heapSIZE_MAX / ( a ) ) ) )
+
+/* Check if adding a and b will result in overflow. */
+#define heapADD_WILL_OVERFLOW( a, b ) ( ( a ) > ( heapSIZE_MAX - ( b ) ) )
/* MSB of the xBlockSize member of an BlockLink_t structure is used to track
* the allocation status of a block. When MSB of the xBlockSize member of
@@ -150,6 +156,7 @@
{
BlockLink_t * pxBlock, * pxPreviousBlock, * pxNewBlockLink;
void * pvReturn = NULL;
+ size_t xAdditionalRequiredSize;
/* The heap must be initialised before the first call to
* prvPortMalloc(). */
@@ -157,35 +164,25 @@
vTaskSuspendAll();
{
- /* The wanted size is increased so it can contain a BlockLink_t
- * structure in addition to the requested amount of bytes. */
- if( ( xWantedSize > 0 ) &&
- ( ( xWantedSize + xHeapStructSize ) > xWantedSize ) ) /* Overflow check. */
+ if( xWantedSize > 0 )
{
- xWantedSize += xHeapStructSize;
+ /* The wanted size must be increased so it can contain a BlockLink_t
+ * structure in addition to the requested amount of bytes. Some
+ * additional increment may also be needed for alignment. */
+ xAdditionalRequiredSize = xHeapStructSize + portBYTE_ALIGNMENT - ( xWantedSize & portBYTE_ALIGNMENT_MASK );
- /* Ensure that blocks are always aligned */
- if( ( xWantedSize & portBYTE_ALIGNMENT_MASK ) != 0x00 )
+ if( heapADD_WILL_OVERFLOW( xWantedSize, xAdditionalRequiredSize ) == 0 )
{
- /* Byte alignment required. Check for overflow */
- if( ( xWantedSize + ( portBYTE_ALIGNMENT - ( xWantedSize & portBYTE_ALIGNMENT_MASK ) ) ) >
- xWantedSize )
- {
- xWantedSize += ( portBYTE_ALIGNMENT - ( xWantedSize & portBYTE_ALIGNMENT_MASK ) );
- }
- else
- {
- xWantedSize = 0;
- }
+ xWantedSize += xAdditionalRequiredSize;
}
else
{
- mtCOVERAGE_TEST_MARKER();
+ xWantedSize = 0;
}
}
else
{
- xWantedSize = 0;
+ mtCOVERAGE_TEST_MARKER();
}
/* Check the block size we are trying to allocate is not so large that the
@@ -365,9 +362,8 @@
size_t xSize )
{
void * pv = NULL;
- const size_t xSizeMaxValue = ~( ( size_t ) 0 );
- if( !heapMULTIPLY_WILL_OVERFLOW( xNum, xSize, xSizeMaxValue ) )
+ if( heapMULTIPLY_WILL_OVERFLOW( xNum, xSize ) == 0 )
{
pv = pvPortMalloc( xNum * xSize );
@@ -502,7 +498,7 @@
* inserted at the end of the region space. */
xAddress = xAlignedHeap + xTotalRegionSize;
xAddress -= xHeapStructSize;
- xAddress &= ~portBYTE_ALIGNMENT_MASK;
+ xAddress &= ~( ( size_t ) portBYTE_ALIGNMENT_MASK );
pxEnd = ( BlockLink_t * ) xAddress;
pxEnd->xBlockSize = 0;
pxEnd->pxNextFreeBlock = NULL;
