Skip to content

Commit

Permalink
Be a bit safer with signed arithmetic, thus quieting some signed-over…
Browse files Browse the repository at this point in the history 
…flow warnings from GCC (#1706)

* Avoid a signed overflow: check the range of `entry_ptr->age` before
increasing it instead of increasing it and then checking the range.
This quiets a GCC warning.

* Avoid the potential for signed overflow by rewriting expressions
`MAX(0, fwidth - n)` as `MAX(n, fwidth) - n` for various `n`.
This change quiets some GCC warnings.

* Change some local variables that cannot take sensible negative values
from signed to unsigned.  This quiets GCC warnings about potential
signed overflow.

* In a handful of instances, check the range of a signed integer before
increasing/decreasing it, just in case the increase/decrease overflows.
This quiets a handful of GCC signed-overflow warnings.

* Committing clang-format changes

Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>
  • Loading branch information
@gnuoyd @github-actions
gnuoyd and github-actions[bot] authored Apr 30, 2022
1 parent ffd311c commit c4d70e3
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 22 deletions.
10 changes: 5 additions & 5 deletions src/H5Bdbg.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -110,20 +110,20 @@ H5B_debug(H5F_t *f, haddr_t addr, FILE *stream, int indent, int fwidth, const H5
*/
for (u = 0; u < bt->nchildren; u++) {
HDfprintf(stream, "%*sChild %d...\n", indent, "", u);
HDfprintf(stream, "%*s%-*s %" PRIuHADDR "\n", indent + 3, "", MAX(0, fwidth - 3),
HDfprintf(stream, "%*s%-*s %" PRIuHADDR "\n", indent + 3, "", MAX(3, fwidth) - 3,
"Address:", bt->child[u]);

/* If there is a key debugging routine, use it to display the left & right keys */
if (type->debug_key) {
/* Decode the 'left' key & print it */
HDfprintf(stream, "%*s%-*s\n", indent + 3, "", MAX(0, fwidth - 3), "Left Key:");
HDfprintf(stream, "%*s%-*s\n", indent + 3, "", MAX(3, fwidth) - 3, "Left Key:");
HDassert(H5B_NKEY(bt, shared, u));
(void)(type->debug_key)(stream, indent + 6, MAX(0, fwidth - 6), H5B_NKEY(bt, shared, u), udata);
(void)(type->debug_key)(stream, indent + 6, MAX(6, fwidth) - 6, H5B_NKEY(bt, shared, u), udata);

/* Decode the 'right' key & print it */
HDfprintf(stream, "%*s%-*s\n", indent + 3, "", MAX(0, fwidth - 3), "Right Key:");
HDfprintf(stream, "%*s%-*s\n", indent + 3, "", MAX(3, fwidth) - 3, "Right Key:");
HDassert(H5B_NKEY(bt, shared, u + 1));
(void)(type->debug_key)(stream, indent + 6, MAX(0, fwidth - 6), H5B_NKEY(bt, shared, u + 1),
(void)(type->debug_key)(stream, indent + 6, MAX(6, fwidth) - 6, H5B_NKEY(bt, shared, u + 1),
udata);
} /* end if */
} /* end for */
Expand Down
32 changes: 17 additions & 15 deletions src/H5C.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4667,10 +4667,11 @@ H5C__autoadjust__ageout__cycle_epoch_marker(H5C_t *cache_ptr)
cache_ptr->epoch_marker_ringbuf_first =
(cache_ptr->epoch_marker_ringbuf_first + 1) % (H5C__MAX_EPOCH_MARKERS + 1);

if (cache_ptr->epoch_marker_ringbuf_size <= 0)
HGOTO_ERROR(H5E_CACHE, H5E_SYSTEM, FAIL, "ring buffer underflow")

cache_ptr->epoch_marker_ringbuf_size -= 1;

if (cache_ptr->epoch_marker_ringbuf_size < 0)
HGOTO_ERROR(H5E_CACHE, H5E_SYSTEM, FAIL, "ring buffer underflow")
if ((cache_ptr->epoch_marker_active)[i] != TRUE)
HGOTO_ERROR(H5E_CACHE, H5E_SYSTEM, FAIL, "unused marker in LRU?!?")

Expand All @@ -4690,11 +4691,11 @@ H5C__autoadjust__ageout__cycle_epoch_marker(H5C_t *cache_ptr)

(cache_ptr->epoch_marker_ringbuf)[cache_ptr->epoch_marker_ringbuf_last] = i;

cache_ptr->epoch_marker_ringbuf_size += 1;

if (cache_ptr->epoch_marker_ringbuf_size > H5C__MAX_EPOCH_MARKERS)
if (cache_ptr->epoch_marker_ringbuf_size >= H5C__MAX_EPOCH_MARKERS)
HGOTO_ERROR(H5E_CACHE, H5E_SYSTEM, FAIL, "ring buffer overflow")

cache_ptr->epoch_marker_ringbuf_size += 1;

H5C__DLL_PREPEND((&((cache_ptr->epoch_markers)[i])), (cache_ptr)->LRU_head_ptr, (cache_ptr)->LRU_tail_ptr,
(cache_ptr)->LRU_list_len, (cache_ptr)->LRU_list_size, (FAIL))
done:
Expand Down Expand Up @@ -4965,13 +4966,13 @@ H5C__autoadjust__ageout__insert_new_marker(H5C_t *cache_ptr)

(cache_ptr->epoch_marker_ringbuf)[cache_ptr->epoch_marker_ringbuf_last] = i;

cache_ptr->epoch_marker_ringbuf_size += 1;

if (cache_ptr->epoch_marker_ringbuf_size > H5C__MAX_EPOCH_MARKERS) {
if (cache_ptr->epoch_marker_ringbuf_size >= H5C__MAX_EPOCH_MARKERS) {

HGOTO_ERROR(H5E_CACHE, H5E_SYSTEM, FAIL, "ring buffer overflow")
}

cache_ptr->epoch_marker_ringbuf_size += 1;

H5C__DLL_PREPEND((&((cache_ptr->epoch_markers)[i])), (cache_ptr)->LRU_head_ptr, (cache_ptr)->LRU_tail_ptr,
(cache_ptr)->LRU_list_len, (cache_ptr)->LRU_list_size, (FAIL))

Expand Down Expand Up @@ -5019,11 +5020,11 @@ H5C__autoadjust__ageout__remove_all_markers(H5C_t *cache_ptr)
cache_ptr->epoch_marker_ringbuf_first =
(cache_ptr->epoch_marker_ringbuf_first + 1) % (H5C__MAX_EPOCH_MARKERS + 1);

cache_ptr->epoch_marker_ringbuf_size -= 1;

if (cache_ptr->epoch_marker_ringbuf_size < 0)
if (cache_ptr->epoch_marker_ringbuf_size <= 0)
HGOTO_ERROR(H5E_CACHE, H5E_SYSTEM, FAIL, "ring buffer underflow")

cache_ptr->epoch_marker_ringbuf_size -= 1;

if ((cache_ptr->epoch_marker_active)[i] != TRUE)
HGOTO_ERROR(H5E_CACHE, H5E_SYSTEM, FAIL, "unused marker in LRU?!?")

Expand Down Expand Up @@ -5092,10 +5093,11 @@ H5C__autoadjust__ageout__remove_excess_markers(H5C_t *cache_ptr)
cache_ptr->epoch_marker_ringbuf_first =
(cache_ptr->epoch_marker_ringbuf_first + 1) % (H5C__MAX_EPOCH_MARKERS + 1);

if (cache_ptr->epoch_marker_ringbuf_size <= 0)
HGOTO_ERROR(H5E_CACHE, H5E_SYSTEM, FAIL, "ring buffer underflow")

cache_ptr->epoch_marker_ringbuf_size -= 1;

if (cache_ptr->epoch_marker_ringbuf_size < 0)
HGOTO_ERROR(H5E_CACHE, H5E_SYSTEM, FAIL, "ring buffer underflow")
if ((cache_ptr->epoch_marker_active)[i] != TRUE)
HGOTO_ERROR(H5E_CACHE, H5E_SYSTEM, FAIL, "unused marker in LRU?!?")

Expand Down Expand Up @@ -5523,8 +5525,8 @@ H5C__flush_invalidate_ring(H5F_t *f, H5C_ring_t ring, unsigned flags)
hbool_t restart_slist_scan;
uint32_t protected_entries = 0;
int32_t i;
int32_t cur_ring_pel_len;
int32_t old_ring_pel_len;
uint32_t cur_ring_pel_len;
uint32_t old_ring_pel_len;
unsigned cooked_flags;
unsigned evict_flags;
H5SL_node_t * node_ptr = NULL;
Expand Down
5 changes: 3 additions & 2 deletions src/H5Cimage.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2690,10 +2690,11 @@ H5C__prep_for_file_close__setup_image_entries_array(H5C_t *cache_ptr)
*/
if (entry_ptr->type->id == H5AC_PREFETCHED_ENTRY_ID) {
image_entries[u].type_id = entry_ptr->prefetch_type_id;
image_entries[u].age = entry_ptr->age + 1;

if (image_entries[u].age > H5AC__CACHE_IMAGE__ENTRY_AGEOUT__MAX)
if (entry_ptr->age >= H5AC__CACHE_IMAGE__ENTRY_AGEOUT__MAX)
image_entries[u].age = H5AC__CACHE_IMAGE__ENTRY_AGEOUT__MAX;
else
image_entries[u].age = entry_ptr->age + 1;
} /* end if */
else {
image_entries[u].type_id = entry_ptr->type->id;
Expand Down

0 comments on commit c4d70e3

Please sign in to comment.