Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix a few Clang sanitizer warnings #1727

Merged
merged 1 commit into from
May 5, 2022

Conversation

jhendersonHDF
Copy link
Collaborator

Noticed while doing other testing

b += b_stride;

if (b)
b += b_stride;
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't do arithmetic on b if b is a NULL pointer.

@@ -3278,7 +3279,8 @@ H5T__conv_vlen(hid_t src_id, hid_t dst_id, H5T_cdata_t *cdata, size_t nelmts, si
else {
s = (uint8_t *)buf + (nelmts - safe) * (size_t)s_stride;
d = (uint8_t *)buf + (nelmts - safe) * (size_t)d_stride;
b = (uint8_t *)bkg + (nelmts - safe) * (size_t)b_stride;
if (bkg)
b = (uint8_t *)bkg + (nelmts - safe) * (size_t)b_stride;
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't do arithmetic on bkg if bkg is a NULL pointer.

H5SL_destroy(H5VL_opt_ops_g[subcls], H5VL__term_opt_operation_cb, NULL);
H5VL_opt_ops_g[subcls] = NULL;
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Set dynamic operations skip list array table entries to NULL to avoid a use after free during package termination.

test/dsets.c Outdated
const hsize_t shape[2] = {8, 8};
const hsize_t maxshape[2] = {H5S_UNLIMITED, H5S_UNLIMITED};
const hsize_t chunk[2] = {8, 8};
const int buffer[8][8] = {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test was previously writing to an 8x8 dataset with an 8x1 array, underflowing into the following variables and causing a stack overflow.

test/dsets.c Outdated
@@ -15267,7 +15276,7 @@ test_h5s_plist(void)
/* Attempt to 'OR' block with invalid dimensions into the selection */
H5E_BEGIN_TRY
{
ret = H5Pset_dataset_io_hyperslab_selection(dxpl_id_copy, 2, H5S_SELECT_OR, &start, &stride, &count,
ret = H5Pset_dataset_io_hyperslab_selection(dxpl_id_copy, H5S_MAX_RANK + 1, H5S_SELECT_OR, &start, &stride, &count,
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Specifying 2 for the dimensionality here caused an overflow since start, stride, count, block are single values, not 2-dimensional arrays. Since the test was trying to 'OR' an invalid block, use H5S_MAX_RANK + 1 for the dimensionality, which should fail early.

@derobins derobins merged commit cdf837d into HDFGroup:develop May 5, 2022
jhendersonHDF added a commit to jhendersonHDF/hdf5 that referenced this pull request May 8, 2022
derobins added a commit that referenced this pull request May 8, 2022
* Warnings fixes (#1680)

* Clean stack size warnings in sio_engine (#1687)

* Fixes stack size warnings in tcoords.c (#1688)

* Address some warnings from casting away of const (#1684)

* Fixes stack size warnings in dtransform (#1696)

* Fixes stack size warnings in set_extent test (#1698)

* Be a bit safer with signed arithmetic, thus quieting some signed-overflow 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>

* Fix object size warnings in cache.c test (#1701)

* Fix some const cast and stack/static object size warnings (#1700)

* Fix various warnings

* Move HDfree_const to H5private.h for wider use

* Print output from all ranks in parallel tests on allocation failure

* Move const pointer freeing macro to h5test.h for now

* Fixes a bug where t_cache fails due to a string size being too small (#1720)

* Fixes a bug where t_cache fails due to a string size being too small

Recent warning reductions led to an incorrect string size being passed
to h5_fileaccess, causing the test to silently fail. In addition to
fixing the bug, the test will now fail noisily on setup failures.

* Updates the t_cache test to fail noisily on setup errors

* Fix a few Clang sanitizer warnings (#1727)

* Stop lying about H5S_t const-ness (#1209)

Hyperslabs can be reworked inside several H5S callbacks, making H5S_t
non-const in some places where it is marked const. This change switches
these incorrectly const H5S_t pointer parameters and variables to
non-const where appropriate.

* Fix a few warnings after recent H5S const-related changes (#1225)

* Adjustments for HDF5 1.10

* Hdf5 1 12 Miscellaneous warnings fixes (#1718)

* Fixes const issues in the version 2 B-trees (#1172)

The operations that were changed are fundamentally not const since the
shadow operation can modify the node structure when SWMR is in use.

* Quiets const warning in H5RS code (#1181)

* Avoid calling H5Ropen_object with a misaligned H5R_ref_t: copy the (#1171)

* Avoid calling H5Ropen_object with a misaligned H5R_ref_t: copy the
raw H5R_ref_t bytes to a heap buffer that's known to have the right
alignment.

* Committing clang-format changes

* Use an automatic H5R_ref_t instead of malloc'ing one.  Go ahead and
initialize the H5R_ref_t to all-0s so that arbitrary stack content
doesn't foul things up.  Bail out with an error if `size` exceeds
`sizeof(H5R_ref_t)`.

* Committing clang-format changes

Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>

* Miscellaneous warnings fixes

Co-authored-by: Dana Robinson <43805+derobins@users.noreply.github.com>
Co-authored-by: David Young <dyoung@hdfgroup.org>
Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>

* Fix several warnings (#747)

Co-authored-by: Dana Robinson <43805+derobins@users.noreply.github.com>
Co-authored-by: David Young <dyoung@hdfgroup.org>
Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>
@jhendersonHDF jhendersonHDF deleted the sanitizer_issues branch February 20, 2023 16:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants