GH-46123: [C++] Undefined behavior in compare_internal.cc and light_array_internal.cc (#46124)

### Rationale for this change

Removing undefined behaviors, adding a CI job for the M1-mac sanitizer now being run.

### What changes are included in this PR?

The fixes + a sanitizer job that confirms they are fixed.

### Are these changes tested?

Yup!

### Are there any user-facing changes?

Fewer undefined behaviors.
* GitHub Issue: #46123

Lead-authored-by: Jonathan Keane <[email protected]>
Co-authored-by: Rossi Sun <[email protected]>
Signed-off-by: Jonathan Keane <[email protected]>

Author: 2 people

ci/scripts/r_sanitize.sh

@@ -36,34 +36,38 @@ ncores=$(${R_BIN} -s -e 'cat(parallel::detectCores())')
 echo "MAKEFLAGS=-j${ncores}" >> ${rhome}/etc/Renviron.site
 
 # build first so that any stray compiled files in r/src are ignored
-${R_BIN} CMD build .
-${R_BIN} CMD INSTALL ${INSTALL_ARGS} arrow*.tar.gz
+${R_BIN} CMD build --no-build-vignettes --no-manual .
 
 # But unset the env var so that it doesn't cause us to run extra dev tests
 unset ARROW_R_DEV
 
 # Set the testthat output to be verbose for easier debugging
 export ARROW_R_VERBOSE_TEST=TRUE
 
-export UBSAN_OPTIONS="print_stacktrace=1,suppressions=/arrow/r/tools/ubsan.supp"
+# We prune dependencies for these, so we need to disable forcing suggests
+export _R_CHECK_FORCE_SUGGESTS_=FALSE
+
+export SUPPRESSION_FILE=$(readlink -f "tools/ubsan.supp")
+export UBSAN_OPTIONS="print_stacktrace=1,suppressions=${SUPPRESSION_FILE}"
 # From the old rhub image https://github.com/r-hub/rhub-linux-builders/blob/master/fedora-clang-devel-san/Dockerfile
 export ASAN_OPTIONS="alloc_dealloc_mismatch=0:detect_leaks=0:detect_odr_violation=0"
 
-# run tests
-pushd tests
-${R_BIN} --no-save < testthat.R > testthat.out 2>&1 || { cat testthat.out; exit 1; }
+${R_BIN} CMD check --no-manual --no-vignettes --no-build-vignettes arrow*.tar.gz
 
-cat testthat.out
-if grep -q "runtime error" testthat.out; then
+# Find sanitizer issues, print the file(s) they are part of, and fail the job
+find . -type f -name "*Rout" -exec grep -l "runtime error\|SUMMARY: UndefinedBehaviorSanitizer" {} \; > sanitizer_errors.txt
+if [ -s sanitizer_errors.txt ]; then
+  echo "Sanitizer errors found in the following files:"
+  cat sanitizer_errors.txt
+  
+  # Print the content of files with errors for debugging
+  while read -r file; do
+    echo "=============== $file ==============="
+    cat "$file"
+    echo "========================================="
+  done < sanitizer_errors.txt
+  
   exit 1
 fi
 
-# run examples
-popd
-${R_BIN} --no-save -e 'library(arrow); testthat::test_examples(".")' >> examples.out 2>&1 || { cat examples.out; exit 1; }
-
-cat examples.out
-if grep -q "runtime error" examples.out; then
-  exit 1
-fi
 popd

cpp/src/arrow/compute/light_array_internal.cc

@@ -611,11 +611,9 @@ Status ExecBatchBuilder::AppendSelected(const std::shared_ptr<ArrayData>& source
           });
     Visit(source, num_rows_to_append - num_rows_to_process, row_ids + num_rows_to_process,
           [&](int i, const uint8_t* ptr, int32_t num_bytes) {
-            uint64_t* dst = reinterpret_cast<uint64_t*>(
-                target->mutable_data(2) +
-                offsets[num_rows_before + num_rows_to_process + i]);
-            const uint64_t* src = reinterpret_cast<const uint64_t*>(ptr);
-            memcpy(dst, src, num_bytes);
+            auto dst = target->mutable_data(2) +
+                       offsets[num_rows_before + num_rows_to_process + i];
+            memcpy(dst, ptr, num_bytes);
           });
   }
 

