Fix memory management issues in assignment#434
Merged
Conversation
A runnable example derived from the paper by Rousonm, Morris and Xia.
4.0.2 to 4.2.0 Update Makefile for changes to regression/run/defaults.mk
Need to call dtor explicitly to release memory. FINAL clause is not hooked up.
Mostly removed in d6abf1d This code is only used with options.F_auto_reference_count which is not documented. This change avoids an error about F_derived_ptr not existing so I can move on to making auto_reference_count work.
Used to understand what swig-fortran is doing with shared pointers.
Allow intent(operator) in statement names. Start work to allow different forms of assignment overload to allow the swig-fortran system to be used. Use option.F_assignment_api to control how to define assignment overload.
This will allow it to be used to construct the name of a helper or other similar uses.
New name is f_helper_capsule_data_basic. The trailing 'helper' was redundant. The new suffix is from option.F_assignment_api. Changes comments in the generated source and JSON files. Replace value 'none' with 'basic' for option.F_assignment_api. It's used with the capsule data type and not just assignment api.
Add the statement name to the message.
Add options.F_capsule_variants to list capsule variants. Have a format template for each variant. These names are used in the helpers which are evaluated outside the context of a class. Set fmt.F_capsule_data_type from one of the variants based on F_assignment_api. This allows fortran statements to be used with any variant. Remove fmt.option_F_assignment_api. Unused in the current scheme.
Similar to F_capsule_data_type.
Continued from cb68687 This allows some C helpers from h_helper_capsule_data_swig to be add C code. Before only the Fortran code was being added.
Easier to grep for fcn_helpers.
It was mixed between upper and lower. This helpers shared.yaml properly set capsue_data_type when using option.F_assignment_api=swig. Otherwise, only Object used swig and not std::shard and std::weak_ptr.
Different compilers require different code. gcc and ifx generate the same output. Add using statement to shared/maincpp.cpp. This is similar to the generated Fortran derived types.
Add assignment operators for each type (shared and weak) This now runs the same on ibm xlf and cray ftn as it does with gfortran.
Make the rhs argument a type instead of a class to avoid ambiguous overload. shared/testmod.f - Move routines to interface assignment(=) block instead of in type contains section. Do not add PROCEDURE statements in the type since the routines are never called directly.
Used to overload assignment operators instead of adding as type bound procedures. Rename FStmts.f_operator as f_operator_body.
The assignment overloads will be associated with the Library and not each individual class. This would also be necessary for shared_ptr to a POD. Write out in todict.py.
Update reference for new test error-fmt.
Add methods for smart pointers after all of the subclasses are created. This will allow one subclass to reference another. ex. shared_ptr and weak_ptr.
It is a list of AssignOperator instances. To be used to create assignment functions.
Instead of creating an additional class method
(GenFunctions.add_weak_smart_methods), create assignment overload
function with arguments (lhs, rhs). The code is controlled by
statements in fc-statements. A single interface assignment(=) block
is created instead of adding as a type bound procedure. This may help
with things like std::shared_ptr<int> instead of requiring class for
the template argument. Also similar to how operator(.eq.) and
operator(.ne.) are implemented.
interface assignment (=)
module procedure object_weak_assign_Object_shared
end interface
fmt.idtor is not yet being set so memory will leak.
BindArg contains fmtdict and meta. This will allow bind to be used with find_idtor.
From wrapc.py. Used to create destructors for C++. This allows find_idtor to be called from fcfmt.fmt_assignment. Keeps destructor code in a single file.
This properly sets the idtor field for shared pointers. Note that the shared pointer case is first while it was last before due to the order of calling find_idtor changing.
This will allow the mixins to be used with other specalized groups. Only a Fortran statement group is needed which also contains C code. Worry about calling assignments from C later.
Do not create overloads where options.wrap_fortran is false. Dump assign_operators into the JSON for NamespaceNodes resulting in JSON file changes.
Only the LibraryNode were being used, but NamespaceNode.assign_operators is now used.
Caught with preprocess.yaml test.
It was not compiled before. It now has assignment overload code which is needed. Several other tests have already had similar fixes. Using F_assignment_api=swig now works for all tests.
Remove _swig from group names in fc-statements.json.
In directory regression/other/swig-fortran add ownership (from ownership.yaml) and manual (example from the swig 4.2 manual to save the output).
Prepare to use option.default_owner. The default will be different for function and arguments since it is based on intent.
Still compute the shared owner, it is used by Python. But allow a place to compute owner for Fortran since owner must be computed after inout.
Check conditions in set_func_owner_fortran and use option.owner_default if appropriate. To be used to set capsule.cmemflags bit for SWIG_MEM_OWN. By setting the owner metaattribute, the JSON files will change. This also changes the statement name which changes comments in the generated code. pointer_owner is now the standard. Add a default destructor for language=c. Used with struct-c to delete struct-as-class constructor.
Check conditions in set_arg_owner_fortran and use option.owner_default if appropriate. To be used to set capsule.cmemflags bit for SWIG_MEM_OWN. By setting the owner metaattribute, the JSON files will change. This also changes the statement name which changes comments in the generated code. The statement name pointer_owner is now the standard. i.e. A pointer must also contain ownership attribute.
Default fmt fields as owner=library. If owner=call, then set to include SWIG_MEM_OWN bit.
The statment group is allocating some local memory which must be destroyed. The user never sees this memory. Remove fcmem.default_owner. Replaced with options.default_owner. The fcmem variable could not be controlled by the user.
Shroud allocates memory to hold the C++ library's return value. The Fortran caller must explicitly delete it.
return-by-value std::string +deref(pointer) is owner=caller since the wrapper creates a new object. getter does not set owner since it points to memory in a class/struct.
Contains a demonstration of the changes between gcc12 and gcc13 which broke the shared.yaml test. It used the final procedure which is now correctly called after an assignment. From Modern Fortran explained incorporating Fortran 2023 page 318: When a finalizable object is about to cease to exist [as] the variable on the left-hand side of an intrinsic assignment statement. The final subroutine is invoked after the expression right-hand side has been evaluated, but before it is assigned to the variable.
When there is no user supplied construct, the Cray CCE compiler
reported a warning:
/regression/run/struct/main.f, Line = 238, Column = 14
Missing component data value for component "CXXMEM" of TYPE("CSTRUCT_AS_CLASS2").
Always add the default value:
type cstruct_as_class
type(STR_SHROUD_capsule_data) :: cxxmem = STR_SHROUD_capsule_data()
end type cstruct_as_class
Test struct.yaml for both struct-c and struct-cxx report [test_struct_class2]:Expected [T], Got [F]; User message: [name_ptr associated with name_cat]
Update run/classes/main.f to test. This produces a segfault with
F_assignment_api=basic because of a double free.
Both the original and the alias try to free.
! Test generic constructor. Allocates new memory.
obj0 = class1()
! Create alias
obj1 = obj0
call obj1%delete ! The alias does not release the memory
call obj0%delete
Test getClass1Copy with swig-fortran. Also renamed from getClassCopy
to identify more specifically what it's doing.
Test the getters to demonstrate that the classes are an alias.
Fix address sanitier error in clibrary.cxx by adding +owner(caller) to Function4a. Also needed to add statement group f_function_char*_buf_copy_caller. Note that the Python wrapper still needs to free the memory. Add make varaibles to simplify testing valgrind=valgrind use_asan=1
This fixes the double free error in the classes.yaml test 'basic' is still an option, but should go away before next release.
Error install gcc 15.
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
1 participant
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.
The
shared.yamltest usedfinalto clean up memory as part of assignment. gcc13 made the compiler more standard compliant causing the test to fail. Copied the technique used by swig-fortran which overloads assignment for each shadow type. It is responsible for releasing the temporary object created by a function call as part of the assignment:obj = object().object()creates a object which is then assigned to obj and must then be released.This also fixes issues with double free when creating an alias for an object. Tested in
classes.yaml.Add
cmemflagsto the capsule fields. TheSWIG_MEM_OWNflag works similar to the olderidtorflag to indicate if the memory needs to be released.idtor=0do not release.Add
options.default_owner=library. Used in looking up statement name already, but is now computed inmetaattr.py. This changes lots of statement groups to frompointertopointer_libraryorpointer_caller. This explicitly indicates who is responsible to release the memory. Likewise withraw.shared_ptris no longer anextendsfrom their object. They are now individual classes. This was needed to help assignment overload avoid ambigious arguments.Move capsule destructor code into new file
fcmem.py.