Bring key changesets from develop (HDFGroup#3052)

* Bump GitHub action macOS version to 13 (HDFGroup#2999, HDFGroup#3009) * Bump GitHub action gcc/g++/gfortran version to 12 (HDFGroup#3015) * Bump Autoconf version to 2.71 (HDFGroup#2944) * Fix missing h5_reset() calls in accum test (HDFGroup#3001) * Only run ttsafe in GitHub thread-safe actions (HDFGroup#2777) * Fix Java debug asserts on Windows (HDFGroup#3012) * Fix long double dt_arith bug on macOS (HDFGroup#3038)
brtnfld · Oct 5, 2023 · bece7c1 · bece7c1
1 parent 39a78a8
commit bece7c1
Show file tree

Hide file tree

Showing 12 changed files with 85 additions and 39 deletions.
diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
@@ -157,7 +157,7 @@ jobs:
           # We could also build with the Autotools via brew installing them,
           # but that seems unnecessary
           - name: "MacOS Clang CMake"
-            os: macos-11
+            os: macos-13
             cpp: ON
             fortran: OFF
             java: ON
@@ -324,10 +324,10 @@ jobs:
         run: |
            sudo apt update
            sudo apt install automake autoconf libtool libtool-bin
-           sudo apt install gcc-11 g++-11 gfortran-11
-           echo "CC=gcc-11" >> $GITHUB_ENV
-           echo "CXX=g++-11" >> $GITHUB_ENV
-           echo "FC=gfortran-11" >> $GITHUB_ENV
+           sudo apt install gcc-12 g++-12 gfortran-12
+           echo "CC=gcc-12" >> $GITHUB_ENV
+           echo "CXX=g++-12" >> $GITHUB_ENV
+           echo "FC=gfortran-12" >> $GITHUB_ENV
            sudo apt install libaec0 libaec-dev
         if: (matrix.generator == 'autogen') && (matrix.parallel != 'enable')
 
@@ -347,7 +347,7 @@ jobs:
 
       - name: Install Dependencies (macOS)
         run: brew install ninja
-        if: matrix.os == 'macos-11'
+        if: matrix.os == 'macos-13'
 
       - name: Set environment for MSVC (Windows)
         run: |
@@ -429,10 +429,23 @@ jobs:
         if: (matrix.generator == 'autogen') && (matrix.run_tests) && ! (matrix.thread_safety.enabled)
 
       - name: CMake Run Tests
-        run: ctest --build . --parallel 2 -C ${{ matrix.build_mode.cmake }} -V
+        run: ctest . --parallel 2 -C ${{ matrix.build_mode.cmake }} -V
         working-directory: ${{ runner.workspace }}/build
-        # Skip Debug MSVC while we investigate H5L Java test timeouts
-        if: (matrix.generator != 'autogen') && (matrix.run_tests) && ! ((matrix.name == 'Windows MSVC CMake') && (matrix.build_mode.cmake == 'Debug'))
+        if: (matrix.generator != 'autogen') && (matrix.run_tests) && ! (matrix.thread_safety.enabled)
+
+      # THREAD-SAFE
+
+      - name: Autotools Run Thread-Safe Tests
+        run: |
+          cd test
+          ./ttsafe
+        working-directory: ${{ runner.workspace }}/build
+        if: (matrix.generator == 'autogen') && (matrix.run_tests) && (matrix.thread_safety.enabled)
+
+      - name: CMake Run Thread-Safe Tests
+        run: ctest . --parallel 2 -C ${{ matrix.build_mode.cmake }} -V -R ttsafe
+        working-directory: ${{ runner.workspace }}/build
+        if: (matrix.generator != 'autogen') && (matrix.run_tests) && (matrix.thread_safety.enabled)
 
       #
       # INSTALL (note that this runs even when we don't run the tests)

diff --git a/config/cmake/HDF5_Examples.cmake.in b/config/cmake/HDF5_Examples.cmake.in
@@ -87,6 +87,11 @@ if(WIN32)
   set(CTEST_BINARY_NAME ${CTEST_SOURCE_NAME}\\build)
   set(CTEST_SOURCE_DIRECTORY "${CTEST_DASHBOARD_ROOT}\\${CTEST_SOURCE_NAME}")
   set(CTEST_BINARY_DIRECTORY "${CTEST_DASHBOARD_ROOT}\\${CTEST_BINARY_NAME}")
+else(APPLE)
+  set(ENV{DYLD_LIBRARY_PATH} "${INSTALLDIR}/lib")
+  set(CTEST_BINARY_NAME ${CTEST_SOURCE_NAME}/build)
+  set(CTEST_SOURCE_DIRECTORY "${CTEST_DASHBOARD_ROOT}/${CTEST_SOURCE_NAME}")
+  set(CTEST_BINARY_DIRECTORY "${CTEST_DASHBOARD_ROOT}/${CTEST_BINARY_NAME}")
 else()
   set(ENV{LD_LIBRARY_PATH} "${INSTALLDIR}/lib")
   set(CTEST_BINARY_NAME ${CTEST_SOURCE_NAME}/build)

diff --git a/config/cmake/jrunTest.cmake b/config/cmake/jrunTest.cmake
@@ -56,6 +56,8 @@ endif ()
 
 if (WIN32)
   set (ENV{PATH} "$ENV{PATH}\\;${TEST_LIBRARY_DIRECTORY}")
+elseif (APPLE)
+  set (ENV{DYLD_LIBRARY_PATH} "$ENV{DYLD_LIBRARY_PATH}:${TEST_LIBRARY_DIRECTORY}")
 else ()
   set (ENV{LD_LIBRARY_PATH} "$ENV{LD_LIBRARY_PATH}:${TEST_LIBRARY_DIRECTORY}")
 endif ()

diff --git a/config/cmake_ext_mod/grepTest.cmake b/config/cmake_ext_mod/grepTest.cmake
@@ -42,6 +42,8 @@ message (STATUS "COMMAND: ${TEST_EMULATOR} ${TEST_PROGRAM} ${TEST_ARGS}")
 if (TEST_LIBRARY_DIRECTORY)
   if (WIN32)
     set (ENV{PATH} "$ENV{PATH};${TEST_LIBRARY_DIRECTORY}")
+  elseif (APPLE)
+    set (ENV{DYLD_LIBRARY_PATH} "$ENV{DYLD_LIBRARY_PATH}:${TEST_LIBRARY_DIRECTORY}")
   else ()
     set (ENV{LD_LIBRARY_PATH} "$ENV{LD_LIBRARY_PATH}:${TEST_LIBRARY_DIRECTORY}")
   endif ()

diff --git a/config/cmake_ext_mod/runTest.cmake b/config/cmake_ext_mod/runTest.cmake
@@ -40,6 +40,8 @@ message (STATUS "COMMAND: ${TEST_EMULATOR} ${TEST_PROGRAM} ${TEST_ARGS}")
 if (TEST_LIBRARY_DIRECTORY)
   if (WIN32)
     set (ENV{PATH} "$ENV{PATH};${TEST_LIBRARY_DIRECTORY}")
+  elseif (APPLE)
+    set (ENV{DYLD_LIBRARY_PATH} "$ENV{DYLD_LIBRARY_PATH}:${TEST_LIBRARY_DIRECTORY}")
   else ()
     set (ENV{LD_LIBRARY_PATH} "$ENV{LD_LIBRARY_PATH}:${TEST_LIBRARY_DIRECTORY}")
   endif ()

diff --git a/configure.ac b/configure.ac
@@ -13,7 +13,7 @@
 ## ----------------------------------------------------------------------
 ## Initialize configure.
 ##
-AC_PREREQ([2.69])
+AC_PREREQ([2.71])
 
 ## AC_INIT takes the name of the package, the version number, and an
 ## email address to report bugs. AC_CONFIG_SRCDIR takes a unique file

diff --git a/java/src/jni/h5lImp.c b/java/src/jni/h5lImp.c
@@ -277,7 +277,7 @@ Java_hdf_hdf5lib_H5_H5Lexists(JNIEnv *env, jclass clss, jlong loc_id, jstring na
 JNIEXPORT jobject JNICALL
 Java_hdf_hdf5lib_H5_H5Lget_1info(JNIEnv *env, jclass clss, jlong loc_id, jstring name, jlong access_id)
 {
-    H5L_info2_t infobuf;
+    H5L_info2_t infobuf  = {H5L_TYPE_ERROR, FALSE, -1, H5T_CSET_ERROR, 0};
     const char *linkName = NULL;
     jvalue      args[5];
     herr_t      status  = FAIL;
@@ -340,7 +340,7 @@ JNIEXPORT jobject JNICALL
 Java_hdf_hdf5lib_H5_H5Lget_1info_1by_1idx(JNIEnv *env, jclass clss, jlong loc_id, jstring name,
                                           jint index_field, jint order, jlong link_n, jlong access_id)
 {
-    H5L_info2_t infobuf;
+    H5L_info2_t infobuf   = {H5L_TYPE_ERROR, FALSE, -1, H5T_CSET_ERROR, 0};
     const char *groupName = NULL;
     jvalue      args[5];
     herr_t      status  = FAIL;
@@ -454,7 +454,7 @@ JNIEXPORT jint JNICALL
 Java_hdf_hdf5lib_H5_H5Lget_1value(JNIEnv *env, jclass clss, jlong loc_id, jstring name,
                                   jobjectArray link_value, jlong access_id)
 {
-    H5L_info2_t infobuf;
+    H5L_info2_t infobuf   = {H5L_TYPE_ERROR, FALSE, -1, H5T_CSET_ERROR, 0};
     const char *file_name = NULL;
     const char *obj_name  = NULL;
     const char *linkName  = NULL;
@@ -544,7 +544,7 @@ Java_hdf_hdf5lib_H5_H5Lget_1value_1by_1idx(JNIEnv *env, jclass clss, jlong loc_i
                                            jint index_field, jint order, jlong link_n,
                                            jobjectArray link_value, jlong access_id)
 {
-    H5L_info2_t infobuf;
+    H5L_info2_t infobuf   = {H5L_TYPE_ERROR, FALSE, -1, H5T_CSET_ERROR, 0};
     const char *file_name = NULL;
     const char *obj_name  = NULL;
     const char *grpName   = NULL;

diff --git a/release_docs/RELEASE.txt b/release_docs/RELEASE.txt
@@ -402,6 +402,14 @@ Bug Fixes since HDF5-1.12.2 release
 
     Configuration
     -------------
+    - The accum test now passes on macOS 12+ (Monterey) w/ CMake
+
+      Due to changes in the way macOS handles LD_LIBRARY_PATH, the accum test
+      started failing on macOS 12+ when building with CMake. CMake has been
+      updated to set DYLD_LIBRARY_PATH on macOS and the test now passes.
+
+      Fixes GitHub #2994, #2261, and #1289
+
     - Fixed syntax of generator expressions used by CMake
 
       Adding quotes around the generator expression should allow CMake to

diff --git a/src/H5FDtest.c b/src/H5FDtest.c
@@ -71,8 +71,6 @@
  *              VFD feature flags (which do not exist until the driver
  *              is instantiated).
  *
- *              See test/Makefile.am for a list of the VFD strings.
- *
  *              This function is only intended for use in the test code.
  *
  * Return:	    TRUE (1) if the VFD supports SWMR I/O or vfd_name is
@@ -82,10 +80,6 @@
  *
  *              This function cannot fail at this time so there is no
  *              error return value.
- *
- * Programmer:	Dana Robinson
- *              Fall 2014
- *
  *-------------------------------------------------------------------------
  */
 hbool_t

diff --git a/src/H5Tconv.c b/src/H5Tconv.c
@@ -887,7 +887,17 @@ done:
 /* Macro defining action on source data which needs to be aligned (before main action) */
 #define H5T_CONV_LOOP_PRE_SALIGN(ST)                                                                         \
     {                                                                                                        \
-        H5MM_memcpy(&src_aligned, src, sizeof(ST));                                                          \
+        /* The uint8_t * cast is required to avoid tripping over undefined behavior.                         \
+         *                                                                                                   \
+         * The typed pointer arrives via a void pointer, which may have any alignment.                       \
+         * We then cast it to a pointer to a type that is assumed to be aligned, which                       \
+         * is undefined behavior (section 6.3.2.3 paragraph 7 of the C99 standard).                          \
+         * In the past this hasn't caused many problems, but in some cases (e.g.                             \
+         * converting long doubles on macOS), an optimizing compiler might do the                            \
+         * wrong thing (in the macOS case, the conversion uses SSE, which has stricter                       \
+         * requirements about alignment).                                                                    \
+         */                                                                                                  \
+        H5MM_memcpy(&src_aligned, (const uint8_t *)src, sizeof(ST));                                         \
     }
 
 /* Macro defining action on source data which doesn't need to be aligned (before main action) */
@@ -919,7 +929,17 @@ done:
 /* Macro defining action on destination data which needs to be aligned (after main action) */
 #define H5T_CONV_LOOP_POST_DALIGN(DT)                                                                        \
     {                                                                                                        \
-        H5MM_memcpy(dst, &dst_aligned, sizeof(DT));                                                          \
+        /* The uint8_t * cast is required to avoid tripping over undefined behavior.                         \
+         *                                                                                                   \
+         * The typed pointer arrives via a void pointer, which may have any alignment.                       \
+         * We then cast it to a pointer to a type that is assumed to be aligned, which                       \
+         * is undefined behavior (section 6.3.2.3 paragraph 7 of the C99 standard).                          \
+         * In the past this hasn't caused many problems, but in some cases (e.g.                             \
+         * converting long doubles on macOS), an optimizing compiler might do the                            \
+         * wrong thing (in the macOS case, the conversion uses SSE, which has stricter                       \
+         * requirements about alignment).                                                                    \
+         */                                                                                                  \
+        H5MM_memcpy((uint8_t *)dst, &dst_aligned, sizeof(DT));                                               \
     }
 
 /* Macro defining action on destination data which doesn't need to be aligned (after main action) */

diff --git a/test/accum_swmr_reader.c b/test/accum_swmr_reader.c
@@ -34,24 +34,24 @@ const char *FILENAME[] = {"accum", "accum_swmr_big", NULL};
  *
  * Return:      Success: EXIT_SUCCESS
  *              Failure: EXIT_FAILURE
- *
- * Programmer:  Vailin Choi; June 2013
- *
  *-------------------------------------------------------------------------
  */
 int
 main(void)
 {
-    hid_t    fid  = -1;   /* File ID */
-    hid_t    fapl = -1;   /* file access property list ID */
-    H5F_t   *f    = NULL; /* File pointer */
+    hid_t    fid  = H5I_INVALID_HID; /* File ID */
+    hid_t    fapl = H5I_INVALID_HID; /* file access property list ID */
+    H5F_t   *f    = NULL;            /* File pointer */
     char     filename[1024];
     unsigned u;                      /* Local index variable */
     uint8_t  rbuf[1024];             /* Buffer for reading */
     uint8_t  buf[1024];              /* Buffer for holding the expected data */
     char    *driver         = NULL;  /* VFD string (from env variable) */
     hbool_t  api_ctx_pushed = FALSE; /* Whether API context pushed */
 
+    /* Testing setup */
+    h5_reset();
+
     /* Skip this test if SWMR I/O is not supported for the VFD specified
      * by the environment variable.
      */

diff --git a/test/swmr.c b/test/swmr.c
@@ -7728,12 +7728,15 @@ test_multiple_same(hid_t in_fapl, hbool_t new_format)
 int
 main(void)
 {
-    int     nerrors      = 0;             /* The # of errors */
-    hid_t   fapl         = -1;            /* File access property list ID */
-    char   *driver       = NULL;          /* VFD string (from env variable) */
-    char   *lock_env_var = NULL;          /* file locking env var pointer */
-    hbool_t use_file_locking;             /* read from env var */
-    hbool_t file_locking_enabled = FALSE; /* Checks if the file system supports locks */
+    int     nerrors      = 0;               /* The # of errors */
+    hid_t   fapl         = H5I_INVALID_HID; /* File access property list ID */
+    char   *driver       = NULL;            /* VFD string (from env variable) */
+    char   *lock_env_var = NULL;            /* file locking env var pointer */
+    hbool_t use_file_locking;               /* read from env var */
+    hbool_t file_locking_enabled = FALSE;   /* Checks if the file system supports locks */
+
+    /* Testing setup */
+    h5_reset();
 
     /* Skip this test if SWMR I/O is not supported for the VFD specified
      * by the environment variable.
@@ -7742,7 +7745,7 @@ main(void)
     if (!H5FD__supports_swmr_test(driver)) {
         HDprintf("This VFD does not support SWMR I/O\n");
         return EXIT_SUCCESS;
-    } /* end if */
+    }
 
     /* Check the environment variable that determines if we care
      * about file locking. File locking should be used unless explicitly
@@ -7761,9 +7764,6 @@ main(void)
             return EXIT_FAILURE;
         }
 
-    /* Set up */
-    h5_reset();
-
     /* Get file access property list */
     fapl = h5_fileaccess();
 
@@ -7822,7 +7822,7 @@ main(void)
          */
         nerrors += test_file_lock_swmr_same(fapl);
         nerrors += test_file_lock_swmr_concur(fapl);
-    } /* end if */
+    }
 
     /* Tests SWMR VFD compatibility flag.
      * Only needs to run when the VFD is the default (sec2).