From c4d70e3571262df0fcf381f5804b671dc9f86bc1 Mon Sep 17 00:00:00 2001 From: David Young Date: Fri, 29 Apr 2022 22:29:20 -0500 Subject: Be a bit safer with signed arithmetic, thus quieting some signed-overflow warnings from GCC (#1706) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 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> --- src/H5Bdbg.c | 10 +++++----- src/H5C.c | 32 +++++++++++++++++--------------- src/H5Cimage.c | 5 +++-- 3 files changed, 25 insertions(+), 22 deletions(-) diff --git a/src/H5Bdbg.c b/src/H5Bdbg.c index 23d0a8f..de07442 100644 --- a/src/H5Bdbg.c +++ b/src/H5Bdbg.c @@ -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 */ diff --git a/src/H5C.c b/src/H5C.c index 77f3069..067acfa 100644 --- a/src/H5C.c +++ b/src/H5C.c @@ -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?!?") @@ -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: @@ -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)) @@ -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?!?") @@ -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?!?") @@ -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; diff --git a/src/H5Cimage.c b/src/H5Cimage.c index ddf5f94..899d6d7 100644 --- a/src/H5Cimage.c +++ b/src/H5Cimage.c @@ -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; -- cgit v0.12