unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* Re: master 2c8b09b06e7: Fix crash on Windows 9X
       [not found] ` <20221206013135.E2E2DC004BE@vcs2.savannah.gnu.org>
@ 2022-12-06  3:59   ` Stefan Monnier
  2022-12-06  4:56     ` Po Lu
  0 siblings, 1 reply; 10+ messages in thread
From: Stefan Monnier @ 2022-12-06  3:59 UTC (permalink / raw)
  To: emacs-devel; +Cc: Po Lu

> --- a/src/emacs.c
> +++ b/src/emacs.c
> @@ -1924,6 +1924,12 @@ Using an Emacs configured with --with-x-toolkit=lucid does not have this problem
>  	 Vcoding_system_hash_table.  */
>        syms_of_coding ();	/* This should be after syms_of_fileio.  */
>        init_frame_once ();       /* Before init_window_once.  */
> +      /* init_window_once calls make_initial_frame, which calls
> +	 Fcurrent_time and bset_display_time, both of which allocate
> +	 bignums.  Without the following call to init_bignums, crashes
> +	 happen on Windows 9X after dumping when GC tries to free a
> +	 pointer allocated on the system heap.  */
> +      init_bignum ();
>        init_window_once ();	/* Init the window system.  */
>  #ifdef HAVE_WINDOW_SYSTEM
>        init_fringe_once ();	/* Swap bitmaps if necessary.  */

I feel like I'm missing something: this adds a call to `init_bignum`
whereas I expected the patch to *move* the call.
Was this call simply missing?


        Stefan




^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: master 2c8b09b06e7: Fix crash on Windows 9X
  2022-12-06  3:59   ` master 2c8b09b06e7: Fix crash on Windows 9X Stefan Monnier
@ 2022-12-06  4:56     ` Po Lu
  2022-12-06 12:21       ` Eli Zaretskii
  0 siblings, 1 reply; 10+ messages in thread
From: Po Lu @ 2022-12-06  4:56 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: emacs-devel

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> --- a/src/emacs.c
>> +++ b/src/emacs.c
>> @@ -1924,6 +1924,12 @@ Using an Emacs configured with --with-x-toolkit=lucid does not have this problem
>>  	 Vcoding_system_hash_table.  */
>>        syms_of_coding ();	/* This should be after syms_of_fileio.  */
>>        init_frame_once ();       /* Before init_window_once.  */
>> +      /* init_window_once calls make_initial_frame, which calls
>> +	 Fcurrent_time and bset_display_time, both of which allocate
>> +	 bignums.  Without the following call to init_bignums, crashes
>> +	 happen on Windows 9X after dumping when GC tries to free a
>> +	 pointer allocated on the system heap.  */
>> +      init_bignum ();
>>        init_window_once ();	/* Init the window system.  */
>>  #ifdef HAVE_WINDOW_SYSTEM
>>        init_fringe_once ();	/* Swap bitmaps if necessary.  */
>
> I feel like I'm missing something: this adds a call to `init_bignum`
> whereas I expected the patch to *move* the call.
> Was this call simply missing?
>
>
>         Stefan

No.  The call I added is only called before dumping, while the second is
called after dumping, AFAIK.



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: master 2c8b09b06e7: Fix crash on Windows 9X
  2022-12-06  4:56     ` Po Lu
@ 2022-12-06 12:21       ` Eli Zaretskii
  2022-12-06 12:51         ` Po Lu
  0 siblings, 1 reply; 10+ messages in thread
From: Eli Zaretskii @ 2022-12-06 12:21 UTC (permalink / raw)
  To: Po Lu; +Cc: monnier, emacs-devel

> From: Po Lu <luangruo@yahoo.com>
> Cc: emacs-devel@gnu.org
> Date: Tue, 06 Dec 2022 12:56:46 +0800
> 
> Stefan Monnier <monnier@iro.umontreal.ca> writes:
> 
> >> --- a/src/emacs.c
> >> +++ b/src/emacs.c
> >> @@ -1924,6 +1924,12 @@ Using an Emacs configured with --with-x-toolkit=lucid does not have this problem
> >>  	 Vcoding_system_hash_table.  */
> >>        syms_of_coding ();	/* This should be after syms_of_fileio.  */
> >>        init_frame_once ();       /* Before init_window_once.  */
> >> +      /* init_window_once calls make_initial_frame, which calls
> >> +	 Fcurrent_time and bset_display_time, both of which allocate
> >> +	 bignums.  Without the following call to init_bignums, crashes
> >> +	 happen on Windows 9X after dumping when GC tries to free a
> >> +	 pointer allocated on the system heap.  */
> >> +      init_bignum ();
> >>        init_window_once ();	/* Init the window system.  */
> >>  #ifdef HAVE_WINDOW_SYSTEM
> >>        init_fringe_once ();	/* Swap bitmaps if necessary.  */
> >
> > I feel like I'm missing something: this adds a call to `init_bignum`
> > whereas I expected the patch to *move* the call.
> > Was this call simply missing?
> >
> >
> >         Stefan
> 
> No.  The call I added is only called before dumping, while the second is
> called after dumping, AFAIK.

No, the second one is done both before and after dumping.  Only the first
call is conditioned.



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: master 2c8b09b06e7: Fix crash on Windows 9X
  2022-12-06 12:21       ` Eli Zaretskii
