Skip to content

DX-128331: Gandiva: normalize Darwin host triple to macosx so exp10 libcall is available - #151

Merged
lriggs merged 4 commits into
dremio:dremio_27.0_23_19from
victor6567758:DX-128331-gandiva-fexp10-crash
Nov 11, 2026
Merged

lriggs merged 4 commits into
dremio:dremio_27.0_23_19from
victor6567758:DX-128331-gandiva-fexp10-crash

Conversation

@victor6567758

@victor6567758 victor6567758 commented Nov 9, 2026 •

Copy link
Copy Markdown

Summary

Fixes DX-128331: on macOS (Apple arm64) with Gandiva IR optimization disabled (we do this past the 2k-expression threshold), expressions such as round(c0, 1) or round(c0, c1) crash the JVM with error: no libcall available for fexp10.

Root cause

Gandiva builds its JIT target machine from llvm::sys::getDefaultTargetTriple(), which on macOS is an unversioned/kernel-versioned arm64-apple-darwin<N>. LLVM only enables macOS libcalls such as __exp10 for the macosx OS name. With optimization off, pow(10, x) (used by get_scale_multiplier for scales outside 0..19) is
lowered to llvm.exp10, and the backend finds no libcall for it.

Fix

Add NormalizeDarwinTriple() (declared in engine.h, defined in engine.cc) and apply it to the host triple in MakeTargetMachineBuilder.

  • darwin<kernel> becomes macosx<version>, using Triple::getMacOSXVersion.
  • If the version is missing or unconvertible, or lower than the floor, use the oldest
    macOS for the architecture: 11.0 for arm64, 10.15 for x86_64.
  • The minor version is always emitted (macosx11.0, not macosx11).
  • Non-Darwin triples are returned unchanged.

