diff --git a/release_docs/RELEASE.txt b/release_docs/RELEASE.txt index 6a2e902431a..51ef5c3db97 100644 --- a/release_docs/RELEASE.txt +++ b/release_docs/RELEASE.txt @@ -514,6 +514,20 @@ Bug Fixes since HDF5-2.0.0 release =================================== Library ------- + - Fixed an assertion failure in H5S__hyper_make_spans() + + Calling H5Sselect_hyperslab() on dataspaces with invalid extents could + result in an assertion failure in debug builds of the library if the + dataspace has an extent with a rank value of 0. This has been fixed by + converting the assertion failure into a normal error check. + + - Fixed an assertion failure in H5S__hyper_new_span_info() + + Calling H5Scopy() on hyperslab selection dataspaces with invalid extents + could result in an assertion failure in debug builds of the library if + the dataspace has an extent with a rank value of 0. This has been fixed + by converting the assertion failure into a normal error check. + - Fixed an error in H5Ddebug H5Ddebug would fail for any chunked dataset with a chunk index, due to its diff --git a/src/H5Shyper.c b/src/H5Shyper.c index 5c6aed8f7c4..7d6739a7fbe 100644 --- a/src/H5Shyper.c +++ b/src/H5Shyper.c @@ -2827,9 +2827,11 @@ H5S__hyper_new_span_info(unsigned rank) FUNC_ENTER_PACKAGE /* Sanity check */ - assert(rank > 0); assert(rank <= H5S_MAX_RANK); + if (rank == 0) + HGOTO_ERROR(H5E_DATASPACE, H5E_BADVALUE, NULL, "dataspace has invalid extent"); + /* Allocate a new span info node */ if (NULL == (ret_value = (H5S_hyper_span_info_t *)H5FL_ARR_CALLOC(hbounds_t, rank * 2))) HGOTO_ERROR(H5E_DATASPACE, H5E_CANTALLOC, NULL, "can't allocate hyperslab span info"); @@ -3204,7 +3206,7 @@ H5S__hyper_free_span(H5S_hyper_span_t *span) static herr_t H5S__hyper_copy(H5S_t *dst, const H5S_t *src, bool share_selection) { - H5S_hyper_sel_t *dst_hslab; /* Pointer to destination hyperslab info */ + H5S_hyper_sel_t *dst_hslab = NULL; /* Pointer to destination hyperslab info */ const H5S_hyper_sel_t *src_hslab; /* Pointer to source hyperslab info */ herr_t ret_value = SUCCEED; /* Return value */ @@ -3215,11 +3217,11 @@ H5S__hyper_copy(H5S_t *dst, const H5S_t *src, bool share_selection) assert(dst); /* Allocate space for the hyperslab selection information */ - if (NULL == (dst->select.sel_info.hslab = H5FL_MALLOC(H5S_hyper_sel_t))) + if (NULL == (dst_hslab = H5FL_MALLOC(H5S_hyper_sel_t))) HGOTO_ERROR(H5E_DATASPACE, H5E_CANTALLOC, FAIL, "can't allocate hyperslab info"); + dst_hslab->span_lst = NULL; /* Set temporary pointers */ - dst_hslab = dst->select.sel_info.hslab; src_hslab = src->select.sel_info.hslab; /* Copy the hyperslab information */ @@ -3229,25 +3231,38 @@ H5S__hyper_copy(H5S_t *dst, const H5S_t *src, bool share_selection) /* Check if there is hyperslab span information to copy */ /* (Regular hyperslab information is copied with the selection structure) */ - if (src->select.sel_info.hslab->span_lst != NULL) { + if (src_hslab->span_lst != NULL) { if (share_selection) { /* Share the source's span tree by incrementing the reference count on it */ - dst->select.sel_info.hslab->span_lst = src->select.sel_info.hslab->span_lst; - dst->select.sel_info.hslab->span_lst->count++; + dst_hslab->span_lst = src_hslab->span_lst; + dst_hslab->span_lst->count++; } /* end if */ - else + else { /* Copy the hyperslab span information */ - dst->select.sel_info.hslab->span_lst = - H5S__hyper_copy_span(src->select.sel_info.hslab->span_lst, src->extent.rank); + dst_hslab->span_lst = H5S__hyper_copy_span(src_hslab->span_lst, src->extent.rank); + if (NULL == dst_hslab->span_lst) + HGOTO_ERROR(H5E_DATASPACE, H5E_CANTCOPY, FAIL, "unable to copy hyperslab span information"); + } } /* end if */ else - dst->select.sel_info.hslab->span_lst = NULL; + dst_hslab->span_lst = NULL; /* Copy the unlimited dimension info */ dst_hslab->unlim_dim = src_hslab->unlim_dim; dst_hslab->num_elem_non_unlim = src_hslab->num_elem_non_unlim; + dst->select.sel_info.hslab = dst_hslab; + done: + if (ret_value < 0) { + if (dst_hslab) { + if (dst_hslab->span_lst && H5S__hyper_free_span_info(dst_hslab->span_lst) < 0) + HDONE_ERROR(H5E_DATASPACE, H5E_CANTFREE, FAIL, "unable to free hyperslab span information"); + + H5FL_FREE(H5S_hyper_sel_t, dst_hslab); + } + } + FUNC_LEAVE_NOAPI(ret_value) } /* end H5S__hyper_copy() */ @@ -8603,12 +8618,14 @@ H5S__hyper_make_spans(unsigned rank, const hsize_t *start, const hsize_t *stride FUNC_ENTER_PACKAGE /* Check args */ - assert(rank > 0); assert(start); assert(stride); assert(count); assert(block); + if (rank == 0) + HGOTO_ERROR(H5E_DATASPACE, H5E_BADVALUE, NULL, "dataspace has invalid extent"); + /* Start creating spans in fastest changing dimension */ for (i = (int)(rank - 1); i >= 0; i--) { hsize_t curr_low, curr_high; /* Current low & high values */ diff --git a/src/H5Sselect.c b/src/H5Sselect.c index 73a044b2723..a449041d7d1 100644 --- a/src/H5Sselect.c +++ b/src/H5Sselect.c @@ -218,7 +218,9 @@ H5Sselect_copy(hid_t dst_id, hid_t src_id) herr_t H5S_select_copy(H5S_t *dst, const H5S_t *src, bool share_selection) { - herr_t ret_value = FAIL; /* Return value */ + H5S_t tmp_space; + bool copied_space = false; + herr_t ret_value = FAIL; /* Return value */ FUNC_ENTER_NOAPI(FAIL) @@ -226,18 +228,29 @@ H5S_select_copy(H5S_t *dst, const H5S_t *src, bool share_selection) assert(dst); assert(src); - /* Release the current selection */ - if (H5S_SELECT_RELEASE(dst) < 0) - HGOTO_ERROR(H5E_DATASPACE, H5E_CANTRELEASE, FAIL, "unable to release selection"); + tmp_space = *dst; /* Copy regular fields */ - dst->select = src->select; + tmp_space.select = src->select; /* Perform correct type of copy based on the type of selection */ - if ((ret_value = (*src->select.type->copy)(dst, src, share_selection)) < 0) + if ((ret_value = (*src->select.type->copy)(&tmp_space, src, share_selection)) < 0) HGOTO_ERROR(H5E_DATASPACE, H5E_CANTCOPY, FAIL, "can't copy selection specific information"); + copied_space = true; + + /* Release the current selection */ + if (H5S_SELECT_RELEASE(dst) < 0) + HGOTO_ERROR(H5E_DATASPACE, H5E_CANTRELEASE, FAIL, "unable to release selection"); + + *dst = tmp_space; done: + + if (ret_value < 0) { + if (copied_space && H5S_SELECT_RELEASE(&tmp_space) < 0) + HGOTO_ERROR(H5E_DATASPACE, H5E_CANTRELEASE, FAIL, "unable to release selection"); + } + FUNC_LEAVE_NOAPI(ret_value) } /* end H5S_select_copy() */ diff --git a/test/th5s.c b/test/th5s.c index b8820d488ef..99c84fff937 100644 --- a/test/th5s.c +++ b/test/th5s.c @@ -3393,6 +3393,97 @@ test_h5s_bug3(void) CHECK(ret, FAIL, "H5Sclose"); } /* test_h5s_bug3() */ +/**************************************************************** +** +** test_h5s_bug6(): Test calling H5Sselect_hyperslab() on a +** dataspace with a NULL extent such that an +** assertion failure happens when the library +** attempts to create new span information for +** the dataspace. +** +****************************************************************/ +static void +test_h5s_bug6(void) +{ + hsize_t start[] = {0}; + hsize_t count[] = {1}; + herr_t ret = SUCCEED; + hid_t space_id = H5I_INVALID_HID; + + space_id = H5Screate(H5S_SIMPLE); + CHECK(space_id, H5I_INVALID_HID, "H5Screate"); + + ret = H5Sselect_hyperslab(space_id, H5S_SELECT_SET, start, NULL, count, NULL); + CHECK(ret, FAIL, "H5Sselect_hyperslab"); + + /* OR in another piece */ + start[0] = 3; + + H5E_BEGIN_TRY + { + ret = H5Sselect_hyperslab(space_id, H5S_SELECT_OR, start, NULL, count, NULL); + } + H5E_END_TRY + VERIFY(ret, FAIL, "H5Sselect_hyperslab"); + + ret = H5Sclose(space_id); + CHECK(ret, FAIL, "H5Sclose"); +} /* test_h5s_bug6() */ + +/**************************************************************** +** +** test_h5s_bug7(): Test calling H5Scopy() on a dataspace with +** a NULL extent such that an assertion failure +** happens when the library attempts to create +** new span information for the dataspace. +** +****************************************************************/ +static void +test_h5s_bug7(void) +{ + hsize_t dims[] = {10}; + hsize_t start[] = {0}; + hsize_t count[] = {1}; + herr_t ret = SUCCEED; + hid_t space_id = H5I_INVALID_HID; + hid_t space_copy_id = H5I_INVALID_HID; + + space_id = H5Screate_simple(1, dims, NULL); + CHECK(space_id, H5I_INVALID_HID, "H5Screate_simple"); + + ret = H5Sselect_hyperslab(space_id, H5S_SELECT_SET, start, NULL, count, NULL); + CHECK(ret, FAIL, "H5Sselect_hyperslab"); + + /* OR in pieces to make irregular hyperslab */ + start[0] = 3; + count[0] = 3; + + ret = H5Sselect_hyperslab(space_id, H5S_SELECT_OR, start, NULL, count, NULL); + CHECK(ret, FAIL, "H5Sselect_hyperslab"); + + /* Change dataspace extent to NULL extent */ + ret = H5Sset_extent_none(space_id); + CHECK(ret, FAIL, "H5Sset_extent_none"); + + /* Copy the dataspace - should fail due to invalid dataspace extent */ + H5E_BEGIN_TRY + { + space_copy_id = H5Scopy(space_id); + } + H5E_END_TRY + VERIFY(space_copy_id, H5I_INVALID_HID, "H5Scopy"); + + H5E_BEGIN_TRY + { + ret = H5Sclose(space_copy_id); + } + H5E_END_TRY + VERIFY(ret, FAIL, "H5Sclose"); + + ret = H5Sclose(space_id); + CHECK(ret, FAIL, "H5Sclose"); +} /* test_h5s_bug7() */ + /*------------------------------------------------------------------------- * Function: test_versionbounds * @@ -3577,6 +3668,8 @@ test_h5s(void H5_ATTR_UNUSED *params) test_h5s_bug1(); /* Test bug in offset initialization */ test_h5s_bug2(); /* Test bug found in H5S__hyper_update_diminfo() */ test_h5s_bug3(); /* Test bug found in H5S__combine_select() */ + test_h5s_bug6(); /* Test bug found in H5S__hyper_make_spans() */ + test_h5s_bug7(); /* Test bug found in H5S__hyper_new_span_info() */ test_versionbounds(); /* Test version bounds with dataspace */ } /* test_h5s() */