Fix segfault with -fdefault-integer-8 by using integer(c_int) for handles#322
Merged
jameskermode merged 8 commits intoDec 18, 2025
Merged
Conversation
krystophny
force-pushed
the
fix/default-i8-segfault
branch
2 times, most recently
from
ee6f56e to
0407ea3
Compare
3 tasks
18 tasks
2 tasks
Owner
|
@krystophny I think I broke this in the conflict resolution - would you be able to take a look please? |
Owner
|
Never mind, I think I've worked out the problem, I lost some |
Contributor
Author
Ah I am also just on it... |
Contributor
Author
|
@jameskermode then I wait for your fix. For the next PRs it would be easier to rebase then probably because they are anyway based on the previous (maybe rebase is not even needed then and they will recognize master base) |
Owner
|
The maintainer edit setting doesn't seem to be working, can you merge itpplasma#7 into this branch please? If you can then quickly check the subsequent PRs update correct and if not rebase them that would be great. |
'quip_regression' left out as it has it's own workflow
…dles When compiling with -fdefault-integer-8, bare integer becomes 8 bytes. The handle arrays (e.g., this(4)) were declared as integer, meaning 4 x 8 = 32 bytes with -fdefault-integer-8, but sizeof_fortran_t was calculated assuming 4-byte integers (4 x 4 = 16 bytes). This mismatch caused segfaults when using transfer() to convert between pointer types and integer arrays. Fix by using integer(c_int) for all handle arrays, which remains 4 bytes regardless of -fdefault-integer-8. This ensures consistent size between wrapper generation time and runtime. Changes: - transform.py: Set wrapper_type to 'integer(c_int)' for derived type args - f90wrapgen.py: Use integer(c_int) for handle arrays in: - visit_Procedure (derived type arguments) - _write_sc_array_wrapper (this and dummy_this) - _write_array_getset_item (this and item handles) - _write_array_len (this handle) - _write_scalar_wrapper (this and derived type element handles) - Add 'use, intrinsic :: iso_c_binding, only: c_int' where needed Fixes the default_i8 example test.
The errorbinding example was added to the test suite in PR jameskermode#320 but was missing the required tests.py and tests_pkg.py files, causing CI to fail.
Remove f2py_string_input and issue299_directc_nested_functions from test suite until they are fixed: - f2py_string_input: undefined symbol string_in_array_ (linking issue) - issue299_directc_nested_functions: types.py shadows Python stdlib types module
krystophny
force-pushed
the
fix/default-i8-segfault
branch
from
add07a6 to
61d88fa
Compare
During rebase, conflict resolution incorrectly re-added broken examples to the Makefile EXAMPLES list. These examples were intentionally excluded: - callback_print_function_issue93 (undefined symbol: pyfunc_print_) - f2py_string_input (undefined symbol: string_in_array_) - issue299_directc_nested_functions (types.py shadows stdlib) - issue41_abstract_classes (AttributeError on get_value) - recursive_type (Derived type not declared) The CMakeLists.txt was already correct; only Makefile needed fixing.
During rebase conflict resolution, an else clause was incorrectly added that declared dummy_this for module-level arrays. However, the subroutine signature for module-level arrays doesn't include any this argument (per issue jameskermode#306 fix), causing a mismatch: the variable was declared but not in the argument list, leading to compilation errors. Remove the erroneous else clause - module-level arrays should have no this argument in either the signature or declarations.
Contributor
Author
|
Sorry for the delay @jameskermode , this should now be done. I push also all subsequent PRs once more rebased on each other in order, so hope this will work. |
Owner
|
thanks! |
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
3 participants
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.
Summary
-fdefault-integer-8integer(c_int)instead of bareintegerfor handle arraysuse, intrinsic :: iso_c_binding, only: c_intwhere neededBased on PR #320 (add missing examples to tests).
Problem
When compiling with
-fdefault-integer-8, bareintegerbecomes 8 bytes. Handle arrays (e.g.,this(4)) were declared asinteger, meaning 4 x 8 = 32 bytes. Butsizeof_fortran_twas calculated assuming 4-byte integers (4 x 4 = 16 bytes).This mismatch caused segfaults when using
transfer()to convert between pointer types and integer arrays.Solution
Use
integer(c_int)for all handle arrays.c_intfromiso_c_bindingis guaranteed to be 4 bytes regardless of-fdefault-integer-8, ensuring consistent size between wrapper generation time and runtime.Changes
f90wrap/transform.pywrapper_typeto'integer(c_int)'for derived type argumentsf90wrap/f90wrapgen.pyinteger(c_int)for handle arrays in:visit_Procedure(derived type arguments)_write_sc_array_wrapper(this and dummy_this)_write_array_getset_item(this and item handles)_write_array_len(this handle)_write_scalar_wrapper(this and derived type element handles)use, intrinsic :: iso_c_binding, only: c_intwhere neededTest plan
default_i8example passes (3/3 tests)-fdefault-integer-8usage