[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[netCDF #SWH-176490]: stack overflow bug + fix

> 
> 
> 
> 
> 
> 
> Dear sir, madam,
> 
> I have similar problems in all nf90_get*(Four|Eight)ByteInt
> routines. Instead of reading directly to the values array, you declare
> a defaultIntArray with full dimensions.
> This gets pushed onto the stack, resulting in stack overflows. At the
> end of the routine, a reshape is called.
> 
> For all OneByteInt and Real routines, no additional array nor reshaping
> is used.
> Could we get rid of this for the four and eight byte ints as well?
> 
> Best regards,
> Arthur van Dam
> 
> On 2009-10-22 16:38, Unidata netCDF Support wrote:
> 
> Arthur van Dam,
> 

Howdy Arthur!

We have been looking at your fortran 90 API issue.

I see what you are talking about, in the data read routines the array that is 
read is copied. This came as a surprise to me:

 function nf90_get_var_1D_EightByteInt(ncid, varid, values, start, count, 
stride, map)
     integer,                         intent( in) :: ncid, varid
     integer (kind = EightByteInt), dimension(:), &
                                      intent(out) :: values
     integer, dimension(:), optional, intent( in) :: start, count, stride, map
     integer                                      :: 
nf90_get_var_1D_EightByteInt

     integer, dimension(nf90_max_var_dims) :: localStart, localCount, 
localStride, localMap
     integer                               :: numDims, counter
     integer, dimension(size(values))      :: defaultIntArray

     ! Set local arguments to default values                                    
                                                                   
     numDims                 = size(shape(values))
     localStart (:         ) = 1
     localCount (:numDims  ) = shape(values)
     localCount (numDims+1:) = 1
     localStride(:         ) = 1
     localMap   (:numDims  ) = (/ 1, (product(localCount(:counter)), counter = 
1, numDims - 1) /)

     if(present(start))  localStart (:size(start) )  = start(:)
     if(present(count))  localCount (:size(count) )  = count(:)
     if(present(stride)) localStride(:size(stride)) = stride(:)
     if(present(map))  then
       localMap   (:size(map))    = map(:)
       nf90_get_var_1D_EightByteInt = &
          nf_get_varm_int(ncid, varid, localStart, localCount, localStride, 
localMap, defaultIntArray)
     else if(present(stride)) then
       nf90_get_var_1D_EightByteInt = &
          nf_get_vars_int(ncid, varid, localStart, localCount, localStride, 
defaultIntArray)
     else
       nf90_get_var_1D_EightByteInt = &
          nf_get_vara_int(ncid, varid, localStart, localCount, defaultIntArray)
     end if
     values(:) = reshape(defaultIntArray(:), shape(values))
   end function nf90_get_var_1D_EightByteInt

So in this code the array that the user passes in, values, does not get used 
when reading from the file! Instead, a locally defined defaultIntArray is used.

As you note, this does not happen with other types, like the OneByteInt.

In the case of eight-byte ints this is necessary because the data in the file 
are 4-byte ints, and the copy of the array is necessary to get each int to its 
correct place after the read. What surprises me is that this memory is not 
allocated and then freed.

In the case of 4-byte ints, it's harder to see why this is necessary, but it 
does take account of the fact that (in olden times) some supercomputers used to 
use 8-byte integers. (I believe the Cray did this.) In which case, the copy of 
the array is needed to get the data into the fourbyteint memory layout.

This all goes back to the C API, in the sense that the fortran INTEGER is going 
to be the same size as the C integer type. 

I don't know if we still need to do that - can we just assume now that all C 
compilers, even those on supercomputers, use a four-byte int, and support the C 
"long long" type for 64-bit ints? Certainly I think if the F90 API were being 
written today, that would be a reasonable assumption.

You say you have modified the code to not do the copy for the FourByteInt, and 
that it works. That should be fine on your platform.

Thanks,

Ed

Ticket Details
===================
Ticket ID: SWH-176490
Department: Support netCDF
Priority: Critical
Status: Closed