Merge pull request #138 from biojppm/posix_memalign

Ensure posix_memalign() is never called with bad alignment values
biojppm · Jul 6, 2024 · cfbc92d · cfbc92d
2 parents edd2649 + 003450a
commit cfbc92d
Show file tree

Hide file tree

Showing 5 changed files with 56 additions and 31 deletions.
diff --git a/.github/setenv.sh b/.github/setenv.sh
@@ -36,6 +36,7 @@ function c4_show_info()
     echo "ARM=$ARM"
     echo "LIBCXX=$LIBCXX"
     echo "VERBOSE_MAKEFILES=$VERBOSE_MAKEFILES"
+    set -x
     which cmake
     cmake --version
     case "$CXX_" in
@@ -52,7 +53,8 @@ function c4_show_info()
         *)
             ;;
     esac
-    set -x
+    pwd
+    ls
     git branch
     git rev-parse HEAD
     git tag || echo

diff --git a/.github/workflows/clang.yml b/.github/workflows/clang.yml
@@ -158,8 +158,13 @@ jobs:
       #... so we checkout manually:
       - name: checkout
         run: |
-          git clone --recursive https://github.com/biojppm/c4core .
-      - run: git config --system --add safe.directory '*'  # needed for running in the docker image. see https://github.com/actions/checkout/issues/1169
+          set -x
+          git init -q .
+          git config --system --add safe.directory '*'  # needed for running in the docker image. see https://github.com/actions/checkout/issues/1169
+          git remote add origin $GITHUB_SERVER_URL/$GITHUB_REPOSITORY
+          git fetch origin $GITHUB_HEAD_REF
+          git reset --hard FETCH_HEAD
+          git submodule update --init --recursive
       - run: c4core-install ${{matrix.cxx}}
       - {name: show info, run: source .github/setenv.sh && c4_show_info}
       - name: shared64-configure---------------------------------------------------

diff --git a/.github/workflows/gcc.yml b/.github/workflows/gcc.yml
@@ -52,8 +52,13 @@ jobs:
       #... so we checkout manually:
       - name: checkout
         run: |
-          git clone --recursive https://github.com/biojppm/c4core .
-      - run: git config --system --add safe.directory '*'  # needed for running in the docker image. see https://github.com/actions/checkout/issues/1169
+          set -x
+          git init -q .
+          git config --system --add safe.directory '*'  # needed for running in the docker image. see https://github.com/actions/checkout/issues/1169
+          git remote add origin $GITHUB_SERVER_URL/$GITHUB_REPOSITORY
+          git fetch origin $GITHUB_HEAD_REF
+          git reset --hard FETCH_HEAD
+          git submodule update --init --recursive
       - run: c4core-install ${{matrix.cxx}}
       - {name: show info, run: source .github/setenv.sh && c4_show_info}
       - name: shared64-configure---------------------------------------------------
@@ -99,8 +104,13 @@ jobs:
       #... so we checkout manually:
       - name: checkout
         run: |
-          git clone --recursive https://github.com/biojppm/c4core .
-      - run: git config --system --add safe.directory '*'  # needed for running in the docker image. see https://github.com/actions/checkout/issues/1169
+          set -x
+          git init -q .
+          git config --system --add safe.directory '*'  # needed for running in the docker image. see https://github.com/actions/checkout/issues/1169
+          git remote add origin $GITHUB_SERVER_URL/$GITHUB_REPOSITORY
+          git fetch origin $GITHUB_HEAD_REF
+          git reset --hard FETCH_HEAD
+          git submodule update --init --recursive
       - run: c4core-install ${{matrix.cxx}}
       - {name: show info, run: source .github/setenv.sh && c4_show_info}
       - name: shared64-configure---------------------------------------------------
@@ -174,8 +184,13 @@ jobs:
       #... so we checkout manually:
       - name: checkout
         run: |
-          git clone --recursive https://github.com/biojppm/c4core .
-      - run: git config --system --add safe.directory '*'  # needed for running in the docker image. see https://github.com/actions/checkout/issues/1169
+          set -x
+          git init -q .
+          git config --system --add safe.directory '*'  # needed for running in the docker image. see https://github.com/actions/checkout/issues/1169
+          git remote add origin $GITHUB_SERVER_URL/$GITHUB_REPOSITORY
+          git fetch origin $GITHUB_HEAD_REF
+          git reset --hard FETCH_HEAD
+          git submodule update --init --recursive
       - run: c4core-install ${{matrix.cxx}}
       - {name: show info, run: source .github/setenv.sh && c4_show_info}
       - name: shared64-configure---------------------------------------------------
@@ -256,7 +271,7 @@ jobs:
   #----------------------------------------------------------------------------
   arm:
     continue-on-error: true
-    if: always()  # https://stackoverflow.com/questions/62045967/github-actions-is-there-a-way-to-continue-on-error-while-still-getting-correct 
+    if: always()  # https://stackoverflow.com/questions/62045967/github-actions-is-there-a-way-to-continue-on-error-while-still-getting-correct
     runs-on: ubuntu-latest
     container: ghcr.io/biojppm/c4core/ubuntu18.04:latest  # use the docker image
     strategy:
@@ -277,8 +292,13 @@ jobs:
       #... so we checkout manually:
       - name: checkout
         run: |
-          git clone --recursive https://github.com/biojppm/c4core .
-      - run: git config --system --add safe.directory '*'  # needed for running in the docker image. see https://github.com/actions/checkout/issues/1169
+          set -x
+          git init -q .
+          git config --system --add safe.directory '*'  # needed for running in the docker image. see https://github.com/actions/checkout/issues/1169
+          git remote add origin $GITHUB_SERVER_URL/$GITHUB_REPOSITORY
+          git fetch origin $GITHUB_HEAD_REF
+          git reset --hard FETCH_HEAD
+          git submodule update --init --recursive
       - name: install gcc-arm-linux-gnueabihf
         run: |
           c4core-install arm-linux-gnueabihf

diff --git a/changelog/current.md b/changelog/current.md
@@ -1,8 +1,9 @@
 
-- Fix [rapidyaml#445](https://github.com/biojppm/biojppm/pull/445): Amalgamate: fix include of `<charconv>`.
+- Amalgamate: fix include of `<charconv>` (see [rapidyaml#445](https://github.com/biojppm/biojppm/pull/445)).
 - Add `C4_MINGW` ([PR#139](https://github.com/biojppm/c4core/pull/139))
 - Annotate `c4::handle_error()` with `[[noreturn]]` ([PR#137](https://github.com/biojppm/c4core/pull/137))
 - Add `bool from_chars(csubstr s, fmt::overflow_checked_<T> *wrapper)`. There was already a function receiving `&wrapper`, but `*wrapper` was missing for use with generic code.
+- Ensure `posix_memalign()` is never called with bad alignment values ([PR#138](https://github.com/biojppm/c4core/pull/138))
 - Update fast_float to v6.1.1 ([PR#136](https://github.com/biojppm/c4core/pull/136))
 
 

diff --git a/src/c4/memory_resource.cpp b/src/c4/memory_resource.cpp
@@ -38,37 +38,34 @@ void afree_impl(void *ptr)
 
 void* aalloc_impl(size_t size, size_t alignment)
 {
+    // alignment must be nonzero and a power of 2
+    C4_CHECK(alignment > 0 && (alignment & (alignment - 1u)) == 0);
+    // NOTE: alignment needs to be sized in multiples of sizeof(void*)
+    if(C4_UNLIKELY(alignment < sizeof(void*)))
+        alignment = sizeof(void*);
+    static_assert((sizeof(void*) & (sizeof(void*)-1u)) == 0, "sizeof(void*) must be a power of 2");
+    C4_CHECK(((alignment & (sizeof(void*) - 1u))) == 0u);
     void *mem;
 #if defined(C4_WIN) || defined(C4_XBOX)
     mem = ::_aligned_malloc(size, alignment);
     C4_CHECK(mem != nullptr || size == 0);
-#elif defined(C4_ARM) || defined(C4_ANDROID)
-    // https://stackoverflow.com/questions/53614538/undefined-reference-to-posix-memalign-in-arm-gcc
-    // https://electronics.stackexchange.com/questions/467382/e2-studio-undefined-reference-to-posix-memalign/467753
-    mem = memalign(alignment, size);
-    C4_CHECK(mem != nullptr || size == 0);
 #elif defined(C4_POSIX) || defined(C4_IOS) || defined(C4_MACOS)
-    // NOTE: alignment needs to be sized in multiples of sizeof(void*)
-    size_t amult = alignment;
-    if(C4_UNLIKELY(alignment < sizeof(void*)))
-    {
-        amult = sizeof(void*);
-    }
-    int ret = ::posix_memalign(&mem, amult, size);
+    int ret = ::posix_memalign(&mem, alignment, size);
     if(C4_UNLIKELY(ret))
     {
-        if(ret == EINVAL)
-        {
-            C4_ERROR("The alignment argument %zu was not a power of two, "
-                     "or was not a multiple of sizeof(void*)", alignment);
-        }
-        else if(ret == ENOMEM)
+        C4_ASSERT(ret != EINVAL); // this was already handled above
+        if(ret == ENOMEM)
         {
             C4_ERROR("There was insufficient memory to fulfill the "
                      "allocation request of %zu bytes (alignment=%lu)", size, size);
         }
         return nullptr;
     }
+#elif defined(C4_ARM) || defined(C4_ANDROID)
+    // https://stackoverflow.com/questions/53614538/undefined-reference-to-posix-memalign-in-arm-gcc
+    // https://electronics.stackexchange.com/questions/467382/e2-studio-undefined-reference-to-posix-memalign/467753
+    mem = memalign(alignment, size);
+    C4_CHECK(mem != nullptr || size == 0);
 #else
     (void)size;
     (void)alignment;