cpp/src/arrow/compute/row/compare_internal.cc

@@ -275,7 +275,11 @@ void KeyCompare::CompareVarBinaryColumnToRowHelper(
       int32_t tail_length = length - j * 8;
       uint64_t tail_mask = ~0ULL >> (64 - 8 * tail_length);
       uint64_t key_left = 0;
-      std::memcpy(&key_left, key_left_ptr + j, tail_length);
+      // NOTE: UBSAN may falsely report "misaligned load" in `std::memcpy` on some
+      // platforms when using 64-bit pointers. Cast to an 8-bit pointer to work around
+      // this.
+      const uint8_t* src_bytes = reinterpret_cast<const uint8_t*>(key_left_ptr + j);
+      std::memcpy(&key_left, src_bytes, tail_length);
       uint64_t key_right = key_right_ptr[j];
       result_or |= tail_mask & (key_left ^ key_right);
     }

dev/tasks/docker-tests/github.linux.yml

@@ -66,7 +66,11 @@ jobs:
         uses: actions/upload-artifact@v4
         with:
           name: test-output
-          path: arrow/r/check/arrow.Rcheck/tests/testthat.Rout*
+          path: |
+            arrow/r/tests/
+            arrow/r/arrow.Rcheck/
+            !arrow/r/arrow.Rcheck/00_pkg_src/
+            !arrow/r/arrow.Rcheck/arrow/
           if-no-files-found: ignore
 
     {% if arrow.is_default_branch() %}

dev/tasks/r/github.macos.m1san.yml

@@ -0,0 +1,97 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+{% import 'macros.jinja' as macros with context %}
+
+{{ macros.github_header() }}
+
+jobs:
+  macos-cran:
+    name: "m1-san on macOS"
+    runs-on: macOS-15
+    strategy:
+      fail-fast: false
+
+    steps:
+      {{ macros.github_checkout_arrow()|indent }}
+
+      - name: Configure dependencies (macos)
+        run: |
+          brew install openssl
+      - uses: r-lib/actions/setup-r@v2
+        with:
+          use-public-rspm: true
+          r-version: devel
+      - name: Install dependencies
+        uses: r-lib/actions/setup-r-dependencies@v2
+        with:
+          cache: false # cache does not work on across branches
+          working-directory: arrow/r
+          extra-packages: |
+            any::rcmdcheck
+            any::sys
+      - name: Setup the sanitizer
+        run: |
+          # From rhub/rhub's m1-san:
+          # https://github.com/r-hub/actions/blob/ab9b2fb5021f43bfd0ee977932e0a14d25f6e59e/run-check/scripts/mac-asan.sh
+          # Use XCode 16.3 ------------------------------------------------
+          sudo rm -f /Applications/Xcode.app
+          sudo ln -sfF /Applications/Xcode_16.3.app /Applications/Xcode.app
+          sudo xcode-select -s /Applications/Xcode.app
+
+          # Compile with sanitizers ---------------------------------------
+          mkdir -p ~/.R
+          cat >> ~/.R/Makevars <<EOF
+          CC+=-mmacos-version-min=15 -fsanitize=address,undefined
+          CXX+=-mmacos-version-min=15 -fsanitize=address,undefined
+          CXX11+=-mmacos-version-min=15 -fsanitize=address,undefined
+          CXX14+=-mmacos-version-min=15 -fsanitize=address,undefined
+          CXX17+=-mmacos-version-min=15 -fsanitize=address,undefined
+          CXX20+=-mmacos-version-min=15 -fsanitize=address,undefined
+          CXX23+=-mmacos-version-min=15 -fsanitize=address,undefined
+          REC_INSTALL_OPT=--dsym
+          EOF
+
+          # Need to patch the shell script, because these env vars --------
+          # do not go through a shell because of Apple security.
+          R=$(readlink `which R`)
+          head -1 ${R} >> /tmp/R
+          cat >> /tmp/R <<EOF
+          export DYLD_FORCE_FLAT_NAMESPACE=1
+          export DYLD_INSERT_LIBRARIES=/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/lib/clang/17/lib/darwin/libclang_rt.asan_osx_dynamic.dylib
+          EOF
+          cat ${R} >> /tmp/R
+          chmod +x /tmp/R
+          sudo mv /tmp/R ${R}
+      - name: Install and check
+        env:
+          R_BIN: R
+          LIBARROW_BINARY: FALSE
+        run: |
+          export ARROW_SOURCE_HOME=$(pwd)/arrow
+          export INSTALL_ARGS="--no-test-load"
+          arrow/ci/scripts/r_sanitize.sh arrow
+      - name: Save the test output
+        uses: actions/upload-artifact@v4
+        with:
+          name: test-output
+          path: |
+            arrow/r/tests/
+            arrow/r/arrow.Rcheck/
+            !arrow/r/arrow.Rcheck/00_pkg_src/
+            !arrow/r/arrow.Rcheck/arrow/
+        if: always()