Why this approach

  • Same as clang: clang's driver does this rewrite when it targets macOS. The JIT skips the driver, so we do it ourselves. It means:

    • arm64-apple-darwin27.2.0 is what llvm::sys::getDefaultTargetTriple() returns. The OS part is darwin, followed by the kernel version.
    • arm64-apple-macosx27.2.0 is what clang uses internally. The OS part is macosx, followed by a macOS version.
  • Fixes the cause for all libcalls: other macOS-only libcalls hit the same wrong-OS-name problem. Rewriting exp10 or pow in precompiled IR would only patch one symptom and would change the IR for every platform.

  • Only "at least macOS 10.9" matters for libcall selection, so the approximate kernel-to-macOS mapping is acceptable. bool RuntimeLibcallsInfo::darwinHasExp10(const Triple &TT) { switch (TT.getOS()) { case Triple::MacOSX: return !TT.isMacOSXVersionLT(10, 9);

  • Fixing darwinHasExp10 from LLVM would require changes in the additional component what is outside of our control: https://github.com/llvm/llvm-project/blob/main/llvm/lib/IR/RuntimeLibcalls.cpp

Tests

  • engine_llvm_test.cc: unit tests for NormalizeDarwinTriple (darwin to macosx, the floor for unversioned/old, floor boundaries, arch aliases, non-Darwin unchanged, idempotence).
  • projector_test.cc: round(float64, int32) with optimization off. They cover:
    • scales in the lookup table, at the table boundary (20)
    • large scales (25, 30)
    • negative scales, which are the pow(10, scale) paths that lower to exp10.
  • The crash reproduces only on macOS, so on Linux these pass with or without the fix.
  • Confirm the fix is working via UI/local run (validated on a Mac with the JarBuild osx-aarch_64 jar, running the round cases with exec.gandiva.optimize_ir=false ). SELECT ROUND(c0, 1) AS r FROM $scratch.g1 works after the fix.
  • Gitlab Action buildJar is OK: https://github.com/dremio/arrow-java/actions/runs/42818876037

Why we need the new tests:

  1. decimal_test.cc: this test runs OK BEFORE this fix. It cannot catch the problematic code:

tests/decimal_test.cc:408-417:
TreeExprBuilder::MakeFunction("round", {field_a, scale_1}, arrow::decimal128(precision, 1)) ->
void round_decimal128_int32 ( precompiled/decimal_wrapper.cc:146) ->
auto out = gandiva::decimalops::Round(x, out_precision, out_scale, rounding_scale, &overflow) ( precompiled/decimal_ops.cc:662) ->
RoundWithPositiveScale at 615 and RoundWithNegativeScale at 644 (cpp/src/gandiva/precompiled/decimal_ops.cc) ->
It does integer arithmetic using BasicDecimal128::GetScaleMultiplier(scale), which is a table lookup on 128-bit integers.

  1. The new tests results BEFORE the fix (OK after the fix):

10:11-12:20 /Users/victor.zoubok/work/codebase2/arrow/cpp/cmake-build-debug_homebrew [DX-128331-gandiva-fexp10-crash*] > cd /Users/victor.zoubok/work/codebase2/arrow/cpp/cmake-build-debug_homebrew/debug
for t in $(./gandiva-projector-test --gtest_list_tests --gtest_filter='TestProjector.TestRoundFloat64*' | awk '/^[^ ]/{s=$1} /^ /{print s $1}'); do
./gandiva-projector-test --gtest_filter="$t" >/tmp/pt.out 2>&1; echo "exit $? : $t"; grep -m1 "no libcall" /tmp/pt.out
done
exit 1 : TestProjector.TestRoundFloat64ScaleInLookupTable
error: no libcall available for fexp10
exit 1 : TestProjector.TestRoundFloat64ScaleAtTableBoundary
error: no libcall available for fexp10
exit 1 : TestProjector.TestRoundFloat64LargeScale
error: no libcall available for fexp10
exit 1 : TestProjector.TestRoundFloat64NegativeScale
error: no libcall available for fexp10
exit 1 : TestProjector.TestRoundFloat64MixedScalesInOneBatch
error: no libcall available for fexp10
exit 1 : TestProjector.TestRoundFloat64LargeValuesSkipRounding
error: no libcall available for fexp10
exit 1 : TestProjector.TestRoundFloat64ZeroValue
error: no libcall available for fexp10

  1. Here is the flow, using the line numbers in cpp/src/gandiva/precompiled/extended_math_ops.cc. Steps 1 to 4 are in the source. Steps 5 and 6 are how LLVM behaves. Note that native unit tests for round_float64_int32 are called directly, no Gandiva JIT, that's why the issue is not detected.

SQL / expression: round(float64 value, int32 scale)
│
▼

  1. ROUND_DECIMAL_TO_SCALE(float64) lines 312-325
    expands to round_float64_int32(number, out_scale)
    line 315: scale_multiplier = get_scale_multiplier(out_scale);
    |
    ▼
  2. get_scale_multiplier(scale) line 424
    • scale in [0, 20) → values[scale] table lookup, no math call
    • otherwise → power_float64_float64(10.0, scale) line 448
    │
    ▼
  3. POWER(float64, float64, float64) line 218
    power_float64_float64(in1, in2) = powl(in1, in2) line 221
    │
    ▼
  4. Precompiled to LLVM bitcode (precompiled/CMakeLists.txt,
    embedded as kPrecompiledBitcode) and loaded by the JIT engine
    → the whole function is compiled, including the powl(10, x) branch,
    whatever scale the data has
    │
    ▼
  5. LLVM optimization (pow(10, x) → exp10(x), as llvm.exp10)
    │
    ▼
  6. Codegen for the host triple "darwin" (engine.cc)
    → no __exp10 libcall enabled for "darwin"
    → "error: no libcall available for fexp10" → process aborts
    Fix: NormalizeDarwinTriple() rewrites the triple to "macosx"

Review comments

  • I don't add the guards by #ifdef __APPLE__ which assert NormalizeDarwinTriple(llvm::Triple(llvm::sys::getDefaultTargetTriple())).getOS() == llvm::Triple::MacOSX because the function is intended to run correctly on any platform, no real value to disable new tests on Windows or Linux.
  • The rewrite changes the OS name for all JIT codegen on macOS, not only exp10. That is the intent.
  • The ticket also notes that LLVM's diagnostic is turned into a native crash instead of a clean projector-build failure. This PR removes the trigger but does not change that error handling. It could be a follow-up work and it is possible to do as a separate PR, smth like:
    • Install a diagnostic handler on each engine's LLVMContext that records error-severity messages instead of letting LLVM exit.
    • Check the recorded error after compilation, i.e. after FinalizeModule and after the lookup that triggers codegen, and return Status::CodeGenError(msg).

@victor6567758
victor6567758 force-pushed the DX-128331-gandiva-fexp10-crash branch from 7b71843 to 61a1a4d Compare November 10, 2026 12:04
@victor6567758
victor6567758 marked this pull request as ready for review November 10, 2026 14:43

@alinaliBQ alinaliBQ left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall LGTM

Comment thread cpp/src/gandiva/engine_llvm_test.cc Resolved
alinaliBQ
alinaliBQ previously approved these changes Nov 10, 2026

@alinaliBQ alinaliBQ left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@lriggs
lriggs merged commit 148cb48 into dremio:dremio_27.0_23_19 Nov 11, 2026
20 of 41 checks passed
lriggs pushed a commit that referenced this pull request Nov 12, 2026
…ibcall is available (#151) (#157)

* DX-128331: Gandiva: normalize Darwin host triple to macosx so exp10 libcall is available

* DX-128331: Gandiva: merge 'unchanged' Darwin triple normalization tests

* DX-128331: Gandiva: document why round(float64, int32) tests cover the exp10 lowering path

* DX-128331: Gandiva: move round(float64, scale) tests to decimal_test.cc

(cherry picked from commit 148cb48)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants