From 3899e090dfd2e88b4a1929e304df835fd7a3d316 Mon Sep 17 00:00:00 2001 From: Sean McBride Date: Mon, 3 May 2021 11:22:53 -0400 Subject: Ubsan fixes (#498) * Fixed use of null pointer identified by UBSan UBSan warned: runtime error: member access within null pointer of type 'named_dt_t' (aka 'struct named_dt_t') with these two tests: H5REPACK-committed_dt_DFF H5REPACK-committed_dt Reformulated per @gnuoyd suggestion. * Fixed undefined float -> unsigned char conversion in HL_test_image * Removed dead skip_overflow_tests_g global The global `skip_overflow_tests_g` was being set but never read. * Don't treat 2d array as 1d array, fixing UBSan complaint in `CPP_testhdf5` * Committing clang-format changes * Remove extra ']' in line 730. * Committing clang-format changes Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Larry Knox --- c++/test/ttypes.cpp | 8 ++-- hl/test/test_image.c | 11 ++--- test/dt_arith.c | 93 ------------------------------------------- tools/src/h5repack/h5repack.c | 18 ++++----- 4 files changed, 19 insertions(+), 111 deletions(-) diff --git a/c++/test/ttypes.cpp b/c++/test/ttypes.cpp index b633740..7d0337d 100644 --- a/c++/test/ttypes.cpp +++ b/c++/test/ttypes.cpp @@ -656,7 +656,6 @@ static void test_named() { static hsize_t ds_size[2] = {10, 20}; - hsize_t i; unsigned attr_data[10][20]; DataType * ds_type = NULL; @@ -726,8 +725,11 @@ test_named() // It should be possible to define an attribute for the named type Attribute attr1 = itype.createAttribute("attr1", PredType::NATIVE_UCHAR, space); - for (i = 0; i < ds_size[0] * ds_size[1]; i++) - attr_data[0][i] = (int)i; /*tricky*/ + for (hsize_t i = 0; i < ds_size[0]; i++) { + for (hsize_t j = 0; j < ds_size[1]; j++) { + attr_data[i][j] = (unsigned)(i * ds_size[1] + j); + } + } attr1.write(PredType::NATIVE_UINT, attr_data); attr1.close(); diff --git a/hl/test/test_image.c b/hl/test/test_image.c index 0c7d511..dc1be96 100644 --- a/hl/test/test_image.c +++ b/hl/test/test_image.c @@ -650,10 +650,10 @@ test_generate(void) HL_TESTING2("make indexed image from land data"); for (i = 0; i < n_elements; i++) { - if (data[i] < 0) + if (data[i] < 0.0f) image_data[i] = 0; else - image_data[i] = (unsigned char)((255 * (data[i])) / xmax); + image_data[i] = (unsigned char)((255 * data[i]) / xmax); } /* make the image */ @@ -671,10 +671,11 @@ test_generate(void) HL_TESTING2("make indexed image from sea data"); for (i = 0; i < n_elements; i++) { - if (data[i] > 0) + if (data[i] > 0.0f) image_data[i] = 0; - else - image_data[i] = (unsigned char)((255 * (data[i] - xmin)) / xmin); + else { + image_data[i] = (unsigned char)((255.0f * (data[i] - xmin)) / (xmax - xmin)); + } } /* make the image */ diff --git a/test/dt_arith.c b/test/dt_arith.c index f2bf6cf..ddc8814 100644 --- a/test/dt_arith.c +++ b/test/dt_arith.c @@ -74,9 +74,6 @@ typedef enum dtype_t { OTHER } dtype_t; -/* Skip overflow tests if non-zero */ -static int skip_overflow_tests_g = 0; - /* * Although we check whether a floating point overflow generates a SIGFPE and * turn off overflow tests in that case, it might still be possible for an @@ -394,7 +391,6 @@ static int without_hardware_g = 0; HDfree(value); \ } -void some_dummy_func(float x); static hbool_t overflows(unsigned char *origin_bits, hid_t src_id, size_t dst_num_bits); static int my_isnan(dtype_t type, void *val); static int my_isinf(int endian, const unsigned char *val, size_t size, size_t mpos, size_t msize, size_t epos, @@ -515,92 +511,6 @@ except_func(H5T_conv_except_t except_type, hid_t H5_ATTR_UNUSED src_id, hid_t H5 } /*------------------------------------------------------------------------- - * Function: some_dummy_func - * - * Purpose: A dummy function to help check for overflow. - * - * Note: DO NOT DECLARE THIS FUNCTION STATIC OR THE COMPILER MIGHT - * PROMOTE ARGUMENT `x' TO DOUBLE AND DEFEAT THE OVERFLOW - * CHECKING. - * - * Return: void - * - * Programmer: Robb Matzke - * Tuesday, July 21, 1998 - * - *------------------------------------------------------------------------- - */ -void -some_dummy_func(float x) -{ - char s[128]; - - HDsnprintf(s, sizeof(s), "%g", (double)x); -} - -/*------------------------------------------------------------------------- - * Function: generates_sigfpe - * - * Purpose: Determines if SIGFPE is generated from overflows. We must be - * able to fork() and waitpid() in order for this test to work - * properly. Sets skip_overflow_tests_g to non-zero if they - * would generate SIGBUS, zero otherwise. - * - * Programmer: Robb Matzke - * Tuesday, July 21, 1998 - * - * Modifications: - * - *------------------------------------------------------------------------- - */ -static void -generates_sigfpe(void) -{ -#ifdef H5_HAVE_UNISTD_H - pid_t pid; - int status; - size_t i, j; - double d; - unsigned char *dp = (unsigned char *)&d; - float f; - - HDfflush(stdout); - HDfflush(stderr); - if ((pid = HDfork()) < 0) { - HDperror("fork"); - HDexit(EXIT_FAILURE); - } - else if (0 == pid) { - for (i = 0; i < 2000; i++) { - for (j = 0; j < sizeof(double); j++) - dp[j] = (unsigned char)HDrand(); - f = (float)d; - some_dummy_func((float)f); - } - HDexit(EXIT_SUCCESS); - } - - while (pid != HDwaitpid(pid, &status, 0)) - /*void*/; - if (WIFEXITED(status) && 0 == WEXITSTATUS(status)) { - HDputs("Floating-point overflow cases will be tested."); - skip_overflow_tests_g = FALSE; - } - else if (WIFSIGNALED(status) && SIGFPE == WTERMSIG(status)) { - HDputs("Floating-point overflow cases cannot be safely tested."); - skip_overflow_tests_g = TRUE; - /* delete the core dump file that SIGFPE may have created */ - HDunlink("core"); - } -#else /* H5_HAVE_UNISTD_H */ - HDputs("Cannot determine if floating-point overflows generate a SIGFPE"); - HDputs("due to a lack of fork(2) - assuming yes."); - HDputs("Overflow cases will not be tested."); - skip_overflow_tests_g = TRUE; -#endif /* H5_HAVE_UNISTD_H */ -} - -/*------------------------------------------------------------------------- * Function: test_hard_query * * Purpose: Tests H5Tcompiler_conv() for querying whether a conversion is @@ -5406,9 +5316,6 @@ main(void) * for user-defined integer types */ nerrors += (unsigned long)test_derived_integer(); - /* Does floating point overflow generate a SIGFPE? */ - generates_sigfpe(); - /* Test degenerate cases */ nerrors += (unsigned long)run_fp_tests("noop"); diff --git a/tools/src/h5repack/h5repack.c b/tools/src/h5repack/h5repack.c index 7bcca8f..7cad36b 100644 --- a/tools/src/h5repack/h5repack.c +++ b/tools/src/h5repack/h5repack.c @@ -225,9 +225,9 @@ hid_t copy_named_datatype(hid_t type_in, hid_t fidout, named_dt_t **named_dt_head_p, trav_table_t *travt, pack_opt_t *options) { - named_dt_t *dt = *named_dt_head_p; /* Stack pointer */ - named_dt_t *dt_ret = NULL; /* Datatype to return */ - H5O_info2_t oinfo; /* Object info of input dtype */ + named_dt_t *dt = NULL; /* Stack pointer */ + named_dt_t *dt_ret = NULL; /* Datatype to return */ + H5O_info2_t oinfo; /* Object info of input dtype */ int token_cmp; hid_t ret_value = H5I_INVALID_HID; @@ -235,15 +235,13 @@ copy_named_datatype(hid_t type_in, hid_t fidout, named_dt_t **named_dt_head_p, t H5TOOLS_GOTO_ERROR(H5I_INVALID_HID, "H5Oget_info failed"); if (*named_dt_head_p) { - if (H5Otoken_cmp(type_in, &dt->obj_token, &oinfo.token, &token_cmp) < 0) - H5TOOLS_GOTO_ERROR(H5I_INVALID_HID, "failed to compare object tokens"); - - /* Stack already exists, search for the datatype */ - while (dt && token_cmp) { - dt = dt->next; - + /* Search the stack for the datatype. */ + for (dt = *named_dt_head_p; dt != NULL; dt = dt->next) { if (H5Otoken_cmp(type_in, &dt->obj_token, &oinfo.token, &token_cmp) < 0) H5TOOLS_GOTO_ERROR(H5I_INVALID_HID, "failed to compare object tokens"); + + if (token_cmp == 0) + break; // found it! } dt_ret = dt; -- cgit v0.12