Re: [COMMIT] Don't blow the stack when printing recursive spec-lists, print_specifier()

Ar an triú lá déag de mí na Samhain, scríobh Raymond Toy: > Run xemacs -vanilla and using the menu, select > Options->Customize->Emacs->Emacs. This crashes xemacs because of default > widget set (no athena installed). gdb backtrace appended below. Thanks, Raymond! I can’t reproduce, probably because I do have the Athena widgets installed and when I try to lead the configure script away from them it picks up Motif, but it’s clear enough that we shouldn’t crash when printing a recursive specifier.

APPROVE COMMIT NOTE: This patch has been committed # HG changeset patch # User Aidan Kehoe <kehoea(a)parhasard.net&gt; # Date 1510706657 0 # Wed Nov 15 00:44:17 2017 +0000 # Node ID 1a2f697b59a7e7443133636304f4dd62c5303120 # Parent 00726cf7425524d7ad434590c8594ae05ee4eda4 Don't blow the stack when printing recursive spec-lists, print_specifier() src/ChangeLog addition: 2017-11-15 Aidan Kehoe <kehoea(a)parhasard.net&gt; * specifier.c (print_specifier): Cons less when printing the spec-list of a specifier, avoiding stack overflow with recursive specifiers due to the COPY_TREE_P argument passed by #'specifier-specs to specifier_get_external_inst_list(). Thank you for the bug report, Raymond Toy! diff -r 00726cf74255 -r 1a2f697b59a7 src/ChangeLog --- a/src/ChangeLog Tue Nov 14 22:13:17 2017 +0000 +++ b/src/ChangeLog Wed Nov 15 00:44:17 2017 +0000 ＠＠ -1,3 +1,12 ＠＠ +2017-11-15 Aidan Kehoe <kehoea(a)parhasard.net&gt; + + * specifier.c (print_specifier): + Cons less when printing the spec-list of a specifier, avoiding + stack overflow with recursive specifiers due to the COPY_TREE_P + argument passed by #'specifier-specs to + specifier_get_external_inst_list(). + Thank you for the bug report, Raymond Toy! + 2017-11-13 Aidan Kehoe <kehoea(a)parhasard.net&gt; * bytecode.c (bytecode_arithcompare): diff -r 00726cf74255 -r 1a2f697b59a7 src/specifier.c --- a/src/specifier.c Tue Nov 14 22:13:17 2017 +0000 +++ b/src/specifier.c Wed Nov 15 00:44:17 2017 +0000 ＠＠ -269,12 +269,18 ＠＠ } } +static Lisp_Object specifier_get_external_inst_list (Lisp_Object specifier, + Lisp_Object locale, + enum spec_locale_type, + Lisp_Object tag_set, + int exact_p, + int short_p, + int copy_tree_p); + static void -print_specifier (Lisp_Object obj, Lisp_Object printcharfun, - int UNUSED (escapeflag)) +print_specifier (Lisp_Object obj, Lisp_Object printcharfun, int escapeflag) { Lisp_Specifier *sp = XSPECIFIER (obj); - int count = specpdl_depth (); Lisp_Object the_specs; if (print_readably) ＠＠ -289,17 +295,36 ＠＠ specbind (Qprint_string_length, make_fixnum (100)); specbind (Qprint_length, make_fixnum (5)); #endif - the_specs = Fspecifier_specs (obj, Qglobal, Qnil, Qnil); + + /* This is preferable to calling Fspecifier_specs() because we can specify + COPY_TREE_P as zero, avoiding copying possibly-recursive structures. See + Raymond Toye's bug report of + https://mid.xemacs.org/CAG14z1F7b90rt22P-XXjk+ 0qT42DrS6E=OAcUSekqoPsQnD9aw(a)mail.gmail.com + + specifier_get_external_inst_list() does cons, even with a zero + COPY_TREE_P, but that's OK, in exchange for that we get the short form + when appropriate, and the instance list has been checked for circularity + when it was specified initially, we won't blow the stack with it. */ + the_specs = specifier_get_external_inst_list (obj, Qglobal, LOCALE_GLOBAL, + Qnil, 0, 1, 0); if (NILP (the_specs)) - /* there are no global specs */ - write_ascstring (printcharfun, "<unspecified>"); + { + /* there are no global specs */ + write_ascstring (printcharfun, "<unspecified>"); + } else - print_internal (the_specs, printcharfun, 1); + { + /* print_internal() does its own circularity checking, we don't have + to. */ + print_internal (the_specs, printcharfun, escapeflag); + } + if (!NILP (sp->fallback)) { - write_fmt_string_lisp (printcharfun, " fallback=%S", sp->fallback); + write_ascstring (printcharfun, " fallback="); + print_internal (sp->fallback, printcharfun, escapeflag); } - unbind_to (count); + write_fmt_string (printcharfun, " 0x%x>", LISP_OBJECT_UID (obj)); } -- ‘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)