[COMMIT] Fix an intermittent bug in error_or_quit_failed_instantiator_in_domain().

This manifested itself as intermittent crashes in the presence of glyphs that XEmacs had trouble converting into images. The crux of the issue was that a value that should have been on the Vall_weak_lists weak object chain wasn’t, and so prune_weak_lists() missed it. A more elegant approach to this, and an approach that would remove build-time C-function order dependencies, would be to have Qnull_pointer represent the end of one of these object chains, rather than Qnil as we do now. Then we wouldn’t need a special initialisation in the syms_of_*() or vars_of_*() functions, though we would still need dump_add_weak_object_chain() calls for the sake of pdump. But the current approach will flush out the bug above going forward. APPROVE COMMIT NOTE: This patch has been committed. # HG changeset patch # User Aidan Kehoe <kehoea(a)parhasard.net&gt; # Date 1534018142 -3600 # Sat Aug 11 21:09:02 2018 +0100 # Node ID 4280f7a3c20943225b9761f20d152881fc4c3c95 # Parent 881402f49f9cc477816cc360a8cbee04b631f4b9 Fix an intermittent bug in error_or_quit_failed_instantiator_in_domain(). src/ChangeLog addition: 2018-08-11 Aidan Kehoe <kehoea(a)parhasard.net&gt; * emacs.c (main_1): vars_of_specifier() uses the weak list infrastructure, and so it needs to be called after vars_of_data() so that the Vall_weak_lists object chain is initialised correctly. * lrecord.h: * lrecord.h (DUMP_ADD_WEAK_OBJECT_CHAIN): New #define, check that an object chain variable is the NULL pointer (as will be the case at early startup if it has not yet been used), then initialise it to Qnil and call dump_add_weak_object_chain(&var). Avoids the bug just fixed in main_1(). * chartab.c (vars_of_chartab): Use it. * data.c: Make Vall_weak_lists available to elhash.c, which has some code that needs to modify it very early. * data.c (vars_of_data): Use it. * elhash.c: * elhash.c (syms_of_elhash): Call DUMP_ADD_WEAK_OBJECT_CHAIN() as appropriate. Set Vall_weak_lists Qnull_pointer after creating a weak list; the initial value will be corrupt. * specifier.c (vars_of_specifier): Call DUMP_ADD_WEAK_OBJECT_CHAIN() here. diff -r 881402f49f9c -r 4280f7a3c209 src/ChangeLog --- a/src/ChangeLog Thu Aug 09 23:24:28 2018 +0100 +++ b/src/ChangeLog Sat Aug 11 21:09:02 2018 +0100 ＠＠ -5,6 +5,34 ＠＠ debug build. Non-debug builds had a performance issue, not a correctness issue. +2018-08-11 Aidan Kehoe <kehoea(a)parhasard.net&gt; + + * emacs.c (main_1): + vars_of_specifier() uses the weak list infrastructure, and so it + needs to be called after vars_of_data() so that the Vall_weak_lists + object chain is initialised correctly. + * lrecord.h: + * lrecord.h (DUMP_ADD_WEAK_OBJECT_CHAIN): + New #define, check that an object chain variable is the NULL + pointer (as will be the case at early startup if it has not yet + been used), then initialise it to Qnil and call + dump_add_weak_object_chain(&var). Avoids the bug just fixed in + main_1(). + * chartab.c (vars_of_chartab): + Use it. + * data.c: + Make Vall_weak_lists available to elhash.c, which has some code + that needs to modify it very early. + * data.c (vars_of_data): + Use it. + * elhash.c: + * elhash.c (syms_of_elhash): + Call DUMP_ADD_WEAK_OBJECT_CHAIN() as appropriate. Set + Vall_weak_lists Qnull_pointer after creating a weak list; the + initial value will be corrupt. + * specifier.c (vars_of_specifier): + Call DUMP_ADD_WEAK_OBJECT_CHAIN() here. + 2018-07-11 Aidan Kehoe <kehoea(a)parhasard.net&gt; * data.c: diff -r 881402f49f9c -r 4280f7a3c209 src/chartab.c --- a/src/chartab.c Thu Aug 09 23:24:28 2018 +0100 +++ b/src/chartab.c Sat Aug 11 21:09:02 2018 +0100 ＠＠ -2523,8 +2523,7 ＠＠ #endif /* MEMORY_USAGE_STATS */ /* DO NOT staticpro this. It works just like Vweak_hash_tables. */ - Vall_syntax_tables = Qnil; - dump_add_weak_object_chain (&Vall_syntax_tables); + DUMP_ADD_WEAK_OBJECT_CHAIN (Vall_syntax_tables); /* The value at level 4 is not 2^32 - 1. With 32-bit EMACS_INTs, it's 2^30 - 1 because characters are only 30 bits wide. */ diff -r 881402f49f9c -r 4280f7a3c209 src/data.c --- a/src/data.c Thu Aug 09 23:24:28 2018 +0100 +++ b/src/data.c Sat Aug 11 21:09:02 2018 +0100 ＠＠ -3193,7 +3193,7 ＠＠ remove them. This is analogous to weak hash tables; see the explanation there for more info. */ -static Lisp_Object Vall_weak_lists; /* Gemarke es nicht!!! */ +Lisp_Object Vall_weak_lists; /* Gemarke es nicht!!! */ static Lisp_Object encode_weak_list_type (enum weak_list_type type); ＠＠ -4475,18 +4475,14 ＠＠ void vars_of_data (void) { - /* This must not be staticpro'd */ - Vall_weak_lists = Qnil; - dump_add_weak_object_chain (&Vall_weak_lists); - - Vall_ephemerons = Qnil; - dump_add_weak_object_chain (&Vall_ephemerons); + DUMP_ADD_WEAK_OBJECT_CHAIN (Vall_weak_lists); + + DUMP_ADD_WEAK_OBJECT_CHAIN (Vall_ephemerons); Vfinalize_list = Qnil; staticpro (&Vfinalize_list); - Vall_weak_boxes = Qnil; - dump_add_weak_object_chain (&Vall_weak_boxes); + DUMP_ADD_WEAK_OBJECT_CHAIN (Vall_weak_boxes); DEFVAR_CONST_INT ("most-negative-fixnum", &Vmost_negative_fixnum /* The fixnum closest in value to negative infinity. diff -r 881402f49f9c -r 4280f7a3c209 src/elhash.c --- a/src/elhash.c Thu Aug 09 23:24:28 2018 +0100 +++ b/src/elhash.c Sat Aug 11 21:09:02 2018 +0100 ＠＠ -2832,6 +2832,8 ＠＠ OBJECT_HAS_METHOD (hash_table, nsubst_structures_descend); } +extern Lisp_Object Vall_weak_lists; + void syms_of_elhash (void) { ＠＠ -2840,11 +2842,14 ＠＠ xhash_table (Vobarray)->next_weak = Qunbound; /* This must NOT be staticpro'd */ - Vall_weak_hash_tables = Qnil; - dump_add_weak_object_chain (&Vall_weak_hash_tables); + DUMP_ADD_WEAK_OBJECT_CHAIN (Vall_weak_hash_tables); staticpro (&Vhash_table_test_weak_list); Vhash_table_test_weak_list = make_weak_list (WEAK_LIST_KEY_ASSOC); + /* syms_of_elhash() is called *very* early, don't confuse the weak list code + in data.c. See also the other code to correct this in + vars_of_elhash(). */ + Vall_weak_lists = Qnull_pointer; DEFSYMBOL (Qeq); DEFSYMBOL (Qeql); diff -r 881402f49f9c -r 4280f7a3c209 src/emacs.c --- a/src/emacs.c Thu Aug 09 23:24:28 2018 +0100 +++ b/src/emacs.c Sat Aug 11 21:09:02 2018 +0100 ＠＠ -2067,9 +2067,6 ＠＠ /* Now allow Fprovide() statements to be made. */ init_provide_once (); - /* Do that before any specifier creation (esp. vars_of_glyphs()) */ - vars_of_specifier (); - vars_of_abbrev (); vars_of_alloc (); vars_of_buffer (); ＠＠ -2081,6 +2078,8 ＠＠ vars_of_cmds (); vars_of_console (); vars_of_data (); + vars_of_specifier (); + #ifdef DEBUG_XEMACS vars_of_debug (); vars_of_tests (); diff -r 881402f49f9c -r 4280f7a3c209 src/lrecord.h --- a/src/lrecord.h Thu Aug 09 23:24:28 2018 +0100 +++ b/src/lrecord.h Sat Aug 11 21:09:02 2018 +0100 ＠＠ -2223,6 +2223,13 ＠＠ #define dump_add_weak_object_chain(varaddr) DO_NOTHING #endif +#define DUMP_ADD_WEAK_OBJECT_CHAIN(var) do { \ + /* Don't add to the chain before marking it for dumping! */ \ + gc_checking_assert (EQ (var, Qnull_pointer)); \ + var = Qnil; \ + dump_add_weak_object_chain (&var); \ + } while (0) + /* Nonzero means Emacs has already been initialized. Used during startup to detect startup of dumped Emacs. */ extern MODULE_API int initialized; diff -r 881402f49f9c -r 4280f7a3c209 src/specifier.c --- a/src/specifier.c Thu Aug 09 23:24:28 2018 +0100 +++ b/src/specifier.c Sat Aug 11 21:09:02 2018 +0100 ＠＠ -4017,8 +4017,7 ＠＠ /* Do NOT mark through this, or specifiers will never be GC'd. This is the same deal as for weak hash tables. */ - Vall_specifiers = Qnil; - dump_add_weak_object_chain (&Vall_specifiers); + DUMP_ADD_WEAK_OBJECT_CHAIN (Vall_specifiers); Vuser_defined_tags = Qnil; staticpro (&Vuser_defined_tags); -- ‘As I sat looking up at the Guinness ad, I could never figure out / How your man stayed up on the surfboard after forty pints of stout’ (C. Moore)