@ 2022-12-06 12:51         ` Po Lu
  2022-12-06 14:28           ` Eli Zaretskii
  0 siblings, 1 reply; 10+ messages in thread
From: Po Lu @ 2022-12-06 12:51 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: monnier, emacs-devel

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Po Lu <luangruo@yahoo.com>
>> Cc: emacs-devel@gnu.org
>> Date: Tue, 06 Dec 2022 12:56:46 +0800
>> 
>> Stefan Monnier <monnier@iro.umontreal.ca> writes:
>> 
>> >> --- a/src/emacs.c
>> >> +++ b/src/emacs.c
>> >> @@ -1924,6 +1924,12 @@ Using an Emacs configured with --with-x-toolkit=lucid does not have this problem
>> >>  	 Vcoding_system_hash_table.  */
>> >>        syms_of_coding ();	/* This should be after syms_of_fileio.  */
>> >>        init_frame_once ();       /* Before init_window_once.  */
>> >> +      /* init_window_once calls make_initial_frame, which calls
>> >> +	 Fcurrent_time and bset_display_time, both of which allocate
>> >> +	 bignums.  Without the following call to init_bignums, crashes
>> >> +	 happen on Windows 9X after dumping when GC tries to free a
>> >> +	 pointer allocated on the system heap.  */
>> >> +      init_bignum ();
>> >>        init_window_once ();	/* Init the window system.  */
>> >>  #ifdef HAVE_WINDOW_SYSTEM
>> >>        init_fringe_once ();	/* Swap bitmaps if necessary.  */
>> >
>> > I feel like I'm missing something: this adds a call to `init_bignum`
>> > whereas I expected the patch to *move* the call.
>> > Was this call simply missing?
>> >
>> >
>> >         Stefan
>> 
>> No.  The call I added is only called before dumping, while the second is
>> called after dumping, AFAIK.
>
> No, the second one is done both before and after dumping.  Only the first
> call is conditioned.

Ah, thanks for the clarification.  In any case, it must come before
init_window_once.  Does calling it twice hurt?



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: master 2c8b09b06e7: Fix crash on Windows 9X
  2022-12-06 12:51         ` Po Lu
@ 2022-12-06 14:28           ` Eli Zaretskii
  2022-12-07  0:58             ` Po Lu
  0 siblings, 1 reply; 10+ messages in thread
From: Eli Zaretskii @ 2022-12-06 14:28 UTC (permalink / raw)
  To: Po Lu; +Cc: monnier, emacs-devel

> From: Po Lu <luangruo@yahoo.com>
> Cc: monnier@iro.umontreal.ca,  emacs-devel@gnu.org
> Date: Tue, 06 Dec 2022 20:51:58 +0800
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> No.  The call I added is only called before dumping, while the second is
> >> called after dumping, AFAIK.
> >
> > No, the second one is done both before and after dumping.  Only the first
> > call is conditioned.
> 
> Ah, thanks for the clarification.  In any case, it must come before
> init_window_once.

init_window_once is called only if (!initialized), so this order is only
relevant for when dumping.

> Does calling it twice hurt?

It might, so I'd prefer not to risk such duplicate calls.  It should be easy
to make sure it is called only once when dumping for unexec, and only once
in the pdumper build (both when dumping and when not dumping).



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: master 2c8b09b06e7: Fix crash on Windows 9X
  2022-12-06 14:28           ` Eli Zaretskii
@ 2022-12-07  0:58             ` Po Lu
  2022-12-07 12:42               ` Eli Zaretskii
  0 siblings, 1 reply; 10+ messages in thread
From: Po Lu @ 2022-12-07  0:58 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: monnier, emacs-devel

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Po Lu <luangruo@yahoo.com>
>> Cc: monnier@iro.umontreal.ca,  emacs-devel@gnu.org
>> Date: Tue, 06 Dec 2022 20:51:58 +0800
>> 
>> Eli Zaretskii <eliz@gnu.org> writes:
>> 
>> >> No.  The call I added is only called before dumping, while the second is
>> >> called after dumping, AFAIK.
>> >
>> > No, the second one is done both before and after dumping.  Only the first
>> > call is conditioned.
>> 
>> Ah, thanks for the clarification.  In any case, it must come before
>> init_window_once.
>
> init_window_once is called only if (!initialized), so this order is only
> relevant for when dumping.
>
>> Does calling it twice hurt?
>
> It might, so I'd prefer not to risk such duplicate calls.  It should be easy
> to make sure it is called only once when dumping for unexec, and only once
> in the pdumper build (both when dumping and when not dumping).

Something like this?

diff --git a/src/emacs.c b/src/emacs.c
index d8a2863fd9c..f0d20f8eb8c 100644
--- a/src/emacs.c
+++ b/src/emacs.c
@@ -1937,7 +1937,10 @@ main (int argc, char **argv)
     }
 
   init_alloc ();
