Fix for potential integer under/overflow issue with reading very large files

[briandobbins]

Fix overflow/underflow issues during qsort for the SUBSET
rearranger.

The comparator function given to the qsort call in the rearranger
returns a 32-bit integer, but the operands can be 64-bit. Depending
on the difference in value, you can get an overflow or underflow,
which means you read an incorrect amount of data.

[dqwu]

@briandobbins
compare_offsets is passed to qsort in the subset rearranger:

int subset_rearrange_create(iosystem_desc_t *ios, int maplen, PIO_Offset *compmap,
                            const int *gdimlen, int ndims, io_desc_t *iodesc)
{
    ...
    /* sort the mapping, this will transpose the data into IO order */
    qsort(map, iodesc->llen, sizeof(mapsort), compare_offsets);
    ...
}

E3SM/SCREAM uses the box rearranger by default, which might explain why you did not reproduce this issue on the ne1024/128-level grids.

To test the subset rearranger in E3SM/SCREAM, you need to explicitly change PIO_REARRANGER (default value is 1) to 2:
./xmlchange PIO_REARRANGER=2

[jayeshkrishna]

Original PR text

Hi all,

This is a fix we recently made to PIO which addresses a bug when using 64-bit NetCDF mode which can result in reading an incorrect amount of data on very large grids. Basically, the comparator function given to the qsort call in the rearranger returns a 32-bit integer, but the operands can be 64-bit. Depending on the difference in value, you can get an overflow or underflow, which means you read an incorrect amount of data.

For us this only occurs in our 42M column CAM-MPAS runs with our 58-level config or higher -- that puts us at 2.43B cells. I figure SCREAM must be reading data differently (by column maybe?) since otherwise I'd have thought it'd hit there, too, on the ne1024 / 128-level grids, but evidently not! If you have insight there, I'm all ears.

Technically, this does add a few instructions to the function, but it hasn't had a measurable impact on performance on the whole.

Cheers,

Brian

[jayeshkrishna]

@dqwu : Can you also add a simple C unit test for this case (testing qsort with simple arrays containing large PIO offset values)?

[dqwu]

@dqwu : Can you also add a simple C unit test for this case (testing qsort with simple arrays containing large PIO offset values)?

Yes, we can create another PR to add a simple C unit test that reproduces the integer underflow/overflow issue, which is expected to fail without the patch in this PR.

[dqwu]

The potential integer under/overflow issue is now reproducible with an updated C unit test in PR #632 (to be fixed by this PR).