A user shared with me an OpenMC model based on the ITER C-Model that had been converted from MCNP using csg2csg. The conversion strategy from csg2csg doesn't result in the same exact expressions that we get from openmc_mcnp_adapter, namely with respect to where parentheses are inserted in region expressions. When we construct a Region object on the C++ side, parentheses may be inserted to enforce precedence of logical operators and it turns out that this particular model revealed a bug in our method for adding parentheses. Namely, a vector is used to maintain the region expression and parentheses are inserted (using vector<T>::insert) at various points, which may invalidate iterators established previously if the insert operation causes a reallocation of the underlying memory allocation. When you try to run the model with OpenMC, segfaults can then result during initialization from dereferencing an iterator that was invalidated.
One important point is that as far as I can tell, this bug does not affect models generated using OpenMC's Python API because the API generates expressions with more parentheses at the time it is writing XML files.
To fix the bug, I've modified the Region::add_parentheses and related methods to keep track of an index within the expression_ vector rather than relying on iterators (that may be invalidated). I've confirmed this resolves the bug with the converted C-Model, avoiding the segfaults that were experienced before.
@myerspat Would you be willing to review this?
Checklist
[x] I have performed a self-review of my own code
[x] I have run clang-format (version 15) on any C++ source files (if applicable)
[x] I have followed the style guidelines for Python source files (if applicable)
[x] I have made corresponding changes to the documentation (if applicable)
[ ] I have added tests that prove my fix is effective or that my feature works (if applicable)
Description
A user shared with me an OpenMC model based on the ITER C-Model that had been converted from MCNP using csg2csg. The conversion strategy from csg2csg doesn't result in the same exact expressions that we get from openmc_mcnp_adapter, namely with respect to where parentheses are inserted in region expressions. When we construct a
Regionobject on the C++ side, parentheses may be inserted to enforce precedence of logical operators and it turns out that this particular model revealed a bug in our method for adding parentheses. Namely, a vector is used to maintain the region expression and parentheses are inserted (usingvector<T>::insert) at various points, which may invalidate iterators established previously if theinsertoperation causes a reallocation of the underlying memory allocation. When you try to run the model with OpenMC, segfaults can then result during initialization from dereferencing an iterator that was invalidated.One important point is that as far as I can tell, this bug does not affect models generated using OpenMC's Python API because the API generates expressions with more parentheses at the time it is writing XML files.
To fix the bug, I've modified the
Region::add_parenthesesand related methods to keep track of an index within theexpression_vector rather than relying on iterators (that may be invalidated). I've confirmed this resolves the bug with the converted C-Model, avoiding the segfaults that were experienced before.@myerspat Would you be willing to review this?
Checklist
I have followed the style guidelines for Python source files (if applicable)I have made corresponding changes to the documentation (if applicable)