gpuResampSlc.cu: fix out of bounds reading bug in transformTile#214
Open
scottstanie wants to merge 1 commit intoisce-framework:developfrom
Open
gpuResampSlc.cu: fix out of bounds reading bug in transformTile#214scottstanie wants to merge 1 commit intoisce-framework:developfrom
gpuResampSlc.cu: fix out of bounds reading bug in transformTile#214scottstanie wants to merge 1 commit intoisce-framework:developfrom
Conversation
I was getting the following error sporadicall when running CUDA-versions
of geo2rdr and gpuResampleSlc
Reading in image data for tile 38 of 39
Interpolating tile 38 of 39
terminate called after throwing an instance of 'isce3::cuda::except::CudaError<cudaError>'
what(): Error in file .../cxx/isce3/cuda/core/gpuLUT1d.cu, line 77, function isce3::cuda::core::gpuLUT1d< <template-parameter-1-1>
>::~gpuLUT1d() [with T = double]: cudaError 700 (an illegal memory access was encountered)
zsh: IOT instruction (core dumped)
The CPU version in cxx/isce3/image/ResampSlc.cpp:322-327 has the correct bounds check:
if ((iRowResampled < chipHalf) ||
(iRowResampled >= (inLength - chipHalf)))
continue;
if ((iColResampled < chipHalf) || (
iColResampled >= (inWidth - chipHalf)))
continue;
However, the GPU version has only `>`.
The chip reading loop iterates iChipRow from 0 to chipSize - 1 (0 to 8), accessing row:
iTileRow = iRowResamp + iChipRow - chipHalf
The maximum iTileRow is iRowResamp + 8 - 4 = iRowResamp + 4.
With the current bounds check (iRowResamp + 4 > inReadableLength → skip), the kernel proceeds when iRowResamp = inReadableLength - 4. That gives:
max iTileRow = (inReadableLength - 4) + 4 = inReadableLength ← OUT OF BOUNDS
The valid row indices are 0 to inReadableLength - 1, so this reads one row past the end of the tile buffer.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
(note: have not been able to recompile this on the GPU machine to fully test. just makes logical sense now)
I was getting the following error sporadically when running CUDA versions of geo2rdr + gpuResampleSlc
I thought it was a "my image is too big problem", since shrinking the
linesPerBlockworked (sometimes). Other times, shrinking it didnt work, but just changing it did.The CPU version in cxx/isce3/image/ResampSlc.cpp:322-327 has the correct bounds check:
The maximum
iTileRowisiRowResamp + 8 - 4 = iRowResamp + 4. With the current bounds check (iRowResamp + 4 > inReadableLength, it will skip), the kernel proceeds wheniRowResamp = inReadableLength - 4. That gives:The valid row indices are 0 to
inReadableLength - 1, so this reads one row past the end of the tile buffer.