There was a test failure under PyPy:

@oscarbenjamin

There was a test failure under PyPy in gh-323:

File "/Users/runner/hostedtoolcache/PyPy/3.11.13/x64/lib/pypy3.11/site-packages/flint/types/fq_default.pypy311-pp73-darwin.so", line 270, in flint.types.fq_default.fq_default_ctx.gen
Failed example:
    gf.gen()
Expected:
    w
Got:
    <BLANKLINE>

The change here does not look like it could cause that so maybe there is something undefined here.

Originally posted by @oscarbenjamin in #323 (comment)

There was a test failure under PyPy:

I can't reproduce it. I've tested under pypy and all tests pass.

The failed doctest is just something like:

from flint import fq_default_ctx

gf = fq_default_ctx(5, 2, var="w")
g = gf.gen()
assert str(g) == 'w'
assert repr(g) == 'w'

I've run that under PyPy 1000 times though and don't see the failure.

The implementation here creates a polynomial and tries to print that:

python-flint/src/flint/types/fq_default.pyx

Lines 505 to 506 in 0a59f53

    
           def str(self): 
        
               return self.polynomial().str(var=self.ctx.var.decode())

The polynomial printing code does not check if var is a non-empty or valid string:

python-flint/src/flint/flint_base/flint_base.pyx

Lines 191 to 224 in 0a59f53

    
               def str(self, bint ascending=False, var="x", *args, **kwargs): 
        
                   """ 
        
                   Convert to a human-readable string (generic implementation for 
        
                   all polynomial types). 
        
                   If *ascending* is *True*, the monomials are output from low degree to 
        
                   high, otherwise from high to low. 
        
                   """ 
        
                   coeffs = [c.str(*args, **kwargs) for c in self] 
        
                   if not coeffs: 
        
                       return "0" 
        
                   s = [] 
        
                   coeffs = enumerate(coeffs) 
        
                   if not ascending: 
        
                       coeffs = reversed(list(coeffs)) 
        
                   for i, c in coeffs: 
        
                       if c == "0": 
        
                           continue 
        
                       else: 
        
                           if c.startswith("-") or (" " in c): 
        
                               c = "(" + c + ")" 
        
                           if i == 0: 
        
                               s.append("%s" % c) 
        
                           elif i == 1: 
        
                               if c == "1": 
        
                                   s.append(var) 
        
                               else: 
        
                                   s.append(f"{c}*{var}") 
        
                           else: 
        
                               if c == "1": 
        
                                   s.append(f"{var}^{i}") 
        
                               else: 
        
                                   s.append(f"{c}*{var}^{i}") 
        
                   return " + ".join(s)

It is maybe worth adding a check there so that we could see if that is what failed if the test ever fails again in future.

The var is coming from self.ctx.var.decode() which is:

python-flint/src/flint/types/fq_default.pxd

Lines 25 to 28 in 0a59f53

    
           cdef class fq_default_ctx: 
        
               cdef fq_default_ctx_t val 
        
               cdef readonly char *var 
        
               cdef bint _initialized

and is initialised here:

python-flint/src/flint/types/fq_default.pyx

Lines 65 to 85 in 0a59f53

    
           @staticmethod 
        
           def _parse_input_var(var): 
        
               # If no variable is given, use x 
        
               if var is None: 
        
                   var = b"z" 
        
               # Encode to bytes for cython to parse 
        
               if isinstance(var, str): 
        
                   var = var.encode() 
        
               # TODO: Flint only wants one-character inputs 
        
               if len(var) > 1: 
        
                   raise ValueError("variable for GF(p^k) generator can only be one character") 
        
               return var 
        
           def __init__(self, p=None, degree=None, var=None, modulus=None, fq_type=fq_default_type.DEFAULT, 
        
                        check_prime=True, check_modulus=True): 
        
               # Ensure the var used for the generator of GF(p^d) is a single byte 
        
               var = self._parse_input_var(var) 
        
               self.var = var

