Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

add DEBUGASSERT_MM_FREE_NODE macro #5968

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

add DEBUGASSERT_MM_FREE_NODE macro #5968

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
756b930
mm/mm_heap: add mm_dump_heap_free_node_list function
ewoodev Aug 3, 2023
a052c05
mm/mm_heap: move dump_node to mm_dump_node
ewoodev Aug 3, 2023
6d39ad7
mm/mm_heap/mm_node.h: add DEBUGASSERT_MM_FREE_NODE macro
ewoodev Aug 3, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions os/include/tinyara/mm/mm.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -691,7 +691,9 @@ void mm_addfreechunk(FAR struct mm_heap_s *heap, FAR struct mm_freenode_s *node)

int mm_size2ndx(size_t size);

void mm_dump_node(struct mm_allocnode_s *node, char *node_type);
void mm_dump_heap_region(uint32_t start, uint32_t end);
void mm_dump_heap_free_node_list(struct mm_heap_s *heap);
int heap_dbg(const char *fmt, ...);
#ifdef CONFIG_DEBUG_MM_HEAPINFO
/* Functions contained in kmm_mallinfo.c . Used to display memory allocation details */
Expand Down
55 changes: 10 additions & 45 deletions os/mm/mm_heap/mm_check_heap_corruption.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,41 +80,6 @@
* Private Functions
****************************************************************************/

static void dump_node(struct mm_allocnode_s *node, char *node_type)
{
#if defined(CONFIG_DEBUG_MM_HEAPINFO) && (CONFIG_TASK_NAME_SIZE > 0)
char myname[CONFIG_TASK_NAME_SIZE + 1];
char is_stack[9] = "'s stack";
#endif

if (!node) {
mfdbg("Unable to dump NULL node\n");
return;
}

mfdbg("%s: addr = 0x%08x, type = %c, size = %u, preceding size = %u\n", node_type, node, IS_ALLOCATED_NODE(node) ? 'A' : 'F', node->size, MM_PREV_NODE_SIZE(node));

#ifdef CONFIG_DEBUG_MM_HEAPINFO
pid_t pid = node->pid;
if (pid < 0) {
/* For stack allocated node, pid is negative value.
* To use the pid, change it to original positive value.
*/
pid = (-1) * pid;
} else {
is_stack[0] = '\0';
}
#if CONFIG_TASK_NAME_SIZE > 0
if (prctl(PR_GET_NAME_BYPID, myname, pid) == OK) {
mfdbg("%s owner pid = %d (%s%s), allocated by code at addr = 0x%08x\n", node_type, node->pid, myname, is_stack, node->alloc_call_addr);
} else {
mfdbg("%s owner pid = %d (Exited Task%s), allocated by code at addr = 0x%08x\n", node_type, node->pid, is_stack, node->alloc_call_addr);
}
#else
mfdbg("%s owner pid = %d%s, allocated by code at addr = 0x%08x\n", node_type, node->pid, is_stack, node->alloc_call_addr);
#endif
#endif
}

