Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Overflow risk? #35

Open
ykempf opened this issue Jul 19, 2018 · 5 comments
Open

Overflow risk? #35

ykempf opened this issue Jul 19, 2018 · 5 comments

Comments

@ykempf
Copy link
Contributor

ykempf commented Jul 19, 2018

Correct me if I am wrong, but if the buffer size is set to the maximum of 2G, then this line will overflow and misbehave if the buffer is just below the limit and if bufferTop + size exceeds 2G, as all of these are int.
https://github.com/fmihpc/vlsv/blob/master/vlsv_writer.cpp#L788

Isn't it?

@markusbattarbee
Copy link
Contributor

Looks like it to me.

@sandroos
Copy link
Contributor

you are correct

@ursg
Copy link
Contributor

ursg commented Jul 30, 2018

I guess the easiest fix for this would be an explicit 64bit unsigned comparison in that spot:
42cb9fa

@ykempf
Copy link
Contributor Author

ykempf commented Jul 30, 2018

The remaining question is, are there more similar bugs lurking?

@sandroos
Copy link
Contributor

sandroos commented Aug 2, 2018

Dunno, this is the new code from frroberts. Looking at this code again I don't think it works at all if the buffer gets filled up. If bufferSize is exceeded then buffer needs to be flushed to disk before the new data can be added, but this is not done.

I have to edit my previous comment a little bit: technically the original code works, MPI can only write getMaxBytesPerRead() bytes in a single collective, and that is less than INT_MAX.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants