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

Fix issue with C++ endf float converter #7

Merged
merged 3 commits into from
Apr 18, 2024

Conversation

aidancrilly
Copy link
Contributor

@aidancrilly aidancrilly commented Apr 17, 2024

This closes #6 - at least for me, I noticed someone having similar issues in #4.

The crash seems to be a seg fault that occurs when the endf float being converted is negative, i.e. has a leading minus sign.

Positive numbers have leading whitespace so hit the continue statement and j is not incremented. Therefore, the first digit is placed at arr[0]. However, negative numbers had the leading minus sign placed at arr[0] and first digit at arr[1]. This means when the null terminator is placed at the end, for negative numbers this is at arr[12] so the crash occurs.

EDIT: my initial commit was not the correct solution, I think it is simply a case of increasing arr to be length 13 so it has space for both the e and null terminator if the input buffer is length 11. Apologies, some how my initial incorrect fix ran (I have no idea how!) but on restarting it failed.

This would seem to me to be an issue across systems.

@paulromano
Copy link
Owner

Thanks @aidancrilly! Can you give a specific example of an 11-character string that triggers the problem?

@aidancrilly
Copy link
Contributor Author

@paulromano
Sure, in ENDF/B-VIII.0 incident neutron data for deuterium there is -2.225002+6 at the beginning of the MF = 3, MT = 16 record. This is the QM/QI value.

Copy link
Owner

@paulromano paulromano left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks again! Just one small request:

src/endf/_records.cpp Outdated Show resolved Hide resolved
@paulromano paulromano merged commit 24075e2 into paulromano:main Apr 18, 2024
4 checks passed
@paulromano
Copy link
Owner

@aidancrilly Would you be up for making a similar PR to OpenMC? The cfloat_endf function was originally borrowed from there:
https://github.com/openmc-dev/openmc/blob/develop/openmc/data/endf.c

It would be good to also add the example you mentioned (-2.225002+6) to the tests here:
https://github.com/openmc-dev/openmc/blob/develop/tests/unit_tests/test_endf.py

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

Successfully merging this pull request may close these issues.

Silent crash in float_endf
2 participants