From 5711e3b5c738bc404f3e3101e63c9130c9026bcf Mon Sep 17 00:00:00 2001 From: "Gabriel A. Devenyi" Date: Fri, 14 Aug 2026 23:31:58 -0400 Subject: [PATCH 1/2] BUG: Do fslio's size and offset arithmetic at 64-bit width A byte count or file offset is declared size_t, but the expression assigned to it multiplies ints, so the arithmetic happens at 32 bits and only the result is widened: size_t FslGetVolSize(...) { return (nx * ny * nz); } /* int product */ offset = ((ydim * zVox + yVox) * xdim + xVox) * wordsize; For a volume over 2GB the product overflows before it is ever assigned to the size_t, so the destination type buys nothing. The signed overflow is undefined, so whether the wide value survives is up to the optimizer: clang folds it to the correct 64-bit result at -O2 and yields 0 at -O0. Affects FslGetVolSize(), FslWriteVolumes(), FslReadSliceSeries(), FslReadRowSeries() and FslReadTimeSeries(). Each operand is now widened before the multiply rather than after. The slbytes and volbytes assignments in FslReadSliceSeries() and FslReadTimeSeries() are left alone because another open pull request changes those exact lines. --- fsliolib/fslio.c | 10 +++++---- fsliolib/fslio_volsize_test.c | 42 +++++++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 4 deletions(-) create mode 100644 fsliolib/fslio_volsize_test.c diff --git a/fsliolib/fslio.c b/fsliolib/fslio.c index 44053fcf..6477d84e 100644 --- a/fsliolib/fslio.c +++ b/fsliolib/fslio.c @@ -918,7 +918,7 @@ size_t FslWriteVolumes(FSLIO *fslio, const void *buffer, size_t nvols) size_t bytes_written; size_t nbytes; long int bpv = fslio->niftiptr->nbyper; /* bytes per voxel */ - nbytes = nvols * FslGetVolSize(fslio) * bpv; + nbytes = nvols * FslGetVolSize(fslio) * (size_t)bpv; if ( (FslBaseFileType(FslGetFileType(fslio))==FSL_TYPE_ANALYZE) && (FslGetLeftRightOrder(fslio)==FSL_NEUROLOGICAL) ) { @@ -1076,7 +1076,7 @@ size_t FslReadRowSeries(FSLIO *fslio, void *buffer, short row, short slice, size if ((slice<0) || (slice>=z)) FSLIOERR("FslReadRowSeries: slice outside valid range"); if ((row<0) || (row>=y)) FSLIOERR("FslReadRowSeries: row outside valid range"); - rowbytes = x * (FslGetDataType(fslio, &type)) / 8; + rowbytes = (size_t)x * (size_t)(FslGetDataType(fslio, &type)) / 8; slbytes = rowbytes * y; volbytes = slbytes * z; @@ -1142,7 +1142,8 @@ size_t FslReadTimeSeries(FSLIO *fslio, void *buffer, short xVox, short yVox, sho volbytes = (size_t)xdim * (size_t)ydim * (size_t)zdim * wordsize; orig_offset = (size_t)znztell(fslio->fileptr); - offset = ((ydim * zVox + yVox) * xdim + xVox) * wordsize; + offset = (((size_t)ydim * (size_t)zVox + (size_t)yVox) * (size_t)xdim + + (size_t)xVox) * (size_t)wordsize; znzseek(fslio->fileptr,(znz_off_t)offset,SEEK_CUR); for (n=0; nniftiptr!=NULL) { - return (fslio->niftiptr->nx * fslio->niftiptr->ny * fslio->niftiptr->nz); + return (size_t)fslio->niftiptr->nx * (size_t)fslio->niftiptr->ny * + (size_t)fslio->niftiptr->nz; } if (fslio->mincptr!=NULL) { fprintf(stderr,"Warning:: Minc is not yet supported\n"); diff --git a/fsliolib/fslio_volsize_test.c b/fsliolib/fslio_volsize_test.c new file mode 100644 index 00000000..9d25385b --- /dev/null +++ b/fsliolib/fslio_volsize_test.c @@ -0,0 +1,42 @@ +/* FslGetVolSize() must evaluate nx*ny*nz at size_t width, not int width. */ + +#include +#include +#include "fslio.h" + +static int check(const char *what, size_t got, size_t want) +{ + if( got == want ) return 0; + fprintf(stderr, "FAILURE: %s: got %llu, expected %llu\n", what, + (unsigned long long)got, (unsigned long long)want); + return 1; +} + +int main(void) +{ + FSLIO fslio; + nifti_image nim; + int failures = 0; + + memset(&fslio, 0, sizeof(fslio)); + memset(&nim, 0, sizeof(nim)); + fslio.niftiptr = &nim; + fslio.mincptr = NULL; + + nim.nx = 64; nim.ny = 64; nim.nz = 32; + failures += check("64x64x32", FslGetVolSize(&fslio), (size_t)64*64*32); + + /* 2048*2048*1024 == 2^32: zero when the product is formed in int. */ + nim.nx = 2048; nim.ny = 2048; nim.nz = 1024; + failures += check("2048x2048x1024", FslGetVolSize(&fslio), + (size_t)2048*2048*1024); + + /* 3000*3000*300 == 2,700,000,000: negative when formed in int. */ + nim.nx = 3000; nim.ny = 3000; nim.nz = 300; + failures += check("3000x3000x300", FslGetVolSize(&fslio), + (size_t)3000*3000*300); + + if( failures ) return 1; + printf("FslGetVolSize 64-bit arithmetic test passed.\n"); + return 0; +} From 6ba8f4eba32bc39d856225970d6cadd08d80248a Mon Sep 17 00:00:00 2001 From: "Gabriel A. Devenyi" Date: Tue, 22 Sep 2026 08:36:34 -0500 Subject: [PATCH 2/2] ENH: Test that FslGetVolSize forms its product at 64-bit width The test drives FslGetVolSize() with dimensions whose product exceeds INT_MAX and asserts the exact 64-bit value, plus a small volume so an over-broad change cannot pass it. It fails in a -O0 build without the widening fix and passes with it; at -O2 clang folds the signed overflow to the wide result, so only the unoptimized build observes it. --- fsliolib/CMakeLists.txt | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/fsliolib/CMakeLists.txt b/fsliolib/CMakeLists.txt index 7ef4b63d..84bb69c5 100644 --- a/fsliolib/CMakeLists.txt +++ b/fsliolib/CMakeLists.txt @@ -16,3 +16,12 @@ if(BUILD_SHARED_LIBS) target_compile_definitions(${NIFTI_FSLIOLIB_NAME} PRIVATE NIFTI_FSL_BUILD_SHARED) target_compile_definitions(${NIFTI_FSLIOLIB_NAME} INTERFACE NIFTI_FSL_USE_SHARED) endif() + +if(NIFTI_BUILD_TESTING) + add_executable(${NIFTI_PACKAGE_PREFIX}fslio_volsize_test fslio_volsize_test.c) + target_link_libraries(${NIFTI_PACKAGE_PREFIX}fslio_volsize_test PUBLIC ${NIFTI_FSLIOLIB_NAME}) + add_test( + NAME fslio_volsize_64bit + COMMAND $ + ) +endif()