/****************************************************************************
* Public Functions
Expand Down Expand Up @@ -166,7 +131,7 @@ int mm_check_heap_corruption(struct mm_heap_s *heap)
mfdbg("ERROR: Heap corruption detected in mm_heapstart node.\n");
mfdbg("ERROR: Check for overflow during backward traversal of heap\n");
mfdbg("=========================================================================================\n");
dump_node(heap->mm_heapstart[region], "HEAP START NODE");
mm_dump_node(heap->mm_heapstart[region], "HEAP START NODE");
iscorrupt_f = true;
start_corrupt = (uint32_t)(heap->mm_heapstart[region]);
mfdbg("=========================================================================================\n");
Expand All @@ -180,8 +145,8 @@ int mm_check_heap_corruption(struct mm_heap_s *heap)
mfdbg("ERROR: Heap corruption detected. Check below nodes for possible corruption.\n");
mfdbg("ERROR: Forward traversal of heap\n");
mfdbg("=========================================================================================\n");
dump_node(prev, "PREV NODE");
dump_node(node, "CORRUPT NODE");
mm_dump_node(prev, "PREV NODE");
mm_dump_node(node, "CORRUPT NODE");
iscorrupt_f = true;
if (prev->size <= MM_LAST_CORRUPT_SIZE) {
start_corrupt = (uint32_t)prev;
Expand All @@ -198,8 +163,8 @@ int mm_check_heap_corruption(struct mm_heap_s *heap)
mfdbg("ERROR: Heap corruption detected. Check below nodes for possible corruption.\n");
mfdbg("ERROR: Forward traversal of heap without reaching heapend\n");
mfdbg("=========================================================================================\n");
dump_node((struct mm_allocnode_s *)((char *)prev - MM_PREV_NODE_SIZE(prev)), "PREV NODE");
dump_node(prev, "CORRUPT NODE");
mm_dump_node((struct mm_allocnode_s *)((char *)prev - MM_PREV_NODE_SIZE(prev)), "PREV NODE");
mm_dump_node(prev, "CORRUPT NODE");
iscorrupt_f = true;
start_corrupt = (uint32_t)prev - (MM_PREV_NODE_SIZE(prev) <= MM_LAST_CORRUPT_SIZE ? MM_PREV_NODE_SIZE(prev) : MM_LAST_CORRUPT_SIZE);
mfdbg("=========================================================================================\n");
Expand All @@ -210,7 +175,7 @@ int mm_check_heap_corruption(struct mm_heap_s *heap)
mfdbg("ERROR: Heap corruption detected in mm_heapend node.\n");
mfdbg("ERROR: Check for overflow from PREV node in forward traversal of heap\n");
mfdbg("=========================================================================================\n");
dump_node(heap->mm_heapend[region], "HEAP END NODE");
mm_dump_node(heap->mm_heapend[region], "HEAP END NODE");
iscorrupt_r = true;
end_corrupt = (uint32_t)(heap->mm_heapend[region]) + SIZEOF_MM_ALLOCNODE;
mfdbg("#########################################################################################\n");
Expand All @@ -223,8 +188,8 @@ int mm_check_heap_corruption(struct mm_heap_s *heap)
((MM_PREV_NODE_SIZE(node) < SIZEOF_MM_ALLOCNODE) &&(node != heap->mm_heapstart[region]))) {
mfdbg("ERROR: Backward traversal of heap\n");
mfdbg("=========================================================================================\n");
dump_node(node, "CORRUPT NODE");
dump_node(prev, "PREV NODE");
mm_dump_node(node, "CORRUPT NODE");
mm_dump_node(prev, "PREV NODE");
iscorrupt_r = true;
end_corrupt = (uint32_t)prev + (prev->size <= MM_LAST_CORRUPT_SIZE ? prev->size : MM_LAST_CORRUPT_SIZE);
mfdbg("#########################################################################################\n");
Expand All @@ -235,8 +200,8 @@ int mm_check_heap_corruption(struct mm_heap_s *heap)
if (!iscorrupt_r && prev != heap->mm_heapstart[region]) {
mfdbg("ERROR: Backward traversal of heap without reaching heapstart\n");
mfdbg("=========================================================================================\n");
dump_node(prev, "CORRUPT NODE");
dump_node((struct mm_allocnode_s *)((char *)prev + prev->size), "PREV NODE");
mm_dump_node(prev, "CORRUPT NODE");
mm_dump_node((struct mm_allocnode_s *)((char *)prev + prev->size), "PREV NODE");
iscorrupt_r = true;
node = (struct mm_allocnode_s *)((char *)prev + prev->size);
end_corrupt = (uint32_t)node + (node->size <= MM_LAST_CORRUPT_SIZE ? node->size : MM_LAST_CORRUPT_SIZE);
Expand Down
4 changes: 2 additions & 2 deletions os/mm/mm_heap/mm_free.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ void mm_free(FAR struct mm_heap_s *heap, FAR void *mem)
/* Remove the next node. There must be a predecessor,
* but there may not be a successor node.
*/

DEBUGASSERT_MM_FREE_NODE(heap, next);
REMOVE_NODE_FROM_LIST(next);

/* Then merge the two chunks */
Expand All @@ -173,7 +173,7 @@ void mm_free(FAR struct mm_heap_s *heap, FAR void *mem)
/* Remove the node. There must be a predecessor, but there may
* not be a successor node.
*/

DEBUGASSERT_MM_FREE_NODE(heap, prev);
REMOVE_NODE_FROM_LIST(prev);

/* Then merge the two chunks */
Expand Down
65 changes: 65 additions & 0 deletions os/mm/mm_heap/mm_heap_dbg.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,3 +74,68 @@ void mm_dump_heap_region(uint32_t start, uint32_t end)
heap_dbg("#########################################################################################\n");
}

