From a80897ee4944ff6008bfb3b93619ebcb58a070d1 Mon Sep 17 00:00:00 2001 From: David Young Date: Sat, 16 Apr 2022 10:21:18 -0500 Subject: Remove H5_NO_ALIGNMENT_RESTRICTIONS (#1426) * Do not conditionally compile code that uses a pointer dereference and assignment to copy a potentially unaligned variable to aligned automatic storage, or vice versa. Instead, always use naked `memcpy(3)`s. Disassembling the generated code reveals that the `memcpy(3)`s optimize (`-O3`) to a single `mov` instruction for x86_64, which is not strict about alignment. This change reduces the size of code and scripts by 143 lines, eases our way to cross-compilation, and avoids invoking undefined behavior. * Committing clang-format changes * Per discussion, use HD and add comments. Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com> --- config/cmake/ConfigureChecks.cmake | 4 -- config/cmake/ConversionTests.c | 53 --------------------- config/cmake/H5pubconf.h.in | 3 -- configure.ac | 19 -------- src/H5Tvlen.c | 94 ++++++++------------------------------ 5 files changed, 20 insertions(+), 153 deletions(-) diff --git a/config/cmake/ConfigureChecks.cmake b/config/cmake/ConfigureChecks.cmake index 43aa243..9c91d34 100644 --- a/config/cmake/ConfigureChecks.cmake +++ b/config/cmake/ConfigureChecks.cmake @@ -416,7 +416,3 @@ H5ConversionTests (${HDF_PREFIX}_LLONG_TO_LDOUBLE_CORRECT "Checking IF correctly # some long double values #----------------------------------------------------------------------------- H5ConversionTests (${HDF_PREFIX}_DISABLE_SOME_LDOUBLE_CONV "Checking IF the cpu is power9 and cannot correctly converting long double values") -# ---------------------------------------------------------------------- -# Check if pointer alignments are enforced -#----------------------------------------------------------------------------- -H5ConversionTests (${HDF_PREFIX}_NO_ALIGNMENT_RESTRICTIONS "Checking IF alignment restrictions are strictly enforced") diff --git a/config/cmake/ConversionTests.c b/config/cmake/ConversionTests.c index f80959f..f100dd8 100644 --- a/config/cmake/ConversionTests.c +++ b/config/cmake/ConversionTests.c @@ -234,59 +234,6 @@ done: } #endif -#ifdef H5_NO_ALIGNMENT_RESTRICTIONS_TEST - -#include -#include - -typedef struct { - size_t len; - void *p; -} hvl_t; - -#ifdef FC_DUMMY_MAIN -#ifndef FC_DUMMY_MAIN_EQ_F77 -# ifdef __cplusplus -extern "C" -# endif -int FC_DUMMY_MAIN() -{ return 1;} -#endif -#endif -int HDF_NO_UBSAN -main () -{ - - char *chp = "beefs"; - char **chpp = malloc (2 * sizeof (char *)); - char **chpp2; - hvl_t vl = { 12345, (void *) chp }; - hvl_t *vlp; - hvl_t *vlp2; - - memcpy ((void *) ((char *) chpp + 1), &chp, sizeof (char *)); - chpp2 = (char **) ((char *) chpp + 1); - if (strcmp (*chpp2, chp)) { - free (chpp); - return 1; - } - free (chpp); - - vlp = malloc (2 * sizeof (hvl_t)); - memcpy ((void *) ((char *) vlp + 1), &vl, sizeof (hvl_t)); - vlp2 = (hvl_t *) ((char *) vlp + 1); - if (vlp2->len != vl.len || vlp2->p != vl.p) { - free (vlp); - return 1; - } - free (vlp); - - ; - return 0; -} - -#endif - #ifdef H5_DISABLE_SOME_LDOUBLE_CONV_TEST #include diff --git a/config/cmake/H5pubconf.h.in b/config/cmake/H5pubconf.h.in index 4956c97..9a581cb 100644 --- a/config/cmake/H5pubconf.h.in +++ b/config/cmake/H5pubconf.h.in @@ -434,9 +434,6 @@ /* Define to enable internal memory allocation sanity checking. */ #cmakedefine H5_MEMORY_ALLOC_SANITY_CHECK @H5_MEMORY_ALLOC_SANITY_CHECK@ -/* Define if we can violate pointer alignment restrictions */ -#cmakedefine H5_NO_ALIGNMENT_RESTRICTIONS @H5_NO_ALIGNMENT_RESTRICTIONS@ - /* Define if deprecated public API symbols are disabled */ #cmakedefine H5_NO_DEPRECATED_SYMBOLS @H5_NO_DEPRECATED_SYMBOLS@ diff --git a/configure.ac b/configure.ac index d5cef35..908ece7 100644 --- a/configure.ac +++ b/configure.ac @@ -3990,25 +3990,6 @@ AC_ARG_ENABLE([embedded-libinfo], ## ---------------------------------------------------------------------- -## Check if pointer alignments are enforced -## -AC_MSG_CHECKING([if alignment restrictions are strictly enforced]) - -TEST_SRC="`(echo \"#define H5_NO_ALIGNMENT_RESTRICTIONS_TEST 1\"; cat $srcdir/config/cmake/ConversionTests.c)`" - -AC_RUN_IFELSE([ - AC_LANG_SOURCE([$TEST_SRC]) - ], [ - AC_DEFINE([NO_ALIGNMENT_RESTRICTIONS], [1], [Define if we can violate pointer alignment restrictions]) - AC_MSG_RESULT([no]) - ], [ - AC_MSG_RESULT([yes]) - ], [ - AC_MSG_RESULT([unknown, assuming yes]) - ]) - - -## ---------------------------------------------------------------------- ## Restore user's CFLAGS. CFLAGS="$saved_user_CFLAGS" FCFLAGS="$saved_user_FCFLAGS" diff --git a/src/H5Tvlen.c b/src/H5Tvlen.c index 7bf28af..39d33d4 100644 --- a/src/H5Tvlen.c +++ b/src/H5Tvlen.c @@ -369,11 +369,7 @@ done: static herr_t H5T__vlen_mem_seq_getlen(H5VL_object_t H5_ATTR_UNUSED *file, const void *_vl, size_t *len) { -#ifdef H5_NO_ALIGNMENT_RESTRICTIONS - const hvl_t *vl = (const hvl_t *)_vl; /* Pointer to the user's hvl_t information */ -#else hvl_t vl; /* User's hvl_t information */ -#endif FUNC_ENTER_PACKAGE_NOERR @@ -381,13 +377,12 @@ H5T__vlen_mem_seq_getlen(H5VL_object_t H5_ATTR_UNUSED *file, const void *_vl, si HDassert(_vl); HDassert(len); -#ifdef H5_NO_ALIGNMENT_RESTRICTIONS - *len = vl->len; -#else - H5MM_memcpy(&vl, _vl, sizeof(hvl_t)); + /* Copy to ensure correct alignment. memcpy is best here because + * it optimizes to fast code. + */ + HDmemcpy(&vl, _vl, sizeof(hvl_t)); *len = vl.len; -#endif FUNC_LEAVE_NOAPI(SUCCEED) } /* end H5T__vlen_mem_seq_getlen() */ @@ -407,25 +402,16 @@ H5T__vlen_mem_seq_getlen(H5VL_object_t H5_ATTR_UNUSED *file, const void *_vl, si static void * H5T__vlen_mem_seq_getptr(void *_vl) { -#ifdef H5_NO_ALIGNMENT_RESTRICTIONS - const hvl_t *vl = (const hvl_t *)_vl; /* Pointer to the user's hvl_t information */ -#else hvl_t vl; /* User's hvl_t information */ -#endif FUNC_ENTER_PACKAGE_NOERR /* check parameters, return result */ -#ifdef H5_NO_ALIGNMENT_RESTRICTIONS - HDassert(vl); - - FUNC_LEAVE_NOAPI(vl->p) -#else HDassert(_vl); - H5MM_memcpy(&vl, _vl, sizeof(hvl_t)); + /* Copy to ensure correct alignment. */ + HDmemcpy(&vl, _vl, sizeof(hvl_t)); FUNC_LEAVE_NOAPI(vl.p) -#endif } /* end H5T__vlen_mem_seq_getptr() */ /*------------------------------------------------------------------------- @@ -443,24 +429,17 @@ H5T__vlen_mem_seq_getptr(void *_vl) static herr_t H5T__vlen_mem_seq_isnull(const H5VL_object_t H5_ATTR_UNUSED *file, void *_vl, hbool_t *isnull) { -#ifdef H5_NO_ALIGNMENT_RESTRICTIONS - const hvl_t *vl = (const hvl_t *)_vl; /* Pointer to the user's hvl_t information */ -#else hvl_t vl; /* User's hvl_t information */ -#endif FUNC_ENTER_PACKAGE_NOERR /* Check parameters */ HDassert(_vl); -#ifdef H5_NO_ALIGNMENT_RESTRICTIONS - *isnull = ((vl->len == 0 || vl->p == NULL) ? TRUE : FALSE); -#else - H5MM_memcpy(&vl, _vl, sizeof(hvl_t)); + /* Copy to ensure correct alignment. */ + HDmemcpy(&vl, _vl, sizeof(hvl_t)); *isnull = ((vl.len == 0 || vl.p == NULL) ? TRUE : FALSE); -#endif FUNC_LEAVE_NOAPI(SUCCEED) } /* end H5T__vlen_mem_seq_isnull() */ @@ -512,27 +491,18 @@ H5T__vlen_mem_seq_setnull(H5VL_object_t H5_ATTR_UNUSED *file, void *_vl, void H5 static herr_t H5T__vlen_mem_seq_read(H5VL_object_t H5_ATTR_UNUSED *file, void *_vl, void *buf, size_t len) { -#ifdef H5_NO_ALIGNMENT_RESTRICTIONS - const hvl_t *vl = (const hvl_t *)_vl; /* Pointer to the user's hvl_t information */ -#else hvl_t vl; /* User's hvl_t information */ -#endif FUNC_ENTER_PACKAGE_NOERR /* check parameters, copy data */ HDassert(buf); -#ifdef H5_NO_ALIGNMENT_RESTRICTIONS - HDassert(vl && vl->p); - - H5MM_memcpy(buf, vl->p, len); -#else HDassert(_vl); - H5MM_memcpy(&vl, _vl, sizeof(hvl_t)); + /* Copy to ensure correct alignment. */ + HDmemcpy(&vl, _vl, sizeof(hvl_t)); HDassert(vl.p); H5MM_memcpy(buf, vl.p, len); -#endif FUNC_LEAVE_NOAPI(SUCCEED) } /* end H5T__vlen_mem_seq_read() */ @@ -606,20 +576,15 @@ done: static herr_t H5T__vlen_mem_str_getlen(H5VL_object_t H5_ATTR_UNUSED *file, const void *_vl, size_t *len) { -#ifdef H5_NO_ALIGNMENT_RESTRICTIONS - const char *s = *(const char *const *)_vl; /* Pointer to the user's string information */ -#else const char *s = NULL; /* Pointer to the user's string information */ -#endif FUNC_ENTER_PACKAGE_NOERR /* check parameters */ HDassert(_vl); -#ifndef H5_NO_ALIGNMENT_RESTRICTIONS - H5MM_memcpy(&s, _vl, sizeof(char *)); -#endif + /* Copy to ensure correct alignment. */ + HDmemcpy(&s, _vl, sizeof(char *)); *len = HDstrlen(s); @@ -641,21 +606,14 @@ H5T__vlen_mem_str_getlen(H5VL_object_t H5_ATTR_UNUSED *file, const void *_vl, si static void * H5T__vlen_mem_str_getptr(void *_vl) { -#ifdef H5_NO_ALIGNMENT_RESTRICTIONS - char *s = *(char **)_vl; /* Pointer to the user's string information */ -#else - char * s = NULL; /* Pointer to the user's string information */ -#endif + char *s = NULL; /* Pointer to the user's string information */ FUNC_ENTER_PACKAGE_NOERR /* check parameters */ -#ifdef H5_NO_ALIGNMENT_RESTRICTIONS - HDassert(s); -#else HDassert(_vl); - H5MM_memcpy(&s, _vl, sizeof(char *)); -#endif + /* Copy to ensure correct alignment. */ + HDmemcpy(&s, _vl, sizeof(char *)); FUNC_LEAVE_NOAPI(s) } /* end H5T__vlen_mem_str_getptr() */ @@ -675,17 +633,12 @@ H5T__vlen_mem_str_getptr(void *_vl) static herr_t H5T__vlen_mem_str_isnull(const H5VL_object_t H5_ATTR_UNUSED *file, void *_vl, hbool_t *isnull) { -#ifdef H5_NO_ALIGNMENT_RESTRICTIONS - char *s = *(char **)_vl; /* Pointer to the user's string information */ -#else char *s = NULL; /* Pointer to the user's string information */ -#endif FUNC_ENTER_PACKAGE_NOERR -#ifndef H5_NO_ALIGNMENT_RESTRICTIONS - H5MM_memcpy(&s, _vl, sizeof(char *)); -#endif + /* Copy to ensure correct alignment. */ + HDmemcpy(&s, _vl, sizeof(char *)); *isnull = (s == NULL ? TRUE : FALSE); @@ -732,23 +685,16 @@ H5T__vlen_mem_str_setnull(H5VL_object_t H5_ATTR_UNUSED *file, void *_vl, void H5 static herr_t H5T__vlen_mem_str_read(H5VL_object_t H5_ATTR_UNUSED *file, void *_vl, void *buf, size_t len) { -#ifdef H5_NO_ALIGNMENT_RESTRICTIONS - char *s = *(char **)_vl; /* Pointer to the user's string information */ -#else - char *s; /* Pointer to the user's string information */ -#endif + char *s; /* Pointer to the user's string information */ FUNC_ENTER_PACKAGE_NOERR if (len > 0) { /* check parameters */ HDassert(buf); -#ifdef H5_NO_ALIGNMENT_RESTRICTIONS - HDassert(s); -#else HDassert(_vl); - H5MM_memcpy(&s, _vl, sizeof(char *)); -#endif + /* Copy to ensure correct alignment. */ + HDmemcpy(&s, _vl, sizeof(char *)); H5MM_memcpy(buf, s, len); } /* end if */ -- cgit v0.12