Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Workaround for GMP crash on Cygwin (regression introduced by PR #3434) #3435

Merged
merged 5 commits into from
May 4, 2019
Merged

Workaround for GMP crash on Cygwin (regression introduced by PR #3434) #3435

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
abd23b1
Initial fix for #3434: Add ENSURE_BAG helper which ensures memory is
embray May 2, 2019
95324a2
Just limit the scope of ENSURE_BAG to integer.c for now since it's th…
embray May 3, 2019
f29a42c
Add inline documentation for ENSURE_BAG and when/how it should be used.
embray May 3, 2019
1a6eb16
Disable the workaround if gmp.h comes from MPIR, in which case __MPIR…
embray May 3, 2019
95a011a
clang-format tweaks
embray May 3, 2019
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
39 changes: 38 additions & 1 deletion src/integer.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,35 @@ static Obj ObjInt_UIntInv( UInt i );

GAP_STATIC_ASSERT( sizeof(mp_limb_t) == sizeof(UInt), "gmp limb size incompatible with GAP word size");



/* This ensures that all memory underlying a bag is actually committed
** to physical memory and can be written to.
** This is a workaround to a bug specific to Cygwin 64-bit and bad
** interaction with GMP, so this is only needed specifically for new
** bags created in this module to hold the outputs of GMP routines.
**
** Thus, any time NewBag is called, it is also necessary to call
** ENSURE_BAG(bag) on the newly created bag if some GMP function will be
** the first place that bag's data is written to.
**
** To give a counter-example, ENSURE_BAG is *not* needed in ObjInt_Int,
** because it just creates a bag to hold a single mp_limb_t, and
** immediately assigns it a value.
**
** The bug this works around is explained more in
** https://github.com/gap-system/gap/issues/3434
*/
static inline void ENSURE_BAG(Bag bag)
{
// Note: This workaround is only required with the original GMP and not with
// MPIR
#if defined(SYS_IS_CYGWIN32) && defined(SYS_IS_64_BIT) && \
!defined(__MPIR_VERSION)
memset(PTR_BAG(bag), 0, SIZE_BAG(bag));
#endif
}


/* for fallbacks to library */
static Obj String;
static Obj OneAttr;
Expand Down Expand Up @@ -274,6 +302,7 @@ static void NEW_FAKEMPZ( fake_mpz_t fake, UInt size )
}
else {
fake->obj = NewBag( T_INTPOS, size * sizeof(mp_limb_t) );
ENSURE_BAG(fake->obj);
}
}

Expand Down Expand Up @@ -1789,9 +1818,11 @@ Obj ModInt(Obj opL, Obj opR)
}

mod = NewBag( TNUM_OBJ(opL), (SIZE_INT(opL)+1)*sizeof(mp_limb_t) );
ENSURE_BAG(mod);

quo = NewBag( T_INTPOS,
(SIZE_INT(opL)-SIZE_INT(opR)+1)*sizeof(mp_limb_t) );
ENSURE_BAG(quo);

/* and let gmp do the work */
mpn_tdiv_qr( (mp_ptr)ADDR_INT(quo), (mp_ptr)ADDR_INT(mod), 0,
Expand Down Expand Up @@ -1888,6 +1919,8 @@ Obj QuoInt(Obj opL, Obj opR)
quo = NewBag( T_INTPOS, SIZE_OBJ(opL) );
else
quo = NewBag( T_INTNEG, SIZE_OBJ(opL) );

ENSURE_BAG(quo);

if ( k < 0 ) k = -k;

Expand All @@ -1906,6 +1939,7 @@ Obj QuoInt(Obj opL, Obj opR)

/* create a new bag for the remainder */
rem = NewBag( TNUM_OBJ(opL), (SIZE_INT(opL)+1)*sizeof(mp_limb_t) );
ENSURE_BAG(rem);

/* allocate a bag for the quotient */
if ( TNUM_OBJ(opL) == TNUM_OBJ(opR) )
Expand All @@ -1914,6 +1948,7 @@ Obj QuoInt(Obj opL, Obj opR)
else
quo = NewBag( T_INTNEG,
(SIZE_INT(opL)-SIZE_INT(opR)+1)*sizeof(mp_limb_t) );
ENSURE_BAG(quo);

mpn_tdiv_qr( (mp_ptr)ADDR_INT(quo), (mp_ptr)ADDR_INT(rem), 0,
(mp_srcptr)CONST_ADDR_INT(opL), SIZE_INT(opL),
Expand Down Expand Up @@ -2035,9 +2070,11 @@ Obj RemInt(Obj opL, Obj opR)
return opL;

rem = NewBag( TNUM_OBJ(opL), (SIZE_INT(opL)+1)*sizeof(mp_limb_t) );
ENSURE_BAG(rem);

quo = NewBag( T_INTPOS,
(SIZE_INT(opL)-SIZE_INT(opR)+1)*sizeof(mp_limb_t) );
ENSURE_BAG(quo);

/* and let gmp do the work */
mpn_tdiv_qr( (mp_ptr)ADDR_INT(quo), (mp_ptr)ADDR_INT(rem), 0,
Expand Down