Fix more undefined behavior issues#6330
Conversation
jhendersonHDF
added
the
Component - C Library
|
With these changes, the default Ubuntu sanitizer build only has a handful of issues left in dt_arith testing, which will be a bit more involved to fix correctly. |
| \ | ||
| tree_val = \ | ||
| ((RESR).type == H5Z_XFORM_INTEGER ? (double)(RESR).value.int_val : (RESR).value.float_val); \ | ||
| p = (TYPE *)(RESL).value.dat_val; \ |
There was a problem hiding this comment.
Casting p from void * to TYPE * here can create misaligned pointers. Use memcpy() from/to p and intermediate TYPE variables to avoid this.
| } | ||
|
|
||
| static herr_t | ||
| scatter_cb(void **src_buf /*out*/, size_t *src_buf_bytes_used /*out*/, void *_scatter_info) |
There was a problem hiding this comment.
The prototype for an H5Dscatter() callback uses const void **, which is trivial to do correctly here.
| \ | ||
| if (n < SRC_PREC - 2) { \ | ||
| value1 = (TYPE)(value1 << 1); \ | ||
| value1 = (TYPE)((uint64_t)value1 << 1); \ |
There was a problem hiding this comment.
Perform shifts in unsigned types before casting back to avoid UBSan warnings about left shifting negative values.
| -75.0F, -82.0F, -89.0F, -97.0F}, | ||
| {25.0F, 17.0F, 10.0F, 3.0F, -4.0F, -11.0F, -19.0F, -26.0F, -33.0F, -40.0F, -48.0F, -55.0F, -62.0F, -69.0F, | ||
| -76.0F, -84.0F, -91.0F, -98.0F}}; | ||
| -76.0F, -84.0F, -91.0F, -97.0F}}; |
There was a problem hiding this comment.
Since intermediate data transformation operations are casted to the final type, -98.0F ends up just outside the range of char during a transformation operation. Cap the value to -97.0F.
| size[1] = ny; | ||
| src_stride[0] = 0; | ||
| src_stride[1] = sizeof(*src); | ||
| dst_stride[0] = (hsize_t)((1 - nx * ny) * sizeof(*src)); |
There was a problem hiding this comment.
This stride value, being negative, was being converted to a huge unsigned value when casted to hsize_t. As the intention is for the stride to be negative, use hssize_t values and H5VM_stride_copy_s() instead.
|
|
||
| /* Check offset of coordinate */ | ||
| if (a[off] != off) | ||
| if (a[off] != off) { |
There was a problem hiding this comment.
This is just for debugging a current CI failure
| set (ADD_BUILD_OPTIONS "${ADD_BUILD_OPTIONS} -DHDF5_ENABLE_PLUGIN_SUPPORT:BOOL=OFF") | ||
| set (ADD_BUILD_OPTIONS "${ADD_BUILD_OPTIONS} -DLIBAEC_USE_LOCALCONTENT:BOOL=OFF") | ||
| set (ADD_BUILD_OPTIONS "${ADD_BUILD_OPTIONS} -DZLIB_USE_LOCALCONTENT:BOOL=OFF") | ||
| set (ADD_BUILD_OPTIONS "${ADD_BUILD_OPTIONS} -DPLUGIN_USE_LOCALCONTENT:BOOL=OFF") |
There was a problem hiding this comment.
This CMake option isn't used in these builds and gives a configuration warning when set due to that
jhendersonHDF
requested review from
bmribler,
brtnfld,
derobins,
fortnern,
glennsong09,
lrknox,
mattjala,
qkoziol and
vchoi-hdfgroup
as code owners
github-project-automation
bot
moved this from To be triaged
to Done
in HDF5 - TRIAGE & TRACK
6 participants
No description provided.