Skip to content

Commit

Permalink
Remove H5_NO_ALIGNMENT_RESTRICTIONS (HDFGroup#1426)
Browse files Browse the repository at this point in the history 
* 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>
  • Loading branch information
@derobins @github-actions
derobins and github-actions[bot] committed Apr 20, 2022
1 parent 819259e commit 60c4912
Show file tree
Hide file tree
Showing 5 changed files with 20 additions and 153 deletions.
4 changes: 0 additions & 4 deletions config/cmake/ConfigureChecks.cmake
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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")
53 changes: 0 additions & 53 deletions config/cmake/ConversionTests.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -234,59 +234,6 @@ int HDF_NO_UBSAN main(void)
}
#endif

#ifdef H5_NO_ALIGNMENT_RESTRICTIONS_TEST

#include <stdlib.h>
#include <string.h>

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 <stdio.h>
Expand Down
3 changes: 0 additions & 3 deletions config/cmake/H5pubconf.h.in
View file
Original file line number Diff line number Diff line change
Expand Up @@ -440,9 +440,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@

Expand Down
19 changes: 0 additions & 19 deletions configure.ac
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4038,25 +4038,6 @@ AC_ARG_ENABLE([embedded-libinfo],
fi


## ----------------------------------------------------------------------
## 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"
Expand Down
94 changes: 20 additions & 74 deletions src/H5Tvlen.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -369,25 +369,20 @@ H5T__vlen_set_loc(H5T_t *dt, H5VL_object_t *file, H5T_loc_t loc)
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

/* Check parameter */
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() */
Expand All @@ -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() */

/*-------------------------------------------------------------------------
Expand All @@ -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() */
Expand Down Expand Up @@ -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() */
Expand Down Expand Up @@ -606,20 +576,15 @@ H5T__vlen_mem_seq_write(H5VL_object_t H5_ATTR_UNUSED *file, const H5T_vlen_alloc
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);

Expand All @@ -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() */
Expand All @@ -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);

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

0 comments on commit 60c4912

Please sign in to comment.