Re: Bignum speedup patch causes crash at startup

[Paul Eggert <eggert@cs.ucla.edu>]

[-- Attachment #1: Type: text/plain, Size: 873 bytes --]

Andy Moreton wrote:

> The recent changes in commit fe042e9d ("Speed up (+ 2 2) by a factor of
> 10") cause an immediate crash in 64bit emacs on Windows.

Thanks for reporting it. Please try the attached patch, which I installed on master.

> A patch that is this invasive should have been posted for comment and
> review before it was committed.

My feeling was that it was limited to bignum calculations, a reasonably compact 
area conceptually where we don't have many reviewers, unfortunately. Also, my 
judgment was that the effort to split this patch into pieces would have been 
more trouble overall than it would have been worth. Obviously this is a judgment 
call.

I do regularly post for comment patches that I think might run into trouble on 
the MS-Windows side. I didn't think this one would cause trouble. Evidently I 
was wrong, sorry; these things happen.

[-- Attachment #2: 0001-Fix-bignum-initialization.patch --]
[-- Type: text/x-patch, Size: 3215 bytes --]

From 1d84e6523250ab6d14f40fba3922c56d7a40416f Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Tue, 4 Sep 2018 09:30:57 -0700
Subject: [PATCH] Fix bignum initialization

Problem reported by Andy Moreton in:
https://lists.gnu.org/r/emacs-devel/2018-09/msg00072.html
and crystal-ball diagnosis by Eli Zaretskii in:
https://lists.gnu.org/r/emacs-devel/2018-09/msg00075.html
* src/alloc.c (xrealloc_for_gmp, xfree_for_gmp): Move to bignum.c.
(init_alloc): Move bignum initialization to init_bignum.
* src/bignum.c (init_bignum): Rename from init_bignum_once.
All users changed.
* src/emacs.c (main): Call init_bignum after init_alloc,
instead of calling init_bignum_once after init_bignum.
---
 src/alloc.c  | 16 ----------------
 src/bignum.c | 18 +++++++++++++++++-
 src/bignum.h |  2 +-
 src/emacs.c  |  2 +-
 4 files changed, 19 insertions(+), 19 deletions(-)

diff --git a/src/alloc.c b/src/alloc.c
index 1eab82d1c2..28ca7804ee 100644
--- a/src/alloc.c
+++ b/src/alloc.c
@@ -7126,18 +7126,6 @@ range_error (void)
   xsignal0 (Qrange_error);
 }
 
-static void *
-xrealloc_for_gmp (void *ptr, size_t ignore, size_t size)
-{
-  return xrealloc (ptr, size);
-}
-
-static void
-xfree_for_gmp (void *ptr, size_t ignore)
-{
-  xfree (ptr);
-}
-
 /* Initialization.  */
 
 void
@@ -7171,10 +7159,6 @@ init_alloc_once (void)
 void
 init_alloc (void)
 {
-  eassert (mp_bits_per_limb == GMP_NUMB_BITS);
-  integer_width = 1 << 16;
-  mp_set_memory_functions (xmalloc, xrealloc_for_gmp, xfree_for_gmp);
-
   Vgc_elapsed = make_float (0.0);
   gcs_done = 0;
 
diff --git a/src/bignum.c b/src/bignum.c
index 2ce7412d06..35894f5647 100644
--- a/src/bignum.c
+++ b/src/bignum.c
@@ -34,9 +34,25 @@ along with GNU Emacs.  If not, see <https://www.gnu.org/licenses/>.  */
 
 mpz_t mpz[4];
 
+static void *
+xrealloc_for_gmp (void *ptr, size_t ignore, size_t size)
+{
+  return xrealloc (ptr, size);
+}
+
+static void
+xfree_for_gmp (void *ptr, size_t ignore)
+{
+  xfree (ptr);
+}
+
 void
-init_bignum_once (void)
+init_bignum (void)
 {
+  eassert (mp_bits_per_limb == GMP_NUMB_BITS);
+  integer_width = 1 << 16;
+  mp_set_memory_functions (xmalloc, xrealloc_for_gmp, xfree_for_gmp);
+
   for (int i = 0; i < ARRAYELTS (mpz); i++)
     mpz_init (mpz[i]);
 }
diff --git a/src/bignum.h b/src/bignum.h
index 07622a37af..0e38c615ee 100644
--- a/src/bignum.h
+++ b/src/bignum.h
@@ -43,7 +43,7 @@ struct Lisp_Bignum
 
 extern mpz_t mpz[4];
 
-extern void init_bignum_once (void);
+extern void init_bignum (void);
 extern Lisp_Object make_integer_mpz (void);
 extern void mpz_set_intmax_slow (mpz_t, intmax_t) ARG_NONNULL ((1));
 
diff --git a/src/emacs.c b/src/emacs.c
index 5b399eca64..b1c96d1828 100644
--- a/src/emacs.c
+++ b/src/emacs.c
@@ -1209,7 +1209,6 @@ Using an Emacs configured with --with-x-toolkit=lucid does not have this problem
   if (!initialized)
     {
       init_alloc_once ();
-      init_bignum_once ();
       init_threads_once ();
       init_obarray ();
       init_eval_once ();
@@ -1257,6 +1256,7 @@ Using an Emacs configured with --with-x-toolkit=lucid does not have this problem
     }
 
   init_alloc ();
+  init_bignum ();
   init_threads ();
 
   if (do_initial_setlocale)
-- 
2.17.1