fix: Avoid implicit 64-to-32-bit conversions on 32-bit platforms by ump45nose · Pull Request #983 · apache/arrow-nanoarrow

ump45nose · 2026-10-21T21:00:33Z

Closes #797.

Building nanoarrow with -Wconversion on a 32-bit target (the armv7 Android build in the vcpkg CI from the issue) fails with -Wshorten-64-to-32 errors, because a number of int64_t sizes are passed straight to functions that take size_t: memcpy/memset/memmove, snprintf, malloc/realloc, fread/fwrite, strncmp, and the flatcc builder/vector helpers.

This adds explicit (size_t) casts at those call sites in common/, device/device.c and ipc/. There's no behaviour change on 64-bit platforms. In ArrowArrayViewCompareBuffer() the size_bytes > 0 check stays on the signed value and only the length handed to memcmp() is converted, so the stringified comparison message the tests check doesn't change.

Testing

clang -m32 -std=gnu99 -fsyntax-only -Wall -Wextra -Wpedantic -Wconversion -Wno-sign-conversion (clang 19, i386) over src/nanoarrow/common/*.c, device/device.c and ipc/{codecs,decoder,encoder,reader,writer}.c: 40 distinct -Wshorten-64-to-32 warning locations on main, none with this change. I don't have the Android NDK setup locally, so this is a 32-bit x86 stand-in for the armv7 build.
Native 64-bit Debug build with NANOARROW_BUILD_TESTS=ON NANOARROW_IPC=ON NANOARROW_DEVICE=ON: ctest 422/422 passed.
clang-format 16.0.6 (the pre-commit version) applied.

This PR was prepared with AI assistance; I reviewed the changes and ran the checks above.

Building with -Wconversion on 32-bit targets (e.g. armv7 Android in the vcpkg CI) fails because several int64_t values are passed to functions that take size_t (memcpy, memset, snprintf, malloc, fread and flatcc builder helpers). Cast these explicitly. Closes apache#797.

codecov-commenter · 2026-10-22T14:05:39Z

Codecov Report

❌ Patch coverage is 73.52941% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 78.58%. Comparing base (efe4274) to head (487d8a9).
⚠️ Report is 2 commits behind head on main.

Files with missing lines	Patch %	Lines
src/nanoarrow/ipc/encoder.c	45.45%	0 Missing and 6 partials ⚠️
src/nanoarrow/ipc/reader.c	57.14%	2 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #983      +/-   ##
==========================================
- Coverage   78.59%   78.58%   -0.02%     
==========================================
  Files         106      106              
  Lines       17153    17173      +20     
  Branches     2043     2045       +2     
==========================================
+ Hits        13482    13495      +13     
- Misses       2422     2427       +5     
- Partials     1249     1251       +2

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

paleolimbot

Thank you! It has always bothered me that this emits warnings with -Wconversion.

I know we don't do this everywhere, but we should check these, at least in debug builds. Perhaps you can add something like

static inline size_t _ArrowInt64ToSizeT(int64_t x) {
  NANOARROW_DCHECK(x < (int64_t)SIZE_T_MAX);
  return (size_t)x
}

...to types_inline.h and wrap these conversions. I believe all of these are safe as DCHECKs (the only place we would want to check and return an error is where the input could come from a user input, either via a high level binding or an IPC file). I believe most of these would surface as allocation failures before the cast.

paleolimbot · 2026-10-22T14:13:53Z

+void* ArrowMalloc(int64_t size) { return malloc((size_t)size); }

-void* ArrowRealloc(void* ptr, int64_t size) { return realloc(ptr, size); }
+void* ArrowRealloc(void* ptr, int64_t size) { return realloc(ptr, (size_t)size); }


I think these conversions should be checked (and return null on failure), cleaning up the pointer in the realloc case

paleolimbot · 2026-10-22T14:18:21Z

+    NANOARROW_ASSERT_OK(
+        ArrowIpcEncoderWriteContinuationAndSize(out, (size_t)padded_size));


We should check the padded size here (or update ArrowIpcEncoderWriteContinuationAndSize() to accept an int64 and check the failure code)

paleolimbot · 2026-10-22T14:26:22Z

+    memcpy(buf, private_data->input.data + private_data->cursor_bytes,
+           (size_t)bytes_to_read);


I think this can be a NANOARROW_DCHECK that bytes_to_read is small enough (there shouldn't have been a single in memory buffer bigger than 4 GB in the process anyway)

paleolimbot · 2026-10-22T14:28:37Z

+  int64_t bytes_read =
+      (int64_t)fread(buf, 1, (size_t)buf_size_bytes, private_data->file_ptr);


I think here we can also NANOARROW_DCHECK (because the output can't have been allocated to a size bigger than 4 GB in a 32 bit process)

paleolimbot · 2026-10-22T14:31:46Z

+  int64_t bytes_written =
+      (int64_t)fwrite(buf, 1, (size_t)buf_size_bytes, private_data->file_ptr);


I think a DCHECK would be good here too

…here input-derived Add _ArrowInt64FitsInSizeT()/_ArrowInt64ToSizeT() to inline_types.h and use them for the conversions touched in this PR. ArrowMalloc()/ArrowRealloc() return NULL (freeing the pointer in the realloc case) for sizes that do not fit in size_t, the IPC encoder checks the padded message size instead of asserting, and the stream reader returns EINVAL if the header size is out of range.

ump45nose · 2026-10-22T15:25:03Z

Thanks @paleolimbot. 487d8a9 adds _ArrowInt64FitsInSizeT() and _ArrowInt64ToSizeT() to inline_types.h (the second one NANOARROW_DCHECKs that the value is non-negative and fits in size_t). The conversions in this PR now go through it, so the IPC reader/writer memcpy/fread/fwrite sizes are DCHECKs as you suggested.

The places you asked to check for real:

ArrowMalloc()/ArrowRealloc() return NULL if the size doesn't fit in size_t, and ArrowRealloc() frees ptr in that case.
ArrowIpcEncoderWriteContinuationAndSize() takes an int64_t and checks 0 <= size <= INT32_MAX. ArrowIpcEncoderFinalizeBuffer() now returns its error instead of NANOARROW_ASSERT_OK.
The legacy-prefix memmove in ArrowIpcArrayStreamReaderNextHeader() returns EINVAL with an error message if the header size is out of range.

ArrowErrorSetString() keeps a plain cast because it comes before NANOARROW_DCHECK is defined, and src_len is already bounded by sizeof(error->message) there.

I ran a Debug build (NANOARROW_DEBUG) with IPC and device enabled, and all 422 ctest tests pass. I couldn't do a real 32-bit build here. The failing meson-build (-Warray-bounds in utils_test.cc) and clang-tidy jobs fail the same way on main, and test-python (3.9) fails because the runner can't find Python 3.9 on Ubuntu 26.04. (AI-assisted reply.)

paleolimbot · 2026-10-23T13:45:56Z

Thank you!

It looks like CI has a few complaints, but once those are resolved I think this is good to go.

ump45nose · 2026-10-23T15:21:39Z

Thanks @paleolimbot! I looked at the two failing jobs:

clang-tidy: all eight warnings are in src/nanoarrow/integration/ipc_integration.cc and src/nanoarrow/hpp/view_test.cc (clang-analyzer-unix.Stream / clang-analyzer-security.ArrayBound), neither of which this PR touches. clang-tidy passed on the last main run (2026-10-22), so this looks like a newer analyzer in the runner image rather than something from this diff; happy to fix those warnings in a separate PR if you would like.
meson-build: the same job also fails on the latest main run of test-c, so it is pre-existing.

Let me know if you see anything in the diff itself that needs changing. (AI-assisted reply.)

paleolimbot · 2026-10-24T03:05:52Z

Thanks! I hadn't noticed, but I've put up a PR to fix that and will sort through it this weekend. Thank you!

paleolimbot reviewed Oct 22, 2026

View reviewed changes

paleolimbot merged commit 41c78c4 into apache:main Oct 24, 2026
40 of 42 checks passed

Navigation Menu

Repository navigation

fix: Avoid implicit 64-to-32-bit conversions on 32-bit platforms - #983

fix: Avoid implicit 64-to-32-bit conversions on 32-bit platforms#983
paleolimbot merged 2 commits into
apache:mainfrom
ump45nose:fix/797-shorten-64-to-32

ump45nose commented Oct 21, 2026

Uh oh!

codecov-commenter commented Oct 22, 2026 •

edited

Loading

Uh oh!

paleolimbot left a comment

Uh oh!

paleolimbot Oct 22, 2026

Uh oh!

paleolimbot Oct 22, 2026

Uh oh!

paleolimbot Oct 22, 2026

Uh oh!

paleolimbot Oct 22, 2026

Uh oh!

paleolimbot Oct 22, 2026

Uh oh!

Uh oh!

ump45nose commented Oct 22, 2026

Uh oh!

paleolimbot commented Oct 23, 2026

Uh oh!

ump45nose commented Oct 23, 2026

Uh oh!

paleolimbot commented Oct 24, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		NANOARROW_ASSERT_OK(
		ArrowIpcEncoderWriteContinuationAndSize(out, (size_t)padded_size));

		memcpy(buf, private_data->input.data + private_data->cursor_bytes,
		(size_t)bytes_to_read);

		int64_t bytes_read =
		(int64_t)fread(buf, 1, (size_t)buf_size_bytes, private_data->file_ptr);

		int64_t bytes_written =
		(int64_t)fwrite(buf, 1, (size_t)buf_size_bytes, private_data->file_ptr);

Conversation

ump45nose commented Oct 21, 2026

Testing

Uh oh!

codecov-commenter commented Oct 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

paleolimbot left a comment

Choose a reason for hiding this comment

Uh oh!

paleolimbot Oct 22, 2026

Choose a reason for hiding this comment

Uh oh!

paleolimbot Oct 22, 2026

Choose a reason for hiding this comment

Uh oh!

paleolimbot Oct 22, 2026

Choose a reason for hiding this comment

Uh oh!

paleolimbot Oct 22, 2026

Choose a reason for hiding this comment

Uh oh!

paleolimbot Oct 22, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ump45nose commented Oct 22, 2026

Uh oh!

paleolimbot commented Oct 23, 2026

Uh oh!

ump45nose commented Oct 23, 2026

Uh oh!

paleolimbot commented Oct 24, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codecov-commenter commented Oct 22, 2026 •

edited

Loading