|
| 1 | +Patch for https://trac.sagemath.org/ticket/27724 |
| 2 | +Can be removed when updating to GAP >= 4.11 |
| 3 | +From abd23b1360bc569fca9bbb36e2fc167d55c7a872 Mon Sep 17 00:00:00 2001 |
| 4 | +From: "Erik M. Bray" < [email protected]> |
| 5 | +Date: Thu, 2 May 2019 16:03:15 +0200 |
| 6 | +Subject: [PATCH 1/5] Initial fix for #3434: Add ENSURE_BAG helper which |
| 7 | + ensures memory is really committed by the OS for a new bag. |
| 8 | + |
| 9 | +Call ENSURE_BAG on new bags created in integer.c if they might be |
| 10 | +passed as an argument to a GMP function. |
| 11 | +--- |
| 12 | + src/gasman.h | 12 ++++++++++++ |
| 13 | + src/integer.c | 9 +++++++++ |
| 14 | + 2 files changed, 21 insertions(+) |
| 15 | + |
| 16 | +diff --git a/src/gasman.h b/src/gasman.h |
| 17 | +index 62d637baa9..e8c3d9eca1 100644 |
| 18 | +--- a/src/gasman.h |
| 19 | ++++ b/src/gasman.h |
| 20 | +@@ -553,6 +553,18 @@ EXPORT_INLINE UInt ResizeWordSizedBag(Bag bag, UInt size) |
| 21 | + } |
| 22 | + |
| 23 | + |
| 24 | ++#if defined(SYS_IS_CYGWIN32) && defined(SYS_IS_64_BIT) |
| 25 | ++EXPORT_INLINE void ENSURE_BAG(Bag bag) |
| 26 | ++{ |
| 27 | ++ memset(PTR_BAG(bag), 0, SIZE_BAG(bag)); |
| 28 | ++} |
| 29 | ++#else |
| 30 | ++EXPORT_INLINE void ENSURE_BAG(Bag bag) |
| 31 | ++{ |
| 32 | ++} |
| 33 | ++#endif |
| 34 | ++ |
| 35 | ++ |
| 36 | + /**************************************************************************** |
| 37 | + ** |
| 38 | + *F CollectBags(<size>,<full>) . . . . . . . . . . . . . . collect dead bags |
| 39 | +diff --git a/src/integer.c b/src/integer.c |
| 40 | +index c15acdb415..ed740684ca 100644 |
| 41 | +--- a/src/integer.c |
| 42 | ++++ b/src/integer.c |
| 43 | +@@ -274,6 +274,7 @@ static void NEW_FAKEMPZ( fake_mpz_t fake, UInt size ) |
| 44 | + } |
| 45 | + else { |
| 46 | + fake->obj = NewBag( T_INTPOS, size * sizeof(mp_limb_t) ); |
| 47 | ++ ENSURE_BAG(fake->obj); |
| 48 | + } |
| 49 | + } |
| 50 | + |
| 51 | +@@ -1789,9 +1790,11 @@ Obj ModInt(Obj opL, Obj opR) |
| 52 | + } |
| 53 | + |
| 54 | + mod = NewBag( TNUM_OBJ(opL), (SIZE_INT(opL)+1)*sizeof(mp_limb_t) ); |
| 55 | ++ ENSURE_BAG(mod); |
| 56 | + |
| 57 | + quo = NewBag( T_INTPOS, |
| 58 | + (SIZE_INT(opL)-SIZE_INT(opR)+1)*sizeof(mp_limb_t) ); |
| 59 | ++ ENSURE_BAG(quo); |
| 60 | + |
| 61 | + /* and let gmp do the work */ |
| 62 | + mpn_tdiv_qr( (mp_ptr)ADDR_INT(quo), (mp_ptr)ADDR_INT(mod), 0, |
| 63 | +@@ -1888,6 +1891,8 @@ Obj QuoInt(Obj opL, Obj opR) |
| 64 | + quo = NewBag( T_INTPOS, SIZE_OBJ(opL) ); |
| 65 | + else |
| 66 | + quo = NewBag( T_INTNEG, SIZE_OBJ(opL) ); |
| 67 | ++ |
| 68 | ++ ENSURE_BAG(quo); |
| 69 | + |
| 70 | + if ( k < 0 ) k = -k; |
| 71 | + |
| 72 | +@@ -1906,6 +1911,7 @@ Obj QuoInt(Obj opL, Obj opR) |
| 73 | + |
| 74 | + /* create a new bag for the remainder */ |
| 75 | + rem = NewBag( TNUM_OBJ(opL), (SIZE_INT(opL)+1)*sizeof(mp_limb_t) ); |
| 76 | ++ ENSURE_BAG(rem); |
| 77 | + |
| 78 | + /* allocate a bag for the quotient */ |
| 79 | + if ( TNUM_OBJ(opL) == TNUM_OBJ(opR) ) |
| 80 | +@@ -1914,6 +1920,7 @@ Obj QuoInt(Obj opL, Obj opR) |
| 81 | + else |
| 82 | + quo = NewBag( T_INTNEG, |
| 83 | + (SIZE_INT(opL)-SIZE_INT(opR)+1)*sizeof(mp_limb_t) ); |
| 84 | ++ ENSURE_BAG(quo); |
| 85 | + |
| 86 | + mpn_tdiv_qr( (mp_ptr)ADDR_INT(quo), (mp_ptr)ADDR_INT(rem), 0, |
| 87 | + (mp_srcptr)CONST_ADDR_INT(opL), SIZE_INT(opL), |
| 88 | +@@ -2035,9 +2042,11 @@ Obj RemInt(Obj opL, Obj opR) |
| 89 | + return opL; |
| 90 | + |
| 91 | + rem = NewBag( TNUM_OBJ(opL), (SIZE_INT(opL)+1)*sizeof(mp_limb_t) ); |
| 92 | ++ ENSURE_BAG(rem); |
| 93 | + |
| 94 | + quo = NewBag( T_INTPOS, |
| 95 | + (SIZE_INT(opL)-SIZE_INT(opR)+1)*sizeof(mp_limb_t) ); |
| 96 | ++ ENSURE_BAG(quo); |
| 97 | + |
| 98 | + /* and let gmp do the work */ |
| 99 | + mpn_tdiv_qr( (mp_ptr)ADDR_INT(quo), (mp_ptr)ADDR_INT(rem), 0, |
| 100 | + |
| 101 | +From 95324a2b978444ccd783b16cf50625b390107d97 Mon Sep 17 00:00:00 2001 |
| 102 | +From: "Erik M. Bray" < [email protected]> |
| 103 | +Date: Fri, 3 May 2019 14:19:57 +0200 |
| 104 | +Subject: [PATCH 2/5] Just limit the scope of ENSURE_BAG to integer.c for now |
| 105 | + since it's the only known use case. |
| 106 | + |
| 107 | +--- |
| 108 | + src/gasman.h | 12 ------------ |
| 109 | + src/integer.c | 10 +++++++++- |
| 110 | + 2 files changed, 9 insertions(+), 13 deletions(-) |
| 111 | + |
| 112 | +diff --git a/src/gasman.h b/src/gasman.h |
| 113 | +index e8c3d9eca1..62d637baa9 100644 |
| 114 | +--- a/src/gasman.h |
| 115 | ++++ b/src/gasman.h |
| 116 | +@@ -553,18 +553,6 @@ EXPORT_INLINE UInt ResizeWordSizedBag(Bag bag, UInt size) |
| 117 | + } |
| 118 | + |
| 119 | + |
| 120 | +-#if defined(SYS_IS_CYGWIN32) && defined(SYS_IS_64_BIT) |
| 121 | +-EXPORT_INLINE void ENSURE_BAG(Bag bag) |
| 122 | +-{ |
| 123 | +- memset(PTR_BAG(bag), 0, SIZE_BAG(bag)); |
| 124 | +-} |
| 125 | +-#else |
| 126 | +-EXPORT_INLINE void ENSURE_BAG(Bag bag) |
| 127 | +-{ |
| 128 | +-} |
| 129 | +-#endif |
| 130 | +- |
| 131 | +- |
| 132 | + /**************************************************************************** |
| 133 | + ** |
| 134 | + *F CollectBags(<size>,<full>) . . . . . . . . . . . . . . collect dead bags |
| 135 | +diff --git a/src/integer.c b/src/integer.c |
| 136 | +index ed740684ca..8120bd44c9 100644 |
| 137 | +--- a/src/integer.c |
| 138 | ++++ b/src/integer.c |
| 139 | +@@ -141,7 +141,15 @@ static Obj ObjInt_UIntInv( UInt i ); |
| 140 | + |
| 141 | + GAP_STATIC_ASSERT( sizeof(mp_limb_t) == sizeof(UInt), "gmp limb size incompatible with GAP word size"); |
| 142 | + |
| 143 | +- |
| 144 | ++ |
| 145 | ++static inline void ENSURE_BAG(Bag bag) |
| 146 | ++{ |
| 147 | ++#if defined(SYS_IS_CYGWIN32) && defined(SYS_IS_64_BIT) |
| 148 | ++ memset(PTR_BAG(bag), 0, SIZE_BAG(bag)); |
| 149 | ++#endif |
| 150 | ++} |
| 151 | ++ |
| 152 | ++ |
| 153 | + /* for fallbacks to library */ |
| 154 | + static Obj String; |
| 155 | + static Obj OneAttr; |
| 156 | + |
| 157 | +From f29a42c60e81732aabda0d0ab11fffdd26da059d Mon Sep 17 00:00:00 2001 |
| 158 | +From: "Erik M. Bray" < [email protected]> |
| 159 | +Date: Fri, 3 May 2019 14:32:36 +0200 |
| 160 | +Subject: [PATCH 3/5] Add inline documentation for ENSURE_BAG and when/how it |
| 161 | + should be used. |
| 162 | + |
| 163 | +--- |
| 164 | + src/integer.c | 17 +++++++++++++++++ |
| 165 | + 1 file changed, 17 insertions(+) |
| 166 | + |
| 167 | +diff --git a/src/integer.c b/src/integer.c |
| 168 | +index 8120bd44c9..4c5cf683d8 100644 |
| 169 | +--- a/src/integer.c |
| 170 | ++++ b/src/integer.c |
| 171 | +@@ -142,6 +142,23 @@ static Obj ObjInt_UIntInv( UInt i ); |
| 172 | + GAP_STATIC_ASSERT( sizeof(mp_limb_t) == sizeof(UInt), "gmp limb size incompatible with GAP word size"); |
| 173 | + |
| 174 | + |
| 175 | ++/* This ensures that all memory underlying a bag is actually committed |
| 176 | ++** to physical memory and can be written to. |
| 177 | ++** This is a workaround to a bug specific to Cygwin 64-bit and bad |
| 178 | ++** interaction with GMP, so this is only needed specifically for new |
| 179 | ++** bags created in this module to hold the outputs of GMP routines. |
| 180 | ++** |
| 181 | ++** Thus, any time NewBag is called, it is also necessary to call |
| 182 | ++** ENSURE_BAG(bag) on the newly created bag if some GMP function will be |
| 183 | ++** the first place that bag's data is written to. |
| 184 | ++** |
| 185 | ++** To give a counter-example, ENSURE_BAG is *not* needed in ObjInt_Int, |
| 186 | ++** because it just creates a bag to hold a single mp_limb_t, and |
| 187 | ++** immediately assigns it a value. |
| 188 | ++** |
| 189 | ++** The bug this works around is explained more in |
| 190 | ++** https://github.com/gap-system/gap/issues/3434 |
| 191 | ++*/ |
| 192 | + static inline void ENSURE_BAG(Bag bag) |
| 193 | + { |
| 194 | + #if defined(SYS_IS_CYGWIN32) && defined(SYS_IS_64_BIT) |
| 195 | + |
| 196 | +From 1a6eb168f005ddd1fe6d5591be310112aa4de393 Mon Sep 17 00:00:00 2001 |
| 197 | +From: "Erik M. Bray" < [email protected]> |
| 198 | +Date: Fri, 3 May 2019 14:36:19 +0200 |
| 199 | +Subject: [PATCH 4/5] Disable the workaround if gmp.h comes from MPIR, in which |
| 200 | + case __MPIR_VERSION should be defined. |
| 201 | + |
| 202 | +--- |
| 203 | + src/integer.c | 3 ++- |
| 204 | + 1 file changed, 2 insertions(+), 1 deletion(-) |
| 205 | + |
| 206 | +diff --git a/src/integer.c b/src/integer.c |
| 207 | +index 4c5cf683d8..b31c5f3d65 100644 |
| 208 | +--- a/src/integer.c |
| 209 | ++++ b/src/integer.c |
| 210 | +@@ -161,7 +161,8 @@ GAP_STATIC_ASSERT( sizeof(mp_limb_t) == sizeof(UInt), "gmp limb size incompatibl |
| 211 | + */ |
| 212 | + static inline void ENSURE_BAG(Bag bag) |
| 213 | + { |
| 214 | +-#if defined(SYS_IS_CYGWIN32) && defined(SYS_IS_64_BIT) |
| 215 | ++// Note: This workaround is only required with the original GMP and not with MPIR |
| 216 | ++#if defined(SYS_IS_CYGWIN32) && defined(SYS_IS_64_BIT) && !defined(__MPIR_VERSION) |
| 217 | + memset(PTR_BAG(bag), 0, SIZE_BAG(bag)); |
| 218 | + #endif |
| 219 | + } |
| 220 | + |
| 221 | +From 95a011a793a326f14a0b63d5f53044d9f8d330bb Mon Sep 17 00:00:00 2001 |
| 222 | +From: "Erik M. Bray" < [email protected]> |
| 223 | +Date: Fri, 3 May 2019 12:33:03 +0000 |
| 224 | +Subject: [PATCH 5/5] clang-format tweaks |
| 225 | + |
| 226 | +--- |
| 227 | + src/integer.c | 6 ++++-- |
| 228 | + 1 file changed, 4 insertions(+), 2 deletions(-) |
| 229 | + |
| 230 | +diff --git a/src/integer.c b/src/integer.c |
| 231 | +index b31c5f3d65..b34d6c382b 100644 |
| 232 | +--- a/src/integer.c |
| 233 | ++++ b/src/integer.c |
| 234 | +@@ -161,8 +161,10 @@ GAP_STATIC_ASSERT( sizeof(mp_limb_t) == sizeof(UInt), "gmp limb size incompatibl |
| 235 | + */ |
| 236 | + static inline void ENSURE_BAG(Bag bag) |
| 237 | + { |
| 238 | +-// Note: This workaround is only required with the original GMP and not with MPIR |
| 239 | +-#if defined(SYS_IS_CYGWIN32) && defined(SYS_IS_64_BIT) && !defined(__MPIR_VERSION) |
| 240 | ++// Note: This workaround is only required with the original GMP and not with |
| 241 | ++// MPIR |
| 242 | ++#if defined(SYS_IS_CYGWIN32) && defined(SYS_IS_64_BIT) && \ |
| 243 | ++ !defined(__MPIR_VERSION) |
| 244 | + memset(PTR_BAG(bag), 0, SIZE_BAG(bag)); |
| 245 | + #endif |
| 246 | + } |
0 commit comments