dev/tasks/tasks.yml

@@ -1087,13 +1087,25 @@ tasks:
       image: ubuntu-r-sanitizer
       timeout: 120
 
-  test-r-clang-sanitizer:
+  test-r-clang-asan:
     ci: github
     template: docker-tests/github.linux.yml
     params:
       env:
         R_PRUNE_DEPS: TRUE
-      image: r-clang-sanitizer
+      image: r-clang-asan
+
+  test-r-clang-ubsan:
+    ci: github
+    template: docker-tests/github.linux.yml
+    params:
+      env:
+        R_PRUNE_DEPS: TRUE
+      image: r-clang-ubsan
+
+  test-r-m1-san:
+    ci: github
+    template: r/github.macos.m1san.yml
 
   # be sure to update binary-task.rb when upgrading Debian
   test-debian-12-docs:

docker-compose.yml

@@ -162,7 +162,8 @@ x-hierarchy:
   - ubuntu-r-valgrind
   - ubuntu-swift
   - ubuntu-verify-rc
-  - r-clang-sanitizer
+  - r-clang-asan
+  - r-clang-ubsan
   - r
   - r-revdepcheck
   # helper services
@@ -1717,7 +1718,7 @@ services:
       /bin/bash -c "
         /arrow/ci/scripts/r_sanitize.sh /arrow"
 
-  r-clang-sanitizer:
+  r-clang-asan:
     image: ${REPO}:r-rhub-clang-devel-latest
     build:
       context: .
@@ -1737,7 +1738,33 @@ services:
       LIBARROW_DOWNLOAD: "false"
       ARROW_SOURCE_HOME: "/arrow"
       ARROW_R_DEV: ${ARROW_R_DEV}
-      # To test for CRAN release, delete ^^ these two env vars so we download the Apache release
+      ARROW_USE_PKG_CONFIG: "false"
+    volumes:
+      - .:/arrow:delegated
+    command: >
+      /bin/bash -c "
+        /arrow/ci/scripts/r_sanitize.sh /arrow"
+
+  r-clang-ubsan:
+    image: ${REPO}:r-rhub-clang-ubsan-devel-latest
+    build:
+      context: .
+      dockerfile: ci/docker/linux-r.dockerfile
+      cache_from:
+        - ${REPO}:r-rhub-clang-ubsan-devel-latest
+      args:
+        base: rhub/clang-ubsan
+        cmake: ${CMAKE}
+        r_dev: ${ARROW_R_DEV}
+        r_bin: R
+        tz: ${TZ}
+        r_prune_deps: ${R_PRUNE_DEPS}
+    shm_size: *shm-size
+    environment:
+      <<: *common
+      LIBARROW_DOWNLOAD: "false"
+      ARROW_SOURCE_HOME: "/arrow"
+      ARROW_R_DEV: ${ARROW_R_DEV}
       ARROW_USE_PKG_CONFIG: "false"
     volumes:
       - .:/arrow:delegated