Skip to content

Commit dbcb879

Browse files
committed
Avoid mutating value field of constructed Integer
1 parent 94fe540 commit dbcb879

File tree

1 file changed

+40
-4
lines changed

1 file changed

+40
-4
lines changed

src/sage/rings/integer.pyx

Lines changed: 40 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -401,6 +401,42 @@ cdef class IntegerWrapper(Integer):
401401
Element.__init__(self, parent=parent)
402402
Integer.__init__(self, x, base=base)
403403

404+
405+
cdef inline Integer move_integer_from_mpz(mpz_t x):
406+
"""
407+
Construct an :class:`Integer` by moving it from a ``mpz_t`` object.
408+
409+
This function is intend to be used as follows::
410+
411+
cdef mpz_t x # local variable
412+
mpz_init(x)
413+
sig_on()
414+
mpz_SOMETHING_MUTATE_X(x, ...)
415+
sig_off()
416+
return move_integer_from_mpz(x) # no need mpz_clear(x)
417+
418+
The reason to do so is explained in :issue:`24986`:
419+
if the user interrupts the operation, ``x`` may be in an inconsistent state,
420+
as such we don't want to call ``fast_tp_dealloc`` on it, because
421+
putting a corrupted object in the integer pool may lead to incorrect results
422+
afterwards.
423+
424+
Here we do not call ``mpz_clear(x)`` either, but double ``free()``
425+
is also undesirable. Compared to these issues, memory leak is the least problematic.
426+
427+
In this case:
428+
429+
- if ``sig_on()`` throws, :func:`move_integer_from_mpz` will not be called, as such
430+
``x`` will not be cleared;
431+
432+
- if ``sig_on()`` does not throw, :func:`move_integer_from_mpz` will call ``mpz_clear(x)``.
433+
"""
434+
cdef Integer y = <Integer>PY_NEW(Integer)
435+
mpz_swap(y.value, x)
436+
mpz_clear(x)
437+
return y
438+
439+
404440
cdef class Integer(sage.structure.element.EuclideanDomainElement):
405441
r"""
406442
The :class:`Integer` class represents arbitrary precision
@@ -2299,14 +2335,14 @@ cdef class Integer(sage.structure.element.EuclideanDomainElement):
22992335
elif n == 1:
23002336
return self
23012337

2302-
cdef Integer x
23032338
cdef Rational q
2339+
cdef mpz_t x
23042340
if n > 0:
2305-
x = PY_NEW(Integer)
2341+
mpz_init(x)
23062342
sig_on()
2307-
mpz_pow_ui(x.value, self.value, n)
2343+
mpz_pow_ui(x, self.value, n)
23082344
sig_off()
2309-
return x
2345+
return move_integer_from_mpz(x)
23102346
else:
23112347
if mpz_sgn(self.value) == 0:
23122348
raise ZeroDivisionError("rational division by zero")

0 commit comments

Comments
 (0)