Skip to content

Conversation

@SeverinDiederichs
Copy link
Collaborator

@SeverinDiederichs SeverinDiederichs commented Nov 21, 2025

This PR cleans FillFromDeviceBuffer kernel:

When looping over the particles to be copied back, the index starts at nAlreadyTransfered and was going to nAlreadyTransferred + maxFromDeviceBuffer. Via an if statement, out of bounds access was prevented.
This PR proposes to directly fix the range correctly in the loop, enabling to remove the out of bounds check later. Thus, this is some minor cleaning PR

Found via code analysis by @dkonst13.

@SeverinDiederichs SeverinDiederichs added the bug Type: Something isn't working label Nov 21, 2025
@phsft-bot
Copy link

Can one of the admins verify this patch?

@SeverinDiederichs
Copy link
Collaborator Author

CI is failing because the shipped VecGeom misses some files like VecGeomConfigVersion.cmake. Under investigation

@JuanGonzalezCaminero
Copy link
Contributor

I think this is not an issue because the index is checked before accessing the buffer:
https://github.com/apt-sim/AdePT/blob/master/include/AdePT/core/AsyncAdePTTransport.cuh#L278

Otherwise it would be crashing

@SeverinDiederichs
Copy link
Collaborator Author

Indeed! As discussed offline, with the correct loop bounds, the if statement can then be removed. So this is rather a cleaning PR :)

@SeverinDiederichs SeverinDiederichs added cleaning improves code clarity, readability or other and removed bug Type: Something isn't working labels Nov 21, 2025
@SeverinDiederichs SeverinDiederichs changed the title [mini] fix out of bounds access in FillFromDeviceBuffer [mini] clean FillFromDeviceBuffer Nov 21, 2025
@SeverinDiederichs
Copy link
Collaborator Author

Failing just the known unstable reproducibility test in sync AdePT, therefore I am merging it now

@SeverinDiederichs SeverinDiederichs merged commit 8a5bcb7 into apt-sim:master Nov 22, 2025
2 of 3 checks passed
@SeverinDiederichs SeverinDiederichs deleted the fix_OOB_FillFromDeviceBuffer branch November 22, 2025 07:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cleaning improves code clarity, readability or other

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants