Skip to content

Fix gap libgap segfault #40585

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

Closed
wants to merge 5 commits into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 41 additions & 5 deletions src/sage/libs/gap/element.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ elements. For general information about GAP, you should read the
# ****************************************************************************

from cpython.object cimport Py_EQ, Py_NE, Py_LE, Py_GE, Py_LT, Py_GT
from cpython.ref cimport Py_INCREF, Py_DECREF
from cysignals.signals cimport sig_on, sig_off

from sage.libs.gap.gap_includes cimport *
Expand All @@ -28,6 +29,8 @@ from sage.cpython.string cimport str_to_bytes, char_to_str
from sage.rings.integer_ring import ZZ
from sage.rings.rational_field import QQ
from sage.rings.real_double import RDF
from libc.stdlib cimport malloc, calloc, free
from libc.string cimport memset

from sage.groups.perm_gps.permgroup_element cimport PermutationGroupElement
from sage.combinat.permutation import Permutation
Expand Down Expand Up @@ -2495,12 +2498,13 @@ cdef class GapElement_Function(GapElement):
sage: libgap_exec('echo hello from the shell')
hello from the shell
"""
cdef Obj result = NULL
cdef Obj arg_list
cdef volatile Obj result = NULL
cdef int n = len(args)
cdef volatile Obj v2
cdef volatile Obj *arg_array = NULL
cdef int i, refs_incremented = 0

if n > 0 and n <= 3:
if n > 0:
libgap = self.parent()
a = [x if isinstance(x, GapElement) else libgap(x) for x in args]

Expand All @@ -2523,8 +2527,40 @@ cdef class GapElement_Function(GapElement):
(<GapElement>a[1]).value,
v2)
else:
arg_list = make_gap_list(args)
result = GAP_CallFuncList(self.value, arg_list)
# ULTRA-ROBUST MEMORY MANAGEMENT for n >= 4 arguments
# Following the comprehensive approach to prevent all GC issues

# 1) Pin all Python GapElement objects BEFORE entering GAP region
for i in range(n):
Py_INCREF(a[i])
refs_incremented = n

# 2) Allocate and zero-initialize the arg array
arg_array = <Obj*>calloc(n, sizeof(Obj))
if arg_array == NULL:
# rollback INCREFs on allocation failure
for i in range(refs_incremented):
Py_DECREF(a[i])
refs_incremented = 0
raise MemoryError("Failed to allocate memory for GAP function arguments")

try:
# 3) Fill arg_array from pinned 'a' (these a[i] won't be GC'd now)
for i in range(n):
arg_array[i] = (<GapElement>a[i]).value

# 4) Single GAP call - all objects are protected
result = GAP_CallFuncArray(self.value, n, <Obj*>arg_array)

finally:
# 5) cleanup: free array and unpin Python objs
if arg_array != NULL:
free(<void*>arg_array)
arg_array = NULL
# DECREF exactly refs_incremented times
for i in range(refs_incremented):
Py_DECREF(a[i])
refs_incremented = 0
sig_off()
finally:
GAP_Leave()
Expand Down
Loading