-  init_bignum ();
+#ifndef HAVE_UNEXEC
+  if (!initialized)
+    init_bignum ();
+#endif
   init_threads ();
   init_eval ();
   running_asynch_code = 0;



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: master 2c8b09b06e7: Fix crash on Windows 9X
  2022-12-07  0:58             ` Po Lu
@ 2022-12-07 12:42               ` Eli Zaretskii
  2022-12-07 13:13                 ` Po Lu
  0 siblings, 1 reply; 10+ messages in thread
From: Eli Zaretskii @ 2022-12-07 12:42 UTC (permalink / raw)
  To: Po Lu; +Cc: monnier, emacs-devel

> From: Po Lu <luangruo@yahoo.com>
> Cc: monnier@iro.umontreal.ca,  emacs-devel@gnu.org
> Date: Wed, 07 Dec 2022 08:58:06 +0800
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > It might, so I'd prefer not to risk such duplicate calls.  It should be easy
> > to make sure it is called only once when dumping for unexec, and only once
> > in the pdumper build (both when dumping and when not dumping).
> 
> Something like this?

Without the "if (!initialized)" part, since in the pdumper build we
want it invoked both when dumping and when starting after dumping.



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: master 2c8b09b06e7: Fix crash on Windows 9X
  2022-12-07 12:42               ` Eli Zaretskii
@ 2022-12-07 13:13                 ` Po Lu
  2022-12-07 14:02                   ` Eli Zaretskii
  0 siblings, 1 reply; 10+ messages in thread
From: Po Lu @ 2022-12-07 13:13 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: monnier, emacs-devel

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Po Lu <luangruo@yahoo.com>
>> Cc: monnier@iro.umontreal.ca,  emacs-devel@gnu.org
>> Date: Wed, 07 Dec 2022 08:58:06 +0800
>> 
>> Eli Zaretskii <eliz@gnu.org> writes:
>> 
>> > It might, so I'd prefer not to risk such duplicate calls.  It should be easy
>> > to make sure it is called only once when dumping for unexec, and only once
>> > in the pdumper build (both when dumping and when not dumping).
>> 
>> Something like this?
>
> Without the "if (!initialized)" part, since in the pdumper build we
> want it invoked both when dumping and when starting after dumping.

So, just like this?  Thanks; now comes the question of how to install
this on the release branch without causing conflicts with master, as
part of the change is already on master.

diff --git a/src/emacs.c b/src/emacs.c
index d8a2863fd9c..2d924da1cdb 100644
--- a/src/emacs.c
+++ b/src/emacs.c
@@ -1937,7 +1937,9 @@ main (int argc, char **argv)
     }
 
   init_alloc ();
+#ifndef HAVE_UNEXEC
   init_bignum ();
+#endif
   init_threads ();
   init_eval ();
   running_asynch_code = 0;



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: master 2c8b09b06e7: Fix crash on Windows 9X
  2022-12-07 13:13                 ` Po Lu
@ 2022-12-07 14:02                   ` Eli Zaretskii
  2022-12-08  0:47                     ` Po Lu
  0 siblings, 1 reply; 10+ messages in thread
From: Eli Zaretskii @ 2022-12-07 14:02 UTC (permalink / raw)
  To: Po Lu; +Cc: monnier, emacs-devel

> From: Po Lu <luangruo@yahoo.com>
> Cc: monnier@iro.umontreal.ca,  emacs-devel@gnu.org
> Date: Wed, 07 Dec 2022 21:13:51 +0800
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > Without the "if (!initialized)" part, since in the pdumper build we
> > want it invoked both when dumping and when starting after dumping.
> 
> So, just like this?

Yes.

> Thanks; now comes the question of how to install this on the release
> branch without causing conflicts with master, as part of the change
> is already on master.

Cherry-pick the change you installed on master to emacs-29, and then
install this additional change on emacs-29.



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: master 2c8b09b06e7: Fix crash on Windows 9X
  2022-12-07 14:02                   ` Eli Zaretskii
@ 2022-12-08  0:47                     ` Po Lu
  0 siblings, 0 replies; 10+ messages in thread
From: Po Lu @ 2022-12-08  0:47 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: monnier, emacs-devel

Eli Zaretskii <eliz@gnu.org> writes:

> Cherry-pick the change you installed on master to emacs-29, and then
> install this additional change on emacs-29.

Ok, thanks.



^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2022-12-08  0:47 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <167029029523.21453.12133435240921985505@vcs2.savannah.gnu.org>
     [not found] ` <20221206013135.E2E2DC004BE@vcs2.savannah.gnu.org>
2022-12-06  3:59   ` master 2c8b09b06e7: Fix crash on Windows 9X Stefan Monnier
2022-12-06  4:56     ` Po Lu
2022-12-06 12:21       ` Eli Zaretskii
2022-12-06 12:51         ` Po Lu
2022-12-06 14:28           ` Eli Zaretskii
2022-12-07  0:58             ` Po Lu
2022-12-07 12:42               ` Eli Zaretskii
2022-12-07 13:13                 ` Po Lu
2022-12-07 14:02                   ` Eli Zaretskii
2022-12-08  0:47                     ` Po Lu

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).