Skip to content

Commit

Permalink
Ubsan fixes (#498)
Browse files Browse the repository at this point in the history 
* 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 <lrknox@hdfgroup.org>
  • Loading branch information
@seanm @github-actions @lrknox
3 people authored May 3, 2021
1 parent 44db310 commit 3899e09
Show file tree
Hide file tree
Showing 4 changed files with 19 additions and 111 deletions.
8 changes: 5 additions & 3 deletions c++/test/ttypes.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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();

Expand Down
11 changes: 6 additions & 5 deletions hl/test/test_image.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand All @@ -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 */
Expand Down
93 changes: 0 additions & 93 deletions test/dt_arith.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -514,92 +510,6 @@ except_func(H5T_conv_except_t except_type, hid_t H5_ATTR_UNUSED src_id, hid_t H5
return ret;
}

/*-------------------------------------------------------------------------
* 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
*
Expand Down Expand Up @@ -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");

Expand Down
18 changes: 8 additions & 10 deletions tools/src/h5repack/h5repack.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -225,25 +225,23 @@ 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;

if (H5Oget_info3(type_in, &oinfo, H5O_INFO_BASIC) < 0)
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;
Expand Down

0 comments on commit 3899e09

Please sign in to comment.