/************************************************************************
* Name: mm_dump_heap_free_node_list
*
* Description: Print the hex value contents of heap free nodes.
************************************************************************/
void mm_dump_heap_free_node_list(struct mm_heap_s *heap)
{
struct mm_freenode_s *node;
heap_dbg("#########################################################################################\n");
heap_dbg("Dump heap free node list\n");
heap_dbg("[ndx], [HEAD]: [FREE NODES(SIZE)]\n");
heap_dbg("#########################################################################################\n");
for (uint8_t ndx = 0; ndx < MM_NNODES; ndx++) {
heap_dbg("%3d, %08x:", ndx, &heap->mm_nodelist[ndx]);
for (node = heap->mm_nodelist[ndx].flink; node; node = node->flink) {
heap_dbg(" %08x(%d)", node, node->size);
}
heap_dbg("\n");
if (heap->mm_nodelist[ndx].size != 0) {
mfdbg(" Corrupted HEAD of free node link %08x\n", &heap->mm_nodelist[ndx]);
}
}
heap_dbg("#########################################################################################\n");
}

/************************************************************************
* Name: mm_dump_node
*
* Description: Print the information of heap node.
************************************************************************/
void mm_dump_node(struct mm_allocnode_s *node, char *node_type)
{
#if defined(CONFIG_DEBUG_MM_HEAPINFO) && (CONFIG_TASK_NAME_SIZE > 0)
char myname[CONFIG_TASK_NAME_SIZE + 1];
char is_stack[9] = "'s stack";
#endif

if (!node) {
mfdbg("Unable to dump NULL node\n");
return;
}

mfdbg("%s: addr = 0x%08x, type = %c, size = %u, preceding size = %u\n", node_type, node, node->preceding & MM_ALLOC_BIT ? 'A' : 'F', node->size, node->preceding & ~MM_ALLOC_BIT);

#ifdef CONFIG_DEBUG_MM_HEAPINFO
pid_t pid = node->pid;
if (pid < 0) {
/* For stack allocated node, pid is negative value.
* To use the pid, change it to original positive value.
*/
pid = (-1) * pid;
} else {
is_stack[0] = '\0';
}
#if CONFIG_TASK_NAME_SIZE > 0
if (prctl(PR_GET_NAME_BYPID, myname, pid) == OK) {
mfdbg("%s owner pid = %d (%s%s), allocated by code at addr = 0x%08x\n", node_type, node->pid, myname, is_stack, node->alloc_call_addr);
} else {
mfdbg("%s owner pid = %d (Exited Task%s), allocated by code at addr = 0x%08x\n", node_type, node->pid, is_stack, node->alloc_call_addr);
}
#else
mfdbg("%s owner pid = %d%s, allocated by code at addr = 0x%08x\n", node_type, node->pid, is_stack, node->alloc_call_addr);
#endif
#endif
}
2 changes: 1 addition & 1 deletion os/mm/mm_heap/mm_malloc.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ FAR void *mm_malloc(FAR struct mm_heap_s *heap, size_t size)
/* Remove the node. There must be a predecessor, but there may not be
* a successor node.
*/

DEBUGASSERT_MM_FREE_NODE(heap, node);
REMOVE_NODE_FROM_LIST(node);

/* Check if we have to split the free node into one of the allocated
Expand Down
2 changes: 1 addition & 1 deletion os/mm/mm_heap/mm_memalign.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@ FAR void *mm_memalign(FAR struct mm_heap_s *heap, size_t alignment, size_t size)
/* Remove the node. There must be a predecessor, but there may not be
* a successor node.
*/

DEBUGASSERT_MM_FREE_NODE(heap, node);
REMOVE_NODE_FROM_LIST(node);

/* Check if there is free space at the beginning of the aligned chunk */
Expand Down
36 changes: 35 additions & 1 deletion os/mm/mm_heap/mm_node.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,14 +24,48 @@
****************************************************************************/

#include <assert.h>
#include <debug.h>

#include <tinyara/mm/mm.h>

/****************************************************************************
* Pre-processor Definitions
****************************************************************************/

#if defined(CONFIG_BUILD_FLAT) || defined(__KERNEL__)
#define DEBUGASSERT_MM_FREE_NODE(heap, node) \
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you have any reasons to separate DEBUGASSERT_MM_NODE according to build type? I think both are same except setting of abort_mode. If it is not necessary as I said in below comment, we don't need to separate them.

