From f1550f70a1e139022e6238c3c5c22cb7bd6923a1 Mon Sep 17 00:00:00 2001 From: Paul Eggert Date: Tue, 2 May 2017 14:52:21 -0700 Subject: [PATCH] Check list object type if --enable-gcc-warnings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * configure.ac (--enable-check-lisp-object-type): Default to "yes" if --enable-gcc-warnings is not "no". * etc/NEWS: Mention this. * src/eval.c (internal_lisp_condition_case): Fix some glitches with 'volatile' uncovered by the above: in particular, 'clauses' should be a pointer to volatile storage on the stack, and need not be volatile itself. Use an int, not ptrdiff_t, to count clauses. Don’t bother gathering binding count if VAR is nil. Use more-specific local names to try to clarify what’s going on. --- configure.ac | 25 +++++++------- etc/NEWS | 5 ++- src/eval.c | 105 ++++++++++++++++++++++++++++++----------------------------- 3 files changed, 69 insertions(+), 66 deletions(-) diff --git a/configure.ac b/configure.ac index 45cfdfc..d6b8001 100644 --- a/configure.ac +++ b/configure.ac @@ -507,16 +507,6 @@ AC_DEFUN [Define this to enable glyphs debugging code.]) fi -AC_ARG_ENABLE(check-lisp-object-type, -[AS_HELP_STRING([--enable-check-lisp-object-type], - [enable compile time checks for the Lisp_Object data type. - This is useful for development for catching certain types of bugs.])], -if test "${enableval}" != "no"; then - AC_DEFINE(CHECK_LISP_OBJECT_TYPE, 1, - [Define this to enable compile time checks for the Lisp_Object data type.]) -fi) - - dnl The name of this option is unfortunate. It predates, and has no dnl relation to, the "sampling-based elisp profiler" added in 24.3. dnl Actually, it stops it working. @@ -877,9 +867,18 @@ AC_DEFUN # just a release imported into Git for patch management. gl_gcc_warnings=no if test -e "$srcdir"/.git && test ! -f "$srcdir"/.tarball-version; then - gl_GCC_VERSION_IFELSE([5], [3], [gl_gcc_warnings=warn-only])] - fi -) + gl_GCC_VERSION_IFELSE([5], [3], [gl_gcc_warnings=warn-only]) + fi]) + +AC_ARG_ENABLE([check-lisp-object-type], + [AS_HELP_STRING([--enable-check-lisp-object-type], + [Enable compile-time checks for the Lisp_Object data type, + which can catch some bugs during development. + The default is "no" if --enable-gcc-warnings is "no".])]) +if test "${enable_check_lisp_object_type-$gl_gcc_warnings}" != "no"; then + AC_DEFINE([CHECK_LISP_OBJECT_TYPE], 1, + [Define to enable compile-time checks for the Lisp_Object data type.]) +fi # clang is unduly picky about some things. AC_CACHE_CHECK([whether the compiler is clang], [emacs_cv_clang], diff --git a/etc/NEWS b/etc/NEWS index 173c4e4..410e681 100644 --- a/etc/NEWS +++ b/etc/NEWS @@ -42,6 +42,9 @@ now the default in developer builds. As before, use '--disable-gcc-warnings' to suppress GCC's warnings, and '--enable-gcc-warnings' to stop the build if GCC issues warnings. +** When GCC warnings are enabled, '--enable-check-lisp-object-type' is +now enabled by default when configuring. + +++ ** The Emacs server now has socket-launching support. This allows socket based activation, where an external process like systemd can @@ -393,7 +396,7 @@ procedure and therefore obeys saving hooks. * Changes in Specialized Modes and Packages in Emacs 26.1 *** Info menu and index completion uses substring completion by default. -This can be customized via the `info-menu` category in +This can be customized via the info-menu category in completion-category-override. +++ diff --git a/src/eval.c b/src/eval.c index af0912f..0030271 100644 --- a/src/eval.c +++ b/src/eval.c @@ -1225,18 +1225,17 @@ usage: (condition-case VAR BODYFORM &rest HANDLERS) */) rather than passed in a list. Used by Fbyte_code. */ Lisp_Object -internal_lisp_condition_case (volatile Lisp_Object var, Lisp_Object bodyform, +internal_lisp_condition_case (Lisp_Object var, Lisp_Object bodyform, Lisp_Object handlers) { - Lisp_Object val; struct handler *oldhandlerlist = handlerlist; - int clausenb = 0; + ptrdiff_t clausenb = 0; CHECK_SYMBOL (var); - for (val = handlers; CONSP (val); val = XCDR (val)) + for (Lisp_Object tail = handlers; CONSP (tail); tail = XCDR (tail)) { - Lisp_Object tem = XCAR (val); + Lisp_Object tem = XCAR (tail); clausenb++; if (! (NILP (tem) || (CONSP (tem) @@ -1246,55 +1245,57 @@ internal_lisp_condition_case (volatile Lisp_Object var, Lisp_Object bodyform, SDATA (Fprin1_to_string (tem, Qt))); } - { /* The first clause is the one that should be checked first, so it should - be added to handlerlist last. So we build in `clauses' a table that - contains `handlers' but in reverse order. SAFE_ALLOCA won't work - here due to the setjmp, so impose a MAX_ALLOCA limit. */ - if (MAX_ALLOCA / word_size < clausenb) - memory_full (SIZE_MAX); - Lisp_Object *clauses = alloca (clausenb * sizeof *clauses); - Lisp_Object *volatile clauses_volatile = clauses; - int i = clausenb; - for (val = handlers; CONSP (val); val = XCDR (val)) - clauses[--i] = XCAR (val); - for (i = 0; i < clausenb; i++) - { - Lisp_Object clause = clauses[i]; - Lisp_Object condition = CONSP (clause) ? XCAR (clause) : Qnil; - if (!CONSP (condition)) - condition = Fcons (condition, Qnil); - struct handler *c = push_handler (condition, CONDITION_CASE); - if (sys_setjmp (c->jmp)) - { - ptrdiff_t count = SPECPDL_INDEX (); - Lisp_Object val = handlerlist->val; - Lisp_Object *chosen_clause = clauses_volatile; - for (c = handlerlist->next; c != oldhandlerlist; c = c->next) - chosen_clause++; - handlerlist = oldhandlerlist; - if (!NILP (var)) - { - if (!NILP (Vinternal_interpreter_environment)) - specbind (Qinternal_interpreter_environment, - Fcons (Fcons (var, val), - Vinternal_interpreter_environment)); - else - specbind (var, val); - } - val = Fprogn (XCDR (*chosen_clause)); - /* Note that this just undoes the binding of var; whoever - longjumped to us unwound the stack to c.pdlcount before - throwing. */ - if (!NILP (var)) - unbind_to (count, Qnil); - return val; - } - } - } + /* The first clause is the one that should be checked first, so it + should be added to handlerlist last. So build in CLAUSES a table + that contains HANDLERS but in reverse order. CLAUSES is pointer + to volatile to avoid issues with setjmp and local storage. + SAFE_ALLOCA won't work here due to the setjmp, so impose a + MAX_ALLOCA limit. */ + if (MAX_ALLOCA / word_size < clausenb) + memory_full (SIZE_MAX); + Lisp_Object volatile *clauses = alloca (clausenb * sizeof *clauses); + clauses += clausenb; + for (Lisp_Object tail = handlers; CONSP (tail); tail = XCDR (tail)) + *--clauses = XCAR (tail); + for (ptrdiff_t i = 0; i < clausenb; i++) + { + Lisp_Object clause = clauses[i]; + Lisp_Object condition = CONSP (clause) ? XCAR (clause) : Qnil; + if (!CONSP (condition)) + condition = list1 (condition); + struct handler *c = push_handler (condition, CONDITION_CASE); + if (sys_setjmp (c->jmp)) + { + Lisp_Object val = handlerlist->val; + Lisp_Object volatile *chosen_clause = clauses; + for (struct handler *h = handlerlist->next; h != oldhandlerlist; + h = h->next) + chosen_clause++; + Lisp_Object handler_body = XCDR (*chosen_clause); + handlerlist = oldhandlerlist; + + if (NILP (var)) + return Fprogn (handler_body); + + if (!NILP (Vinternal_interpreter_environment)) + { + val = Fcons (Fcons (var, val), + Vinternal_interpreter_environment); + var = Qinternal_interpreter_environment; + } + + /* Bind VAR to VAL while evaluating HANDLER_BODY. The + unbind_to just undoes VAR's binding; whoever longjumped + to us unwound the stack to C->pdlcount before throwing. */ + ptrdiff_t count = SPECPDL_INDEX (); + specbind (var, val); + return unbind_to (count, Fprogn (handler_body)); + } + } - val = eval_sub (bodyform); + Lisp_Object result = eval_sub (bodyform); handlerlist = oldhandlerlist; - return val; + return result; } /* Call the function BFUN with no arguments, catching errors within it -- 2.9.3