I'm a bit suspicious about just assigning a Python bytes object to a char*.

Is that making a copy or possibly just storing a pointer?

The test failure is under PyPy so it is possible that this takes a different codepath in the Cython-generated C code.

Looking in the generated C code in build/src/flint/types/fq_default.pypy311-pp73-x86_64-linux-gnu.so.p/src/flint/types/fq_default.pyx.c there is:

  /* "flint/types/fq_default.pyx":85
 *         # Ensure the var used for the generator of GF(p^d) is a single byte
 *         var = self._parse_input_var(var)
 *         self.var = var             # <<<<<<<<<<<<<<
 * 
 *         # Ensure the fq_type is an integer between 0, 5 -- we allow users to
*/
  __pyx_t_4 = __Pyx_PyObject_AsWritableString(__pyx_v_var); if (unlikely((!__pyx_t_4) && PyErr_Occurred())) __PYX_ERR(0, 85, __pyx_L1_error)
  __pyx_v_self->var = __pyx_t_4;

Chasing the __Pyx_PyObject_AsWritableString macro leads to:

static CYTHON_INLINE const char* __Pyx_PyObject_AsStringAndSize(PyObject* o, Py_ssize_t *length) {
#if __PYX_DEFAULT_STRING_ENCODING_IS_ASCII || __PYX_DEFAULT_STRING_ENCODING_IS_UTF8
    if (PyUnicode_Check(o)) {
        return __Pyx_PyUnicode_AsStringAndSize(o, length);
    } else
#endif
    if (PyByteArray_Check(o)) {
#if (CYTHON_ASSUME_SAFE_SIZE && CYTHON_ASSUME_SAFE_MACROS) || (CYTHON_COMPILING_IN_PYPY && (defined(PyByteArray_AS_STRING) && defined(PyByteArray_GET_SIZE)))
        *length = PyByteArray_GET_SIZE(o);
        return PyByteArray_AS_STRING(o);
#else
        *length = PyByteArray_Size(o);
        if (*length == -1) return NULL;
        return PyByteArray_AsString(o);
#endif
    } else
    {
        char* result;
        int r = PyBytes_AsStringAndSize(o, &result, length);
        if (unlikely(r < 0)) {
            return NULL;
        } else {
            return result;
        }
    }
}

The CYTHON_COMPILING_IN_PYPY define looks potentially relevant.

The macro PyByteArray_AS_STRING is defined here:
https://github.com/python/cpython/blob/d7b9ea5cab984497526fcc0b988e4eb5988c1e88/Include/cpython/bytearrayobject.h#L5-L28
It looks like this just returns the raw pointer i.e. it does not allocate a new string.

My guess then is that the fq_default_ctx ends up holding a pointer to the buffer for a bytes object that was deallocated. The memory was overwritten with something and then self.ctx.var.decode() ends up having a pointer to a null byte "'\0'" which decode turns into an empty string.

I guess we need a malloc/copy here or on the other hand if as noted in the comments FLINT only allows a single character string we could change it so that var is char[2] rather than a char* and just copy the first byte in.

Also _parse_input_var should check that the string is not of length 0.

I'd be very suspicious of that assignment to char*. The Python string ultimately owns the memory so you need to ensure that it outlives that char*. That's often fine for char* function locals but not so fine for class attributes.

There's probably some cases where it'll work (the val is None case will end up pointing to a cached string for example) but a lot of others where it won't.

So I don't think this is specifically a PyPy issue here