do { \
DEBUGASSERT(node); \
if (!((node)->blink)) { \
abort_mode = true; \
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DEBUGPANIC() is a macro for up_assert function and up_assert sets abort_mode to true internally. Is it necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

#if defined(CONFIG_BUILD_FLAT) || defined(__KERNEL__)
#define mfdbg(format, ...)						\
	do {								\
		if (!IS_SECURE_STATE()) {				\
			if (abort_mode || up_interrupt_context()) {	\
				lldbg(format, ##__VA_ARGS__);		\
			} else {					\
				dbg(format, ##__VA_ARGS__);		\
			}						\
		}							\
	} while (0)
#else
#define mfdbg(format, ...) dbg(format, ##__VA_ARGS__)
#endif

TizenRT/os/include/debug.h

Line 504 in 4cced9a

#define mfdbg(format, ...) \

mfdbg operation has dependency with abort_mode.

if after print mfdbg( using dbg), up_assert is called, dbg can't finsh printing.
becase uart driver and lib stdout buffer is stop and print only using lldbg.

So, To complete print just before up_assert, abort mode setting is needed.

Copy link
Contributor Author

@ewoodev ewoodev Nov 14, 2023 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In fact.., This issue is not solved by setting abort_mode early about loadable user heap.. .
So, I'm checking to add lib stdout flush at up_assert.c

mfdbg("Corrupted free node %08x\n", node); \
mm_dump_heap_free_node_list(heap); \
mm_dump_node((struct mm_allocnode_s *)node, "CORRUPT NODE"); \
mm_dump_node((struct mm_allocnode_s *)((char *)node - (node->preceding & ~MM_ALLOC_BIT)), "PREV NODE"); \
mm_dump_node((struct mm_allocnode_s *)((char *)node + node->size), "NEXT NODE"); \
DEBUGPANIC(); \
} \
} while (0)

#else
#define DEBUGASSERT_MM_FREE_NODE(heap, node) \
do { \
DEBUGASSERT(node); \
if (!((node)->blink)) { \
mfdbg("Corrupted free node %08x\n", node); \
mm_dump_heap_free_node_list(heap); \
mm_dump_node((struct mm_allocnode_s *)node, "CORRUPT NODE"); \
mm_dump_node((struct mm_allocnode_s *)((char *)node - (node->preceding & ~MM_ALLOC_BIT)), "PREV NODE"); \
mm_dump_node((struct mm_allocnode_s *)((char *)node + node->size), "NEXT NODE"); \
DEBUGPANIC(); \
} \
} while (0)

#endif


#define REMOVE_NODE_FROM_LIST(node) \
do { \
DEBUGASSERT((node)->blink); \
(node)->blink->flink = (node)->flink; \
if ((node)->flink) { \
(node)->flink->blink = (node)->blink; \
Expand Down
4 changes: 2 additions & 2 deletions os/mm/mm_heap/mm_realloc.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,7 @@ FAR void *mm_realloc(FAR struct mm_heap_s *heap, FAR void *oldmem, size_t size)
/* Remove the previous node. There must be a predecessor, but
* there may not be a successor node.
*/

DEBUGASSERT_MM_FREE_NODE(heap, prev);
REMOVE_NODE_FROM_LIST(prev);

/* Extend the node into the previous free chunk */
Expand Down Expand Up @@ -320,7 +320,7 @@ FAR void *mm_realloc(FAR struct mm_heap_s *heap, FAR void *oldmem, size_t size)
/* Remove the next node. There must be a predecessor, but there
* may not be a successor node.
*/

DEBUGASSERT_MM_FREE_NODE(heap, next);
REMOVE_NODE_FROM_LIST(next);

/* Extend the node into the next chunk */
Expand Down
2 changes: 1 addition & 1 deletion os/mm/mm_heap/mm_shrinkchunk.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ void mm_shrinkchunk(FAR struct mm_heap_s *heap, FAR struct mm_allocnode_s *node,
/* Remove the next node. There must be a predecessor, but there may
* not be a successor node.
*/

DEBUGASSERT_MM_FREE_NODE(heap, next);
REMOVE_NODE_FROM_LIST(next);

/* Create a new chunk that will hold both the next chunk and the
Expand Down