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

Fix free list tracking and cleanup cast alignment warnings #1288

Merged
merged 2 commits into from
Dec 13, 2021
Merged

Fix free list tracking and cleanup cast alignment warnings #1288

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0e99739
Fix free list tracking and cleanup cast alignment warnings
jhendersonHDF Dec 13, 2021
e42d620
Add free list tracking code to H5FL 'arr' routines
jhendersonHDF Dec 13, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
147 changes: 120 additions & 27 deletions src/H5FL.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1001,27 +1001,32 @@ H5FL_blk_free(H5FL_blk_head_t *head, void *block)

#ifdef H5FL_TRACK
{
H5FL_track_t *trk = block = ((unsigned char *)block) - sizeof(H5FL_track_t);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This previous assignment to 'block' was corrupting memory because later code subtracted the size of the H5FL tracking info after it had already been subtracted here.

unsigned char *block_ptr = ((unsigned char *)block) - sizeof(H5FL_track_t);
H5FL_track_t trk;

HDmemcpy(&trk, block_ptr, sizeof(H5FL_track_t));
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Copy data in and out of a local H5FL_track_t struct instance so we don't have potential alignment issues when casting from unsigned char * to H5FL_track_t *


/* Free tracking information about the allocation location */
H5CS_close_stack(trk->stack);
H5CS_close_stack(trk.stack);
/* The 'func' & 'file' strings are statically allocated (by the compiler)
* and are not allocated, so there's no need to free them.
*/
trk->file = NULL;
trk->func = NULL;
trk.file = NULL;
trk.func = NULL;

/* Remove from "outstanding allocations" list */
if (trk == H5FL_out_head_g) {
if ((void *)block_ptr == (void *)H5FL_out_head_g) {
H5FL_out_head_g = H5FL_out_head_g->next;
if (H5FL_out_head_g)
H5FL_out_head_g->prev = NULL;
} /* end if */
else {
trk->prev->next = trk->next;
if (trk->next)
trk->next->prev = trk->prev;
trk.prev->next = trk.next;
if (trk.next)
trk.next->prev = trk.prev;
} /* end else */

HDmemcpy(block_ptr, &trk, sizeof(H5FL_track_t));
}
#endif /* H5FL_TRACK */

Expand Down Expand Up @@ -1120,25 +1125,30 @@ H5FL_blk_realloc(H5FL_blk_head_t *head, void *block, size_t new_size H5FL_TRACK_
else {
#ifdef H5FL_TRACK
{
H5FL_track_t *trk = (H5FL_track_t *)(((unsigned char *)block) - sizeof(H5FL_track_t));
unsigned char *block_ptr = ((unsigned char *)block) - sizeof(H5FL_track_t);
H5FL_track_t trk;

HDmemcpy(&trk, block_ptr, sizeof(H5FL_track_t));

/* Release previous tracking information */
H5CS_close_stack(trk->stack);
H5CS_close_stack(trk.stack);
/* The 'func' & 'file' strings are statically allocated (by the compiler)
* and are not allocated, so there's no need to free them.
*/
trk->file = NULL;
trk->func = NULL;
trk.file = NULL;
trk.func = NULL;

/* Store new tracking information */
trk->stack = H5CS_copy_stack();
HDassert(trk->stack);
trk.stack = H5CS_copy_stack();
HDassert(trk.stack);
/* The 'call_func' & 'call_file' strings are statically allocated (by the compiler)
* there's no need to duplicate them.
*/
trk->file = call_file;
trk->func = call_func;
trk->line = call_line;
trk.file = call_file;
trk.func = call_func;
trk.line = call_line;

HDmemcpy(block_ptr, &trk, sizeof(H5FL_track_t));
}
#endif /* H5FL_TRACK */
ret_value = block;
Expand Down Expand Up @@ -1426,10 +1436,42 @@ H5FL_arr_free(H5FL_arr_head_t *head, void *obj)
/* Make certain that the free list is initialized */
HDassert(head->init);

#ifdef H5FL_TRACK
{
unsigned char *block_ptr = ((unsigned char *)obj) - sizeof(H5FL_track_t);
H5FL_track_t trk;

HDmemcpy(&trk, block_ptr, sizeof(H5FL_track_t));

/* Free tracking information about the allocation location */
H5CS_close_stack(trk.stack);
/* The 'func' & 'file' strings are statically allocated (by the compiler)
* and are not allocated, so there's no need to free them.
*/
trk.file = NULL;
trk.func = NULL;

/* Remove from "outstanding allocations" list */
if ((void *)block_ptr == H5FL_out_head_g) {
H5FL_out_head_g = H5FL_out_head_g->next;
if (H5FL_out_head_g)
H5FL_out_head_g->prev = NULL;
} /* end if */
else {
trk.prev->next = trk.next;
if (trk.next)
trk.next->prev = trk.prev;
} /* end else */

HDmemcpy(block_ptr, &trk, sizeof(H5FL_track_t));
}
#endif

/* Get the pointer to the info header in front of the block to free */
temp = (H5FL_arr_list_t *)((
void *)((unsigned char *)obj -
sizeof(H5FL_arr_list_t))); /*lint !e826 Pointer-to-pointer cast is appropriate here */
(sizeof(H5FL_arr_list_t) +
H5FL_TRACK_SIZE))); /*lint !e826 Pointer-to-pointer cast is appropriate here */

/* Get the number of elements */
free_nelem = temp->nelem;
Expand Down Expand Up @@ -1482,7 +1524,7 @@ H5FL_arr_free(H5FL_arr_head_t *head, void *obj)
*-------------------------------------------------------------------------
*/
void *
H5FL_arr_malloc(H5FL_arr_head_t *head, size_t elem)
H5FL_arr_malloc(H5FL_arr_head_t *head, size_t elem H5FL_TRACK_PARAMS)
{
H5FL_arr_list_t *new_obj; /* Pointer to the new free list node allocated */
size_t mem_size; /* Size of memory block being recycled */
Expand Down Expand Up @@ -1523,7 +1565,8 @@ H5FL_arr_malloc(H5FL_arr_head_t *head, size_t elem)
} /* end if */
/* Otherwise allocate a node */
else {
if (NULL == (new_obj = (H5FL_arr_list_t *)H5FL__malloc(sizeof(H5FL_arr_list_t) + mem_size)))
if (NULL ==
(new_obj = (H5FL_arr_list_t *)H5FL__malloc(sizeof(H5FL_arr_list_t) + H5FL_TRACK_SIZE + mem_size)))
HGOTO_ERROR(H5E_RESOURCE, H5E_NOSPACE, NULL, "memory allocation failed")

/* Increment the number of blocks of this size */
Expand All @@ -1539,6 +1582,28 @@ H5FL_arr_malloc(H5FL_arr_head_t *head, size_t elem)
/* Get a pointer to the new block */
ret_value = ((char *)new_obj) + sizeof(H5FL_arr_list_t);

#ifdef H5FL_TRACK
/* Copy allocation location information */
((H5FL_track_t *)ret_value)->stack = H5CS_copy_stack();
HDassert(((H5FL_track_t *)ret_value)->stack);
/* The 'call_func' & 'call_file' strings are statically allocated (by the compiler)
* there's no need to duplicate them.
*/
((H5FL_track_t *)ret_value)->file = call_file;
((H5FL_track_t *)ret_value)->func = call_func;
((H5FL_track_t *)ret_value)->line = call_line;

/* Add to "outstanding allocations" list */
((H5FL_track_t *)ret_value)->prev = NULL;
((H5FL_track_t *)ret_value)->next = H5FL_out_head_g;
if (H5FL_out_head_g)
H5FL_out_head_g->prev = (H5FL_track_t *)ret_value;
H5FL_out_head_g = (H5FL_track_t *)ret_value;

/* Adjust for allocation tracking information */
ret_value = ((unsigned char *)ret_value) + sizeof(H5FL_track_t);
#endif

done:
FUNC_LEAVE_NOAPI(ret_value)
} /* end H5FL_arr_malloc() */
Expand All @@ -1557,7 +1622,7 @@ H5FL_arr_malloc(H5FL_arr_head_t *head, size_t elem)
*-------------------------------------------------------------------------
*/
void *
H5FL_arr_calloc(H5FL_arr_head_t *head, size_t elem)
H5FL_arr_calloc(H5FL_arr_head_t *head, size_t elem H5FL_TRACK_PARAMS)
{
void *ret_value = NULL; /* Pointer to the block to return */

Expand All @@ -1568,7 +1633,7 @@ H5FL_arr_calloc(H5FL_arr_head_t *head, size_t elem)
HDassert(elem);

/* Allocate the array */
if (NULL == (ret_value = H5FL_arr_malloc(head, elem)))
if (NULL == (ret_value = H5FL_arr_malloc(head, elem H5FL_TRACK_INFO_INT)))
HGOTO_ERROR(H5E_RESOURCE, H5E_NOSPACE, NULL, "memory allocation failed")

/* Clear to zeros */
Expand All @@ -1592,7 +1657,7 @@ H5FL_arr_calloc(H5FL_arr_head_t *head, size_t elem)
*-------------------------------------------------------------------------
*/
void *
H5FL_arr_realloc(H5FL_arr_head_t *head, void *obj, size_t new_elem)
H5FL_arr_realloc(H5FL_arr_head_t *head, void *obj, size_t new_elem H5FL_TRACK_PARAMS)
{
void *ret_value = NULL; /* Pointer to the block to return */

Expand All @@ -1604,7 +1669,7 @@ H5FL_arr_realloc(H5FL_arr_head_t *head, void *obj, size_t new_elem)

/* Check if we are really allocating the object */
if (obj == NULL)
ret_value = H5FL_arr_malloc(head, new_elem);
ret_value = H5FL_arr_malloc(head, new_elem H5FL_TRACK_INFO_INT);
else {
H5FL_arr_list_t *temp; /* Temp. ptr to the new free list node allocated */

Expand All @@ -1614,14 +1679,15 @@ H5FL_arr_realloc(H5FL_arr_head_t *head, void *obj, size_t new_elem)
/* Get the pointer to the info header in front of the block to free */
temp = (H5FL_arr_list_t *)((
void *)((unsigned char *)obj -
sizeof(H5FL_arr_list_t))); /*lint !e826 Pointer-to-pointer cast is appropriate here */
(sizeof(H5FL_arr_list_t) +
H5FL_TRACK_SIZE))); /*lint !e826 Pointer-to-pointer cast is appropriate here */

/* Check if the size is really changing */
if (temp->nelem != new_elem) {
size_t blk_size; /* Size of block */

/* Get the new array of objects */
ret_value = H5FL_arr_malloc(head, new_elem);
ret_value = H5FL_arr_malloc(head, new_elem H5FL_TRACK_INFO_INT);

/* Copy the appropriate amount of elements */
blk_size = head->list_arr[MIN(temp->nelem, new_elem)].size;
Expand All @@ -1630,8 +1696,35 @@ H5FL_arr_realloc(H5FL_arr_head_t *head, void *obj, size_t new_elem)
/* Free the old block */
H5FL_arr_free(head, obj);
} /* end if */
else
else {
#ifdef H5FL_TRACK
unsigned char *block_ptr = ((unsigned char *)obj) - sizeof(H5FL_track_t);
H5FL_track_t trk;

HDmemcpy(&trk, block_ptr, sizeof(H5FL_track_t));

/* Release previous tracking information */
H5CS_close_stack(trk.stack);
/* The 'func' & 'file' strings are statically allocated (by the compiler)
* and are not allocated, so there's no need to free them.
*/
trk.file = NULL;
trk.func = NULL;

/* Store new tracking information */
trk.stack = H5CS_copy_stack();
HDassert(trk.stack);
/* The 'call_func' & 'call_file' strings are statically allocated (by the compiler)
* there's no need to duplicate them.
*/
trk.file = call_file;
trk.func = call_func;
trk.line = call_line;

HDmemcpy(block_ptr, &trk, sizeof(H5FL_track_t));
#endif
ret_value = obj;
}
} /* end else */

FUNC_LEAVE_NOAPI(ret_value)
Expand Down