(I suspect a short string like w might end up interned by the Python interpreter so you get away with it. But that's not something to rely on)

Thanks, I forgot about this issue. Yes the assignment to char* I think is unsafe. My notes above were just me trying to figure out what it does which I think is basically just

cdef bytes bytestring
cdef char *var
var = bytestring->ob_start

It isn't safe to store that pointer if we don't also hold a reference to the bytestring.

Looking at it I think that var should just be a Python string object rather than char * though. There are many places doing self.var.decode() and char* is only needed at context initialisation time for functions like fq_default_ctx_init_type.

Assuming it works as shown above I think that while we hold a reference to the bytes object it should be safe to pass a bytes from Cython into a C function that expects const char* so that FLINT can malloc/copy and attach it to the fq_default_ctx struct.

Assuming it works as shown above I think that while we hold a reference to the bytes object it should be safe to pass a bytes from Cython into a C function that expects const char* so that FLINT can malloc/copy and attach it to the fq_default_ctx struct.

Yes that should be safe (and is the intention of the bytes to const char* conversion)

fix(fq_default): store variable name as str

@da-woods

Thanks. I've fixed this by going for the safer option of just storing a normal Python string that can be reference counted.

It isn't clearly documented in FLINT but the char* that gets passed in to fq_default_ctx_init_* functions is copied into a newly malloc'd buffer which is then attached to the struct and gets cleared when the struct is freed:

python-flint/src/flint/types/fq_default.pyx

Lines 45 to 48 in 80f941f

    
           def __dealloc__(self): 
        
               if self._initialized: 
        
                   fq_default_ctx_clear(self.val) 
        
                   self._initialized = False

By the way @da-woods since you are here do you know if there are any guarantees about how many times __dealloc__ is called or whether it could be called from two separate threads (in free-threading)?

The user code in __dealloc__ is called exactly once.

I don't think you can reason too much in advance about which thread might run it but it should only be one thread.

Thanks, that is reassuring. In our case here it doesn't matter which thread but we want to call that *_clear function exactly once for each object so that is the precise guarantee that is needed.

Navigation Menu

Repository navigation

There was a test failure under PyPy: #325

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

	def str(self):
	return self.polynomial().str(var=self.ctx.var.decode())

	def str(self, bint ascending=False, var="x", args, *kwargs):
	"""
	Convert to a human-readable string (generic implementation for
	all polynomial types).

	If ascending is True, the monomials are output from low degree to
	high, otherwise from high to low.
	"""
	coeffs = [c.str(args, *kwargs) for c in self]
	if not coeffs:
	return "0"
	s = []
	coeffs = enumerate(coeffs)
	if not ascending:
	coeffs = reversed(list(coeffs))
	for i, c in coeffs:
	if c == "0":
	continue
	else:
	if c.startswith("-") or (" " in c):
	c = "(" + c + ")"
	if i == 0:
	s.append("%s" % c)
	elif i == 1:
	if c == "1":
	s.append(var)
	else:
	s.append(f"{c}*{var}")
	else:
	if c == "1":
	s.append(f"{var}^{i}")
	else:
	s.append(f"{c}*{var}^{i}")
	return " + ".join(s)

	cdef class fq_default_ctx:
	cdef fq_default_ctx_t val
	cdef readonly char *var
	cdef bint _initialized

	@staticmethod
	def _parse_input_var(var):
	# If no variable is given, use x
	if var is None:
	var = b"z"

	# Encode to bytes for cython to parse
	if isinstance(var, str):
	var = var.encode()

	# TODO: Flint only wants one-character inputs
	if len(var) > 1:
	raise ValueError("variable for GF(p^k) generator can only be one character")

	return var

	def __init__(self, p=None, degree=None, var=None, modulus=None, fq_type=fq_default_type.DEFAULT,
	check_prime=True, check_modulus=True):
	# Ensure the var used for the generator of GF(p^d) is a single byte
	var = self._parse_input_var(var)
	self.var = var

	def __dealloc__(self):
	if self._initialized:
	fq_default_ctx_clear(self.val)
	self._initialized = False

There was a test failure under PyPy: #325

Description

Activity

oscarbenjamin commented on Sep 7, 2025

da-woods commented on Nov 1, 2025

oscarbenjamin commented on Nov 1, 2025

da-woods commented on Nov 1, 2025

oscarbenjamin commented on Nov 1, 2025

da-woods commented on Nov 1, 2025

oscarbenjamin commented on Nov 2, 2025

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions