* inlinable functions instead of macros @ 2012-08-17 23:48 Stefan Monnier 2012-08-17 23:59 ` Paul Eggert ` (3 more replies) 0 siblings, 4 replies; 34+ messages in thread From: Stefan Monnier @ 2012-08-17 23:48 UTC (permalink / raw) To: emacs-devel While inlinable functions are much cleaner than macros, they have a very serious downside: you just end up with lisp.h:2416: Emacs fatal error: assertion failed: found == !EQ (blv->defcell, blv->valcell) I.e. the file&line info is always the same rather than giving the file&line where the inlinable function was called. Stefan ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: inlinable functions instead of macros 2012-08-17 23:48 inlinable functions instead of macros Stefan Monnier @ 2012-08-17 23:59 ` Paul Eggert 2012-08-18 22:15 ` Daniel Colascione 2012-08-21 17:50 ` Stefan Monnier 2012-08-18 19:08 ` Richard Stallman ` (2 subsequent siblings) 3 siblings, 2 replies; 34+ messages in thread From: Paul Eggert @ 2012-08-17 23:59 UTC (permalink / raw) To: Stefan Monnier; +Cc: emacs-devel On 08/17/2012 04:48 PM, Stefan Monnier wrote: > While inlinable functions are much cleaner than macros, they have > a very serious downside: you just end up with > > lisp.h:2416: Emacs fatal error: assertion failed: found == !EQ (blv->defcell, blv->valcell) > > I.e. the file&line info is always the same rather than giving the > file&line where the inlinable function was called. On systems that use glibc we could adjust eassert so that it also prints a backtrace, using glibc's 'backtrace' function. See <http://www.gnu.org/software/libc/manual/html_node/Backtraces.html>. This would not be quite the same thing, as it would print function names, insn offsets, and return addresses; but it would recapture some of the ground lost here, and the backtrace info would in some cases be more useful than what we have now. ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: inlinable functions instead of macros 2012-08-17 23:59 ` Paul Eggert @ 2012-08-18 22:15 ` Daniel Colascione 2012-08-18 22:55 ` John Yates 2012-08-21 17:50 ` Stefan Monnier 1 sibling, 1 reply; 34+ messages in thread From: Daniel Colascione @ 2012-08-18 22:15 UTC (permalink / raw) To: Paul Eggert; +Cc: Stefan Monnier, emacs-devel [-- Attachment #1: Type: text/plain, Size: 1465 bytes --] On 8/17/12 4:59 PM, Paul Eggert wrote: > On 08/17/2012 04:48 PM, Stefan Monnier wrote: >> While inlinable functions are much cleaner than macros, they have >> a very serious downside: you just end up with >> >> lisp.h:2416: Emacs fatal error: assertion failed: found == !EQ (blv->defcell, blv->valcell) >> >> I.e. the file&line info is always the same rather than giving the >> file&line where the inlinable function was called. > > On systems that use glibc we could adjust eassert so that it > also prints a backtrace, using glibc's 'backtrace' function. See > <http://www.gnu.org/software/libc/manual/html_node/Backtraces.html>. > This would not be quite the same thing, as it would print function > names, insn offsets, and return addresses; but it would recapture > some of the ground lost here, and the backtrace info would in some > cases be more useful than what we have now. On Windows, we solved this problem a long time ago by teaching the operating system itself how to walk stack frames. When a program crashes, the OS captures a minidump containing the faulting stack, instruction pointer, register contents, and (optionally) things like data segments. If you have one of these minidumps, you don't need particularly good assertion-failure output: you have everything you need in the minidump, which you can load into the debugger and examine. I hear Google's breakpad provides similar facilities for other systems. [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 235 bytes --] ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: inlinable functions instead of macros 2012-08-18 22:15 ` Daniel Colascione @ 2012-08-18 22:55 ` John Yates 2012-08-19 4:22 ` Richard Stallman 0 siblings, 1 reply; 34+ messages in thread From: John Yates @ 2012-08-18 22:55 UTC (permalink / raw) To: Daniel Colascione; +Cc: Paul Eggert, Stefan Monnier, emacs-devel On Sat, Aug 18, 2012 at 6:15 PM, Daniel Colascione <dancol@dancol.org> wrote: > > On Windows, we solved this problem a long time ago by teaching the > operating system itself how to walk stack frames. When a program > crashes, the OS captures a minidump containing the faulting stack, > instruction pointer, register contents, and (optionally) things like > data segments. If you have one of these minidumps, you don't need > particularly good assertion-failure output: you have everything you > need in the minidump, which you can load into the debugger and examine. And even before that Vax/VMS solved the problem by capturing a stack trace every time a signal was propagated (including of course ACCVIO or access violation, the equivalent of SEGV). A shell setting controlled whether the trace was displayed by default of not. Even if one choose not to display it by default it was always available by explicit request. VMS coding conventions encouraged failure notification via signalling and required a signal handler to use a subsystem specific status when propagating a signal. This lead to very informative stack traces consisting of call sites interspersed with local statuses showing how failure was interpreted and reported as it propagated up the stack. /john ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: inlinable functions instead of macros 2012-08-18 22:55 ` John Yates @ 2012-08-19 4:22 ` Richard Stallman 0 siblings, 0 replies; 34+ messages in thread From: Richard Stallman @ 2012-08-19 4:22 UTC (permalink / raw) To: John Yates; +Cc: dancol, emacs-devel, monnier, eggert I think the Unix solution for this was to dump core so you could examine everything in a debugger. -- Dr Richard Stallman President, Free Software Foundation 51 Franklin St Boston MA 02110 USA www.fsf.org www.gnu.org Skype: No way! That's nonfree (freedom-denying) software. Use Ekiga or an ordinary phone call ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: inlinable functions instead of macros 2012-08-17 23:59 ` Paul Eggert 2012-08-18 22:15 ` Daniel Colascione @ 2012-08-21 17:50 ` Stefan Monnier 2012-08-22 9:24 ` C backtraces for Emacs Paul Eggert 2012-08-22 9:28 ` inlinable functions instead of macros Paul Eggert 1 sibling, 2 replies; 34+ messages in thread From: Stefan Monnier @ 2012-08-21 17:50 UTC (permalink / raw) To: Paul Eggert; +Cc: emacs-devel Could some please check our inlinable functions and move any easserts in them to a macro (i.e. rename the function, replace the old name with a macro that does the assert and calls the inlinable function)? If someone wants to *additionally* provide a patch to use glibc's `backtrace' in our `eassert' that's fine, but the two are separate issues. Stefan ^ permalink raw reply [flat|nested] 34+ messages in thread
* C backtraces for Emacs 2012-08-21 17:50 ` Stefan Monnier @ 2012-08-22 9:24 ` Paul Eggert 2012-08-22 16:01 ` Eli Zaretskii 2012-08-24 10:48 ` Paul Eggert 2012-08-22 9:28 ` inlinable functions instead of macros Paul Eggert 1 sibling, 2 replies; 34+ messages in thread From: Paul Eggert @ 2012-08-22 9:24 UTC (permalink / raw) To: Stefan Monnier; +Cc: Juanma Barranquero, Eli Zaretskii, emacs-devel On 08/21/2012 10:50 AM, Stefan Monnier wrote: > If someone wants to *additionally* provide a patch to use glibc's > `backtrace' in our `eassert' that's fine I've prepared a patch to do this additional work, enclosed below. The key part is at the end; the rest is boilerplate for non-GNUish systems. I've tested this on Fedora 15, Solaris 11, and Solaris 10. I'll hold off installing this to give Eli and Juanma a heads-up, as it creates a new substitute header <execinfo.h> on non-GNUish systems, and the Windows build will need to do something similar. If I understand things correctly Windows already does backtraces in a different way, so its execinfo.h can just be a copy of execinfo.in.h. === modified file 'ChangeLog' --- ChangeLog 2012-08-22 06:55:44 +0000 +++ ChangeLog 2012-08-22 08:59:10 +0000 @@ -1,3 +1,11 @@ +2012-08-22 Paul Eggert <eggert@cs.ucla.edu> + + On assertion failure, print backtrace if available. + Merge from gnulib, incorporating: + 2012-08-22 execinfo: new module + * lib/gnulib.mk, m4/gnulib-comp.m4: Regenerate. + * lib/execinfo.c, lib/execinfo.in.h, m4/execinfo.m4: New files. + 2012-08-22 Glenn Morris <rgm@gnu.org> * Makefile.in (install-arch-dep): If NO_BIN_LINK is non-null, === modified file 'admin/ChangeLog' --- admin/ChangeLog 2012-08-16 21:58:44 +0000 +++ admin/ChangeLog 2012-08-22 08:59:10 +0000 @@ -1,3 +1,8 @@ +2012-08-22 Paul Eggert <eggert@cs.ucla.edu> + + On assertion failure, print backtrace if available. + * merge-gnulib (GNULIB_MODULES): Add execinfo. + 2012-08-16 Paul Eggert <eggert@cs.ucla.edu> Use ASCII tests for character types. === modified file 'admin/merge-gnulib' --- admin/merge-gnulib 2012-08-16 21:58:44 +0000 +++ admin/merge-gnulib 2012-08-22 08:59:10 +0000 @@ -28,7 +28,7 @@ GNULIB_MODULES=' alloca-opt c-ctype c-strcase careadlinkat crypto/md5 crypto/sha1 crypto/sha256 crypto/sha512 - dtoastr dtotimespec dup2 environ + dtoastr dtotimespec dup2 environ execinfo filemode getloadavg getopt-gnu gettime gettimeofday ignore-value intprops largefile lstat manywarnings mktime pselect pthread_sigmask readlink === added file 'lib/execinfo.c' --- lib/execinfo.c 1970-01-01 00:00:00 +0000 +++ lib/execinfo.c 2012-08-22 08:59:10 +0000 @@ -0,0 +1,3 @@ +#include <config.h> +#define _GL_EXECINFO_INLINE _GL_EXTERN_INLINE +#include "execinfo.h" === added file 'lib/execinfo.in.h' --- lib/execinfo.in.h 1970-01-01 00:00:00 +0000 +++ lib/execinfo.in.h 2012-08-22 08:59:10 +0000 @@ -0,0 +1,54 @@ +/* Information about executables. + + Copyright (C) 2012 Free Software Foundation, Inc. + + This program is free software: you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see <http://www.gnu.org/licenses/>. */ + +/* Written by Paul Eggert. */ + +#ifndef _GL_EXECINFO_H +#define _GL_EXECINFO_H + +_GL_INLINE_HEADER_BEGIN +#ifndef _GL_EXECINFO_INLINE +# define _GL_EXECINFO_INLINE _GL_INLINE +#endif + +_GL_EXECINFO_INLINE int +backtrace (void **buffer, int size) +{ + (void) buffer; + (void) size; + return 0; +} + +_GL_EXECINFO_INLINE char ** +backtrace_symbols (void *const *buffer, int size) +{ + (void) buffer; + (void) size; + return 0; +} + +_GL_EXECINFO_INLINE void +backtrace_symbols_fd (void *const *buffer, int size, int fd) +{ + (void) buffer; + (void) size; + (void) fd; +} + +_GL_INLINE_HEADER_END + +#endif === modified file 'lib/gnulib.mk' --- lib/gnulib.mk 2012-08-19 23:31:24 +0000 +++ lib/gnulib.mk 2012-08-22 08:59:10 +0000 @@ -21,7 +21,7 @@ # the same distribution terms as the rest of that program. # # Generated by gnulib-tool. -# Reproduce by: gnulib-tool --import --dir=. --lib=libgnu --source-base=lib --m4-base=m4 --doc-base=doc --tests-base=tests --aux-dir=build-aux --avoid=errno --avoid=fcntl --avoid=fcntl-h --avoid=fstat --avoid=msvc-inval --avoid=msvc-nothrow --avoid=raise --avoid=select --avoid=sigprocmask --avoid=sys_types --avoid=threadlib --makefile-name=gnulib.mk --conditional-dependencies --no-libtool --macro-prefix=gl --no-vc-files alloca-opt c-ctype c-strcase careadlinkat crypto/md5 crypto/sha1 crypto/sha256 crypto/sha512 dtoastr dtotimespec dup2 environ filemode getloadavg getopt-gnu gettime gettimeofday ignore-value intprops largefile lstat manywarnings mktime pselect pthread_sigmask readlink socklen stat-time stdalign stdarg stdbool stdio strftime strtoimax strtoumax symlink sys_stat sys_time tim e timespec-add timespec-sub utimens warnings +# Reproduce by: gnulib-tool --import --dir=. --lib=libgnu --source-base=lib --m4-base=m4 --doc-base=doc --tests-base=tests --aux-dir=build-aux --avoid=errno --avoid=fcntl --avoid=fcntl-h --avoid=fstat --avoid=msvc-inval --avoid=msvc-nothrow --avoid=raise --avoid=select --avoid=sigprocmask --avoid=sys_types --avoid=threadlib --makefile-name=gnulib.mk --conditional-dependencies --no-libtool --macro-prefix=gl --no-vc-files alloca-opt c-ctype c-strcase careadlinkat crypto/md5 crypto/sha1 crypto/sha256 crypto/sha512 dtoastr dtotimespec dup2 environ execinfo filemode getloadavg getopt-gnu gettime gettimeofday ignore-value intprops largefile lstat manywarnings mktime pselect pthread_sigmask readlink socklen stat-time stdalign stdarg stdbool stdio strftime strtoimax strtoumax symlink sys_stat sys _time time timespec-add timespec-sub utimens warnings MOSTLYCLEANFILES += core *.stackdump @@ -150,6 +150,31 @@ ## end gnulib module dup2 +## begin gnulib module execinfo + +BUILT_SOURCES += $(EXECINFO_H) + +# We need the following in order to create <execinfo.h> when the system +# doesn't have one that works. +if GL_GENERATE_EXECINFO_H +execinfo.h: execinfo.in.h $(top_builddir)/config.status + $(AM_V_GEN)rm -f $@-t $@ && \ + { echo '/* DO NOT EDIT! GENERATED AUTOMATICALLY! */'; \ + cat $(srcdir)/execinfo.in.h; \ + } > $@-t && \ + mv $@-t $@ +else +execinfo.h: $(top_builddir)/config.status + rm -f $@ +endif +MOSTLYCLEANFILES += execinfo.h execinfo.h-t + +EXTRA_DIST += execinfo.c execinfo.in.h + +EXTRA_libgnu_a_SOURCES += execinfo.c + +## end gnulib module execinfo + ## begin gnulib module filemode libgnu_a_SOURCES += filemode.c === added file 'm4/execinfo.m4' --- m4/execinfo.m4 1970-01-01 00:00:00 +0000 +++ m4/execinfo.m4 2012-08-22 08:59:10 +0000 @@ -0,0 +1,21 @@ +# Check for GNU-style execinfo.h. + +dnl Copyright 2012 Free Software Foundation, Inc. +dnl This file is free software; the Free Software Foundation +dnl gives unlimited permission to copy and/or distribute it, +dnl with or without modifications, as long as this notice is preserved. + +AC_DEFUN([gl_EXECINFO_H], +[ + AC_CHECK_HEADERS_ONCE([execinfo.h]) + + if test $ac_cv_header_execinfo_h = yes; then + EXECINFO_H='' + else + EXECINFO_H='execinfo.h' + AC_LIBOBJ([execinfo]) + fi + + AC_SUBST([EXECINFO_H]) + AM_CONDITIONAL([GL_GENERATE_EXECINFO_H], [test -n "$EXECINFO_H"]) +]) === modified file 'm4/gnulib-comp.m4' --- m4/gnulib-comp.m4 2012-08-19 23:31:24 +0000 +++ m4/gnulib-comp.m4 2012-08-22 08:59:10 +0000 @@ -53,6 +53,7 @@ # Code from module dtotimespec: # Code from module dup2: # Code from module environ: + # Code from module execinfo: # Code from module extensions: AC_REQUIRE([gl_USE_SYSTEM_EXTENSIONS]) # Code from module extern-inline: @@ -152,6 +153,7 @@ gl_UNISTD_MODULE_INDICATOR([dup2]) gl_ENVIRON gl_UNISTD_MODULE_INDICATOR([environ]) + gl_EXECINFO_H AC_REQUIRE([gl_EXTERN_INLINE]) gl_FILEMODE gl_GETLOADAVG @@ -534,6 +536,8 @@ lib/dtoastr.c lib/dtotimespec.c lib/dup2.c + lib/execinfo.c + lib/execinfo.in.h lib/filemode.c lib/filemode.h lib/ftoastr.c @@ -605,6 +609,7 @@ m4/clock_time.m4 m4/dup2.m4 m4/environ.m4 + m4/execinfo.m4 m4/extensions.m4 m4/extern-inline.m4 m4/filemode.m4 === modified file 'src/ChangeLog' --- src/ChangeLog 2012-08-22 07:20:42 +0000 +++ src/ChangeLog 2012-08-22 08:59:10 +0000 @@ -1,5 +1,9 @@ 2012-08-22 Paul Eggert <eggert@cs.ucla.edu> + On assertion failure, print backtrace if available. + * alloc.c [ENABLE_CHECKING]: Include <execinfo.h>. + (die) [ENABLE_CHECKING]: Print a backtrace if available. + * fontset.c (FONTSET_ADD): Return void, not Lisp_Object. Otherwise, the compiler complains about (A?B:C) where B is void and C is Lisp_Object. This fixes an incompatibility with Sun C 5.12. === modified file 'src/alloc.c' --- src/alloc.c 2012-08-21 23:39:56 +0000 +++ src/alloc.c 2012-08-22 08:59:10 +0000 @@ -6681,13 +6681,21 @@ } #ifdef ENABLE_CHECKING + +# include <execinfo.h> + bool suppress_checking; void die (const char *msg, const char *file, int line) { + enum { NPOINTERS_MAX = 500 }; + void *buffer[NPOINTERS_MAX]; + int npointers; fprintf (stderr, "\r\n%s:%d: Emacs fatal error: %s\r\n", file, line, msg); + npointers = backtrace (buffer, NPOINTERS_MAX); + backtrace_symbols_fd (buffer, npointers, STDERR_FILENO); abort (); } #endif ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: C backtraces for Emacs 2012-08-22 9:24 ` C backtraces for Emacs Paul Eggert @ 2012-08-22 16:01 ` Eli Zaretskii 2012-08-22 16:39 ` Paul Eggert 2012-08-23 0:40 ` Daniel Colascione 2012-08-24 10:48 ` Paul Eggert 1 sibling, 2 replies; 34+ messages in thread From: Eli Zaretskii @ 2012-08-22 16:01 UTC (permalink / raw) To: Paul Eggert; +Cc: lekktu, monnier, emacs-devel > Date: Wed, 22 Aug 2012 02:24:31 -0700 > From: Paul Eggert <eggert@cs.ucla.edu> > Cc: Juanma Barranquero <lekktu@gmail.com>, Eli Zaretskii <eliz@gnu.org>, > emacs-devel@gnu.org > > I'll hold off installing this to give Eli and Juanma a heads-up, as it > creates a new substitute header <execinfo.h> on non-GNUish systems, > and the Windows build will need to do something similar. If I understand > things correctly Windows already does backtraces in a different way, so > its execinfo.h can just be a copy of execinfo.in.h. I'm not sure what you mean. An assertion violation on Windows calls a function that displays an abort dialogue, giving the user a chance to attach a debugger. If the user does attach a debugger, she can then display a backtrace from the debugger. There's no other facility to display a backtrace on Windows that I'm aware of. ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: C backtraces for Emacs 2012-08-22 16:01 ` Eli Zaretskii @ 2012-08-22 16:39 ` Paul Eggert 2012-08-23 0:40 ` Daniel Colascione 1 sibling, 0 replies; 34+ messages in thread From: Paul Eggert @ 2012-08-22 16:39 UTC (permalink / raw) To: Eli Zaretskii; +Cc: lekktu, monnier, emacs-devel On 08/22/2012 09:01 AM, Eli Zaretskii wrote: > I'm not sure what you mean. An assertion violation on Windows calls a > function that displays an abort dialogue, giving the user a chance to > attach a debugger. If the user does attach a debugger, she can then > display a backtrace from the debugger. There's no other facility to > display a backtrace on Windows that I'm aware of. Yes, sorry, that's what I meant. The idea is that the Windows build can just copy execinfo.in.h to execinfo.h, which means Emacs will continue to behave as before when eassert fails. ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: C backtraces for Emacs 2012-08-22 16:01 ` Eli Zaretskii 2012-08-22 16:39 ` Paul Eggert @ 2012-08-23 0:40 ` Daniel Colascione 2012-08-23 16:55 ` Eli Zaretskii 1 sibling, 1 reply; 34+ messages in thread From: Daniel Colascione @ 2012-08-23 0:40 UTC (permalink / raw) To: Eli Zaretskii; +Cc: lekktu, Paul Eggert, monnier, emacs-devel [-- Attachment #1: Type: text/plain, Size: 2274 bytes --] On 8/22/2012 9:01 AM, Eli Zaretskii wrote: >> Date: Wed, 22 Aug 2012 02:24:31 -0700 >> From: Paul Eggert <eggert@cs.ucla.edu> >> Cc: Juanma Barranquero <lekktu@gmail.com>, Eli Zaretskii <eliz@gnu.org>, >> emacs-devel@gnu.org >> >> I'll hold off installing this to give Eli and Juanma a heads-up, as it >> creates a new substitute header <execinfo.h> on non-GNUish systems, >> and the Windows build will need to do something similar. If I understand >> things correctly Windows already does backtraces in a different way, so >> its execinfo.h can just be a copy of execinfo.in.h. > > I'm not sure what you mean. An assertion violation on Windows calls a > function that displays an abort dialogue, giving the user a chance to > attach a debugger. If the user does attach a debugger, she can then > display a backtrace from the debugger. There's no other facility to > display a backtrace on Windows that I'm aware of. You can use the StackWalk64 function from dbghelp.dll. "The StackWalk64 function provides a portable method for obtaining a stack trace. Using the StackWalk64 function is recommended over writing your own function because of all the complexities associated with stack walking on platforms. In addition, there are compiler options that cause the stack to appear differently, depending on how the module is compiled. By using this function, your application has a portable stack trace that continues to work as the compiler and operating system change." Despite its name, StackWalk64 works on 32-bit platforms too. Windows actually has very good support for this kind of thing. By the way: when we assert, we should kill Emacs using RaiseFailFastException. This function produces a nice WER (Windows Error Reporting) dialog and bypasses a lot of layers of exception handling machinery that would ordinarily run in userspace --- that is, with RaiseFailFastException, fewer things can go wrong. abort() under Windows actually tried to run global destructors, send messages to loaded DLLs about pending process termination, and a bunch of other things. RFFE is much more reliable. On systems without RFFE, plain old RaiseException is fine too, and RaiseException is present on all supported Windows versions. [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 259 bytes --] ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: C backtraces for Emacs 2012-08-23 0:40 ` Daniel Colascione @ 2012-08-23 16:55 ` Eli Zaretskii 0 siblings, 0 replies; 34+ messages in thread From: Eli Zaretskii @ 2012-08-23 16:55 UTC (permalink / raw) To: Daniel Colascione; +Cc: lekktu, eggert, monnier, emacs-devel > Date: Wed, 22 Aug 2012 17:40:08 -0700 > From: Daniel Colascione <dancol@dancol.org> > CC: Paul Eggert <eggert@cs.ucla.edu>, lekktu@gmail.com, > monnier@IRO.UMontreal.CA, emacs-devel@gnu.org > > You can use the StackWalk64 function from dbghelp.dll. Will that work with GCC-compiled programs and DLLs? Isn't dbghelp.dll VC-specific? If it doesn't produce addresses that can be plugged into GDB, then it's not good, because any meaningful debugging after the assertion violation will have to be with GDB. Also, dbghelp.dll seems to need to be installed separately, and MinGW doesn't provide its import library, so we cannot link against it by default. Did I miss something? Anyway, if this is workable, patches to use it are welcome. > By the way: when we assert, we should kill Emacs using RaiseFailFastException. > This function produces a nice WER (Windows Error Reporting) dialog and bypasses > a lot of layers of exception handling machinery that would ordinarily run in > userspace --- that is, with RaiseFailFastException, fewer things can go wrong. > abort() under Windows actually tried to run global destructors, send messages to > loaded DLLs about pending process termination, and a bunch of other things. RFFE > is much more reliable. On systems without RFFE, plain old RaiseException is fine > too, and RaiseException is present on all supported Windows versions. Emacs overrides 'abort' with a custom function that calls our own abort dialog. So I don't think the problems with the standard 'abort' function are relevant to Emacs. ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: C backtraces for Emacs 2012-08-22 9:24 ` C backtraces for Emacs Paul Eggert 2012-08-22 16:01 ` Eli Zaretskii @ 2012-08-24 10:48 ` Paul Eggert 2012-08-25 4:41 ` Paul Eggert 1 sibling, 1 reply; 34+ messages in thread From: Paul Eggert @ 2012-08-24 10:48 UTC (permalink / raw) To: Stefan Monnier; +Cc: Juanma Barranquero, Eli Zaretskii, emacs-devel Bastien Roucariès mentioned that the 'backtrace' function and friends need the -lexecinfo linker flag on FreeBSD, so here's an updated version of the patch that does this. This affects only 'configure' and 'make', not Emacs itself. The NT and DOS builds are affected (a new makefile variable LIB_EXECINFO, which can be empty there) so I'm sending this to Eli and Juanma as a heads-up. === modified file 'ChangeLog' --- ChangeLog 2012-08-22 06:55:44 +0000 +++ ChangeLog 2012-08-24 10:43:12 +0000 @@ -1,3 +1,12 @@ +2012-08-24 Paul Eggert <eggert@cs.ucla.edu> + + On assertion failure, print backtrace if available. + Merge from gnulib, incorporating: + 2012-08-24 execinfo: port to FreeBSD + 2012-08-22 execinfo: new module + * lib/gnulib.mk, m4/gnulib-comp.m4: Regenerate. + * lib/execinfo.c, lib/execinfo.in.h, m4/execinfo.m4: New files. + 2012-08-22 Glenn Morris <rgm@gnu.org> * Makefile.in (install-arch-dep): If NO_BIN_LINK is non-null, === modified file 'admin/ChangeLog' --- admin/ChangeLog 2012-08-16 21:58:44 +0000 +++ admin/ChangeLog 2012-08-24 10:43:12 +0000 @@ -1,3 +1,8 @@ +2012-08-24 Paul Eggert <eggert@cs.ucla.edu> + + On assertion failure, print backtrace if available. + * merge-gnulib (GNULIB_MODULES): Add execinfo. + 2012-08-16 Paul Eggert <eggert@cs.ucla.edu> Use ASCII tests for character types. === modified file 'admin/merge-gnulib' --- admin/merge-gnulib 2012-08-16 21:58:44 +0000 +++ admin/merge-gnulib 2012-08-22 08:59:10 +0000 @@ -28,7 +28,7 @@ GNULIB_MODULES=' alloca-opt c-ctype c-strcase careadlinkat crypto/md5 crypto/sha1 crypto/sha256 crypto/sha512 - dtoastr dtotimespec dup2 environ + dtoastr dtotimespec dup2 environ execinfo filemode getloadavg getopt-gnu gettime gettimeofday ignore-value intprops largefile lstat manywarnings mktime pselect pthread_sigmask readlink === added file 'lib/execinfo.c' --- lib/execinfo.c 1970-01-01 00:00:00 +0000 +++ lib/execinfo.c 2012-08-22 08:59:10 +0000 @@ -0,0 +1,3 @@ +#include <config.h> +#define _GL_EXECINFO_INLINE _GL_EXTERN_INLINE +#include "execinfo.h" === added file 'lib/execinfo.in.h' --- lib/execinfo.in.h 1970-01-01 00:00:00 +0000 +++ lib/execinfo.in.h 2012-08-22 08:59:10 +0000 @@ -0,0 +1,54 @@ +/* Information about executables. + + Copyright (C) 2012 Free Software Foundation, Inc. + + This program is free software: you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see <http://www.gnu.org/licenses/>. */ + +/* Written by Paul Eggert. */ + +#ifndef _GL_EXECINFO_H +#define _GL_EXECINFO_H + +_GL_INLINE_HEADER_BEGIN +#ifndef _GL_EXECINFO_INLINE +# define _GL_EXECINFO_INLINE _GL_INLINE +#endif + +_GL_EXECINFO_INLINE int +backtrace (void **buffer, int size) +{ + (void) buffer; + (void) size; + return 0; +} + +_GL_EXECINFO_INLINE char ** +backtrace_symbols (void *const *buffer, int size) +{ + (void) buffer; + (void) size; + return 0; +} + +_GL_EXECINFO_INLINE void +backtrace_symbols_fd (void *const *buffer, int size, int fd) +{ + (void) buffer; + (void) size; + (void) fd; +} + +_GL_INLINE_HEADER_END + +#endif === modified file 'lib/gnulib.mk' --- lib/gnulib.mk 2012-08-19 23:31:24 +0000 +++ lib/gnulib.mk 2012-08-22 08:59:10 +0000 @@ -21,7 +21,7 @@ # the same distribution terms as the rest of that program. # # Generated by gnulib-tool. -# Reproduce by: gnulib-tool --import --dir=. --lib=libgnu --source-base=lib --m4-base=m4 --doc-base=doc --tests-base=tests --aux-dir=build-aux --avoid=errno --avoid=fcntl --avoid=fcntl-h --avoid=fstat --avoid=msvc-inval --avoid=msvc-nothrow --avoid=raise --avoid=select --avoid=sigprocmask --avoid=sys_types --avoid=threadlib --makefile-name=gnulib.mk --conditional-dependencies --no-libtool --macro-prefix=gl --no-vc-files alloca-opt c-ctype c-strcase careadlinkat crypto/md5 crypto/sha1 crypto/sha256 crypto/sha512 dtoastr dtotimespec dup2 environ filemode getloadavg getopt-gnu gettime gettimeofday ignore-value intprops largefile lstat manywarnings mktime pselect pthread_sigmask readlink socklen stat-time stdalign stdarg stdbool stdio strftime strtoimax strtoumax symlink sys_stat sys_time time timespec-add timespec-sub utimens warnings +# Reproduce by: gnulib-tool --import --dir=. --lib=libgnu --source-base=lib --m4-base=m4 --doc-base=doc --tests-base=tests --aux-dir=build-aux --avoid=errno --avoid=fcntl --avoid=fcntl-h --avoid=fstat --avoid=msvc-inval --avoid=msvc-nothrow --avoid=raise --avoid=select --avoid=sigprocmask --avoid=sys_types --avoid=threadlib --makefile-name=gnulib.mk --conditional-dependencies --no-libtool --macro-prefix=gl --no-vc-files alloca-opt c-ctype c-strcase careadlinkat crypto/md5 crypto/sha1 crypto/sha256 crypto/sha512 dtoastr dtotimespec dup2 environ execinfo filemode getloadavg getopt-gnu gettime gettimeofday ignore-value intprops largefile lstat manywarnings mktime pselect pthread_sigmask readlink socklen stat-time stdalign stdarg stdbool stdio strftime strtoimax strtoumax symlink sys_stat sys_time time timespec-add timespec-sub utimens warnings MOSTLYCLEANFILES += core *.stackdump @@ -150,6 +150,31 @@ ## end gnulib module dup2 +## begin gnulib module execinfo + +BUILT_SOURCES += $(EXECINFO_H) + +# We need the following in order to create <execinfo.h> when the system +# doesn't have one that works. +if GL_GENERATE_EXECINFO_H +execinfo.h: execinfo.in.h $(top_builddir)/config.status + $(AM_V_GEN)rm -f $@-t $@ && \ + { echo '/* DO NOT EDIT! GENERATED AUTOMATICALLY! */'; \ + cat $(srcdir)/execinfo.in.h; \ + } > $@-t && \ + mv $@-t $@ +else +execinfo.h: $(top_builddir)/config.status + rm -f $@ +endif +MOSTLYCLEANFILES += execinfo.h execinfo.h-t + +EXTRA_DIST += execinfo.c execinfo.in.h + +EXTRA_libgnu_a_SOURCES += execinfo.c + +## end gnulib module execinfo + ## begin gnulib module filemode libgnu_a_SOURCES += filemode.c === added file 'm4/execinfo.m4' --- m4/execinfo.m4 1970-01-01 00:00:00 +0000 +++ m4/execinfo.m4 2012-08-24 10:43:12 +0000 @@ -0,0 +1,31 @@ +# Check for GNU-style execinfo.h. + +dnl Copyright 2012 Free Software Foundation, Inc. +dnl This file is free software; the Free Software Foundation +dnl gives unlimited permission to copy and/or distribute it, +dnl with or without modifications, as long as this notice is preserved. + +AC_DEFUN([gl_EXECINFO_H], +[ + AC_CHECK_HEADERS_ONCE([execinfo.h]) + + LIB_EXECINFO='' + EXECINFO_H='execinfo.h' + + if test $ac_cv_header_execinfo_h = yes; then + gl_saved_libs=$LIBS + AC_SEARCH_LIBS([backtrace_symbols_fd], [execinfo], + [test "$ac_cv_search_backtrace_symbols_fd" = "none required" || + LIB_EXECINFO=$ac_cv_search_backtrace_symbols_fd]) + LIBS=$gl_saved_libs + test "$ac_cv_search_backtrace_symbols_fd" = no || EXECINFO_H='' + fi + + if test -n "$EXECINFO_H"; then + AC_LIBOBJ([execinfo]) + fi + + AC_SUBST([EXECINFO_H]) + AC_SUBST([LIB_EXECINFO]) + AM_CONDITIONAL([GL_GENERATE_EXECINFO_H], [test -n "$EXECINFO_H"]) +]) === modified file 'm4/gnulib-comp.m4' --- m4/gnulib-comp.m4 2012-08-19 23:31:24 +0000 +++ m4/gnulib-comp.m4 2012-08-22 08:59:10 +0000 @@ -53,6 +53,7 @@ # Code from module dtotimespec: # Code from module dup2: # Code from module environ: + # Code from module execinfo: # Code from module extensions: AC_REQUIRE([gl_USE_SYSTEM_EXTENSIONS]) # Code from module extern-inline: @@ -152,6 +153,7 @@ gl_UNISTD_MODULE_INDICATOR([dup2]) gl_ENVIRON gl_UNISTD_MODULE_INDICATOR([environ]) + gl_EXECINFO_H AC_REQUIRE([gl_EXTERN_INLINE]) gl_FILEMODE gl_GETLOADAVG @@ -534,6 +536,8 @@ lib/dtoastr.c lib/dtotimespec.c lib/dup2.c + lib/execinfo.c + lib/execinfo.in.h lib/filemode.c lib/filemode.h lib/ftoastr.c @@ -605,6 +609,7 @@ m4/clock_time.m4 m4/dup2.m4 m4/environ.m4 + m4/execinfo.m4 m4/extensions.m4 m4/extern-inline.m4 m4/filemode.m4 === modified file 'src/ChangeLog' --- src/ChangeLog 2012-08-24 04:37:57 +0000 +++ src/ChangeLog 2012-08-24 10:43:12 +0000 @@ -1,5 +1,11 @@ 2012-08-24 Paul Eggert <eggert@cs.ucla.edu> + On assertion failure, print backtrace if available. + * alloc.c [ENABLE_CHECKING]: Include <execinfo.h>. + (die) [ENABLE_CHECKING]: Print a backtrace if available. + * Makefile.in (LIB_EXECINFO): New macro. + (LIBES): Use it. + * buffer.c, buffer.h: Use bool for boolean. * buffer.c (reset_buffer_local_variables) (buffer_lisp_local_variables, Fset_buffer_modified_p) === modified file 'src/Makefile.in' --- src/Makefile.in 2012-08-20 23:57:35 +0000 +++ src/Makefile.in 2012-08-24 10:43:12 +0000 @@ -159,6 +159,8 @@ ## dbusbind.o if HAVE_DBUS, else empty. DBUS_OBJ = @DBUS_OBJ@ +LIB_EXECINFO=@LIB_EXECINFO@ + SETTINGS_CFLAGS = @SETTINGS_CFLAGS@ SETTINGS_LIBS = @SETTINGS_LIBS@ @@ -383,6 +385,7 @@ ## with GCC, we might need LIB_GCC again after them. LIBES = $(LIBS) $(LIBX_BASE) $(LIBX_OTHER) $(LIBSOUND) \ $(RSVG_LIBS) $(IMAGEMAGICK_LIBS) $(LIB_CLOCK_GETTIME) $(DBUS_LIBS) \ + $(LIB_EXECINFO) \ $(LIBXML2_LIBS) $(LIBGPM) $(LIBRESOLV) $(LIBS_SYSTEM) \ $(LIBS_TERMCAP) $(GETLOADAVG_LIBS) $(SETTINGS_LIBS) $(LIBSELINUX_LIBS) \ $(FREETYPE_LIBS) $(FONTCONFIG_LIBS) $(LIBOTF_LIBS) $(M17N_FLT_LIBS) \ === modified file 'src/alloc.c' --- src/alloc.c 2012-08-21 23:39:56 +0000 +++ src/alloc.c 2012-08-22 08:59:10 +0000 @@ -6681,13 +6681,21 @@ } #ifdef ENABLE_CHECKING + +# include <execinfo.h> + bool suppress_checking; void die (const char *msg, const char *file, int line) { + enum { NPOINTERS_MAX = 500 }; + void *buffer[NPOINTERS_MAX]; + int npointers; fprintf (stderr, "\r\n%s:%d: Emacs fatal error: %s\r\n", file, line, msg); + npointers = backtrace (buffer, NPOINTERS_MAX); + backtrace_symbols_fd (buffer, npointers, STDERR_FILENO); abort (); } #endif ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: C backtraces for Emacs 2012-08-24 10:48 ` Paul Eggert @ 2012-08-25 4:41 ` Paul Eggert 2012-08-25 5:57 ` Eli Zaretskii 0 siblings, 1 reply; 34+ messages in thread From: Paul Eggert @ 2012-08-25 4:41 UTC (permalink / raw) To: Eli Zaretskii; +Cc: Juanma Barranquero, emacs-devel I've checked in that change (trunk bzr 109765 and 109766) and so the Windows builds probably need tweaking now along the lines previously discussed. One other thing I just now noticed -- the Windows substitute for unistd.h may need something like this: #define STDIN_FILENO 0 #define STDOUT_FILENO 1 #define STDERR_FILENO 2 if Windows doesn't already have the equivalent. The backtrace code uses STDERR_FILENO rather than fileno (stderr) as it's better not to rely on stdio's data structures after an assertion has failed. ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: C backtraces for Emacs 2012-08-25 4:41 ` Paul Eggert @ 2012-08-25 5:57 ` Eli Zaretskii 0 siblings, 0 replies; 34+ messages in thread From: Eli Zaretskii @ 2012-08-25 5:57 UTC (permalink / raw) To: Paul Eggert; +Cc: lekktu, emacs-devel > Date: Fri, 24 Aug 2012 21:41:03 -0700 > From: Paul Eggert <eggert@cs.ucla.edu> > CC: emacs-devel@gnu.org, Juanma Barranquero <lekktu@gmail.com> > > I've checked in that change (trunk bzr 109765 and 109766) > and so the Windows builds probably need tweaking now > along the lines previously discussed. Done. > One other thing I just now noticed -- the Windows substitute > for unistd.h may need something like this: > > #define STDIN_FILENO 0 > #define STDOUT_FILENO 1 > #define STDERR_FILENO 2 > > if Windows doesn't already have the equivalent. They are defined on stdio.h, so no changes are needed. ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: inlinable functions instead of macros 2012-08-21 17:50 ` Stefan Monnier 2012-08-22 9:24 ` C backtraces for Emacs Paul Eggert @ 2012-08-22 9:28 ` Paul Eggert 2012-08-22 9:48 ` Andreas Schwab ` (2 more replies) 1 sibling, 3 replies; 34+ messages in thread From: Paul Eggert @ 2012-08-22 9:28 UTC (permalink / raw) To: Stefan Monnier; +Cc: emacs-devel On 08/21/2012 10:50 AM, Stefan Monnier wrote: > Could some please check our inlinable functions and move any easserts in > them to a macro (i.e. rename the function, replace the old name with > a macro that does the assert and calls the inlinable function)? I took a look at doing this, but it's problematic. I assume we want to do this only with the "small" functions -- where we're not interested in the function itself, but in the function's caller. Doing it with all inlinable functions would not be useful, since so many functions are inlinable these days. Unfortunately wrapping "small" functions with macros breaks down in the common case where the caller is another "small" function. For example, set_hash_key_slot is a "small" function, and it calls another "small" function gc_aset, which invokes eassert. If both functions are wrapped, an assertion failure in the inner reports the outer's location, instead of what's wanted (the outer's caller's location). How about if we try the following instead? It's easier to implement and is less intrusive to the code. The idea is to use the <execinfo.h> change that I mentioned in my previous message (a change we should do independently anyway). With that change, when an assertion fails, it will generate output that looks like this: alloc.c:800: Emacs fatal error: assertion failed: 0 <= nitems && 0 < item_size ./temacs[0x5bbdaf] ./temacs[0x5bf5e2] ./temacs[0x549cbd] ./temacs[0x417681] /lib64/libc.so.6(__libc_start_main+0xed)[0x3b3082135d] ./temacs[0x418d85] You can then find out the source locations corresponding to the backtrace by giving those addresses to the debugger, like this: $ gdb temacs ... (gdb) b *0x5bbdaf Breakpoint 2 at 0x5bbdaf: file alloc.c, line 6698. (gdb) b *0x5bf5e2 Breakpoint 3 at 0x5bf5e2: file alloc.c, line 802. (gdb) b *0x549cbd Breakpoint 4 at 0x549cbd: file emacs.c, line 1799. The top of the stack is 'die' itself; the second slot is the "small" function xnmalloc; the third slot is the "big" function sort_args, where it calls xnmalloc. This gives all the information that the current scheme does, plus more, because you have a full backtrace. Once we have this, we shouldn't need to create extra macro wrappers and address the extra hassles that they'd cause. ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: inlinable functions instead of macros 2012-08-22 9:28 ` inlinable functions instead of macros Paul Eggert @ 2012-08-22 9:48 ` Andreas Schwab 2012-08-22 14:31 ` Stefan Monnier 2012-08-22 16:03 ` Eli Zaretskii 2 siblings, 0 replies; 34+ messages in thread From: Andreas Schwab @ 2012-08-22 9:48 UTC (permalink / raw) To: Paul Eggert; +Cc: Stefan Monnier, emacs-devel Paul Eggert <eggert@cs.ucla.edu> writes: > alloc.c:800: Emacs fatal error: assertion failed: 0 <= nitems && 0 < item_size > ./temacs[0x5bbdaf] > ./temacs[0x5bf5e2] > ./temacs[0x549cbd] > ./temacs[0x417681] > /lib64/libc.so.6(__libc_start_main+0xed)[0x3b3082135d] > ./temacs[0x418d85] > > You can then find out the source locations corresponding to the > backtrace by giving those addresses to the debugger, like this: Or use addr2line(1). Andreas. -- Andreas Schwab, schwab@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different." ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: inlinable functions instead of macros 2012-08-22 9:28 ` inlinable functions instead of macros Paul Eggert 2012-08-22 9:48 ` Andreas Schwab @ 2012-08-22 14:31 ` Stefan Monnier 2012-08-24 21:37 ` Paul Eggert 2012-08-22 16:03 ` Eli Zaretskii 2 siblings, 1 reply; 34+ messages in thread From: Stefan Monnier @ 2012-08-22 14:31 UTC (permalink / raw) To: Paul Eggert; +Cc: emacs-devel >> Could some please check our inlinable functions and move any easserts in >> them to a macro (i.e. rename the function, replace the old name with >> a macro that does the assert and calls the inlinable function)? > I took a look at doing this, but it's problematic. No it's not: we had that a few weeks ago. I don't want to rely on execinfo for that. It was working simply and effectively and want it to work again simply and effectively, using plain C without relying on any special support in glibc. Stefan ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: inlinable functions instead of macros 2012-08-22 14:31 ` Stefan Monnier @ 2012-08-24 21:37 ` Paul Eggert 2012-08-25 1:38 ` Tom Tromey 2012-08-25 2:05 ` Stefan Monnier 0 siblings, 2 replies; 34+ messages in thread From: Paul Eggert @ 2012-08-24 21:37 UTC (permalink / raw) To: Stefan Monnier; +Cc: emacs-devel On 08/22/2012 07:31 AM, Stefan Monnier wrote: > It was working simply and effectively It's simple to put easserts into macros, but as you know this has problems that make the macros hard to use. They cannot be invoked from GDB, which complicates debugging. More important, callers sometimes cannot specify arguments with side effects, and this restriction makes callers error-prone. We've tried to address some of the side-effect issues with this hack: /* The IDX==IDX tries to detect when the macro argument is side-effecting. */ #define ASET(ARRAY, IDX, VAL) \ (eassert ((IDX) == (IDX)), \ eassert ((IDX) >= 0 && (IDX) < ASIZE (ARRAY)), \ XVECTOR (ARRAY)->contents[IDX] = (VAL)) but even this hack doesn't catch the side-effect problem reliably, and anyway it's better to not introduce further problems like this. Here's how I'd like to address this issue: 1. Revert the setters and getters (e.g., bset_filename, BVAR) and go back to ordinary field accesses, as described in <http://bugs.gnu.org/12215#61>. This will simplify the problem by reducing the number of functions/macros affected. We're still working on the details here but it's clear that most of these functions will go. 2. Revert the easserts recently added to Emacs that are contributing to this problem. We went without these easserts for years, and there's no particular reason to add them now if they cause problems. 3. Assuming (1) and (2), only two problematic easserts remain: in blv_found and set_blv_found. Address them with the simple solution that Sam Steingold proposed in <http://lists.gnu.org/archive/html/emacs-devel/2012-08/msg00641.html>, combined with an externally-visible wrapper for GDB's sake. 4. After 24.3, look into fixing the side-effect problem for ASET and other macros too, as described in Bug#11935. That's a bigger project, but it will have other benefits too. Here's a patch to implement (2) and (3); it assumes the patch mentioned in (1). This patch makes Emacs a tiny bit smaller with the default build, so it should be OK on efficiency grounds. === modified file 'src/ChangeLog' --- src/ChangeLog 2012-08-24 04:43:52 +0000 +++ src/ChangeLog 2012-08-24 21:24:45 +0000 @@ -1,5 +1,17 @@ 2012-08-24 Paul Eggert <eggert@cs.ucla.edu> + Report better line numbers when assertions fail (Bug#12215). + * alloc.c, lisp.h (LISP_INLINE_EXTERNALLY_VISIBLE): New macro. + * buffer.h (buffer_intervals, set_buffer_intervals): + * lisp.h (gc_aset, vcopy): Remove recently-introduced easserts, as + they're not that useful in practice and they emit bogus line numbers. + * lisp.h (eassert, blv_found, set_blv_found): + Reimplement in terms of eassert_f, blv_found_f, set_blv_found_f. + (eassert_f, blv_found_f, set_blv_found_f): New macros or functions, + containing the guts of the old, but which let the caller specify + file and line number. + (blv_found, set_blv_found): Also define as a function, for GDB. + Remove setters like bset_keymap and getters like BVAR (Bug#12215). They make the code harder to read, and for now they don't bring anything to the table. Open-code them instead. === modified file 'src/alloc.c' --- src/alloc.c 2012-08-23 06:49:09 +0000 +++ src/alloc.c 2012-08-24 21:24:45 +0000 @@ -21,6 +21,7 @@ #include <config.h> #define LISP_INLINE EXTERN_INLINE +#define LISP_INLINE_EXTERNALLY_VISIBLE EXTERN_INLINE EXTERNALLY_VISIBLE #include <stdio.h> #include <limits.h> /* For CHAR_BIT. */ === modified file 'src/buffer.h' --- src/buffer.h 2012-08-24 04:43:52 +0000 +++ src/buffer.h 2012-08-24 21:24:45 +0000 @@ -948,7 +948,6 @@ BUFFER_INLINE INTERVAL buffer_intervals (struct buffer *b) { - eassert (b->text != NULL); return b->text->intervals; } @@ -957,7 +956,6 @@ BUFFER_INLINE void set_buffer_intervals (struct buffer *b, INTERVAL i) { - eassert (b->text != NULL); b->text->intervals = i; } === modified file 'src/lisp.h' --- src/lisp.h 2012-08-24 04:43:52 +0000 +++ src/lisp.h 2012-08-24 21:24:45 +0000 @@ -32,6 +32,7 @@ INLINE_HEADER_BEGIN #ifndef LISP_INLINE # define LISP_INLINE INLINE +# define LISP_INLINE_EXTERNALLY_VISIBLE INLINE #endif /* The ubiquitous max and min macros. */ @@ -110,8 +111,13 @@ /* Define an Emacs version of 'assert (COND)', since some system-defined 'assert's are flaky. COND should be free of side effects; it may or may not be evaluated. */ + +#define eassert(cond) eassert_f (cond, __FILE__, __LINE__) + #ifndef ENABLE_CHECKING -# define eassert(X) ((void) (0 && (X))) /* Check that X compiles. */ + /* Do nothing, but check that args compile. */ +# define eassert_f(cond, file, line) \ + ((void) (0 && (cond) && (file)[(line) >> 31])) #else /* ENABLE_CHECKING */ extern _Noreturn void die (const char *, const char *, int); @@ -126,10 +132,10 @@ testing that STRINGP (x) is true, making the test redundant. */ extern bool suppress_checking EXTERNALLY_VISIBLE; -# define eassert(cond) \ +# define eassert_f(cond, file, line) \ ((cond) || suppress_checking \ ? (void) 0 \ - : die ("assertion failed: " # cond, __FILE__, __LINE__)) + : die ("assertion failed: " # cond, file, line)) #endif /* ENABLE_CHECKING */ \f /* Use the configure flag --enable-check-lisp-object-type to make @@ -2335,7 +2341,6 @@ { /* Like ASET, but also can be used in the garbage collector: sweep_weak_table calls set_hash_key etc. while the table is marked. */ - eassert (0 <= idx && idx < (ASIZE (array) & ~ARRAY_MARK_FLAG)); XVECTOR (array)->contents[idx] = val; } @@ -2344,7 +2349,6 @@ LISP_INLINE void vcopy (Lisp_Object v, ptrdiff_t offset, Lisp_Object *args, ptrdiff_t count) { - eassert (0 <= offset && 0 <= count && offset + count <= ASIZE (v)); memcpy (XVECTOR (v)->contents + offset, args, count * sizeof *args); } @@ -2383,18 +2387,33 @@ /* Buffer-local (also frame-local) variable access functions. */ LISP_INLINE int -blv_found (struct Lisp_Buffer_Local_Value *blv) +blv_found_f (struct Lisp_Buffer_Local_Value *blv, char const *file, int line) { - eassert (blv->found == !EQ (blv->defcell, blv->valcell)); + eassert_f (blv->found == !EQ (blv->defcell, blv->valcell), file, line); return blv->found; } +#define blv_found(blv) \ + blv_found_f (blv, __FILE__, __LINE__) +LISP_INLINE_EXTERNALLY_VISIBLE int +(blv_found) (struct Lisp_Buffer_Local_Value *blv) +{ + return blv_found (blv); +} LISP_INLINE void -set_blv_found (struct Lisp_Buffer_Local_Value *blv, int found) +set_blv_found_f (struct Lisp_Buffer_Local_Value *blv, int found, + char const *file, int line) { - eassert (found == !EQ (blv->defcell, blv->valcell)); + eassert_f (found == !EQ (blv->defcell, blv->valcell), file, line); blv->found = found; } +#define set_blv_found(blv, found) \ + set_blv_found_f (blv, found, __FILE__, __LINE__) +LISP_INLINE_EXTERNALLY_VISIBLE void +(set_blv_found) (struct Lisp_Buffer_Local_Value *blv, int found) +{ + set_blv_found (blv, found); +} LISP_INLINE Lisp_Object blv_value (struct Lisp_Buffer_Local_Value *blv) ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: inlinable functions instead of macros 2012-08-24 21:37 ` Paul Eggert @ 2012-08-25 1:38 ` Tom Tromey 2012-08-25 2:58 ` Paul Eggert 2012-08-25 2:05 ` Stefan Monnier 1 sibling, 1 reply; 34+ messages in thread From: Tom Tromey @ 2012-08-25 1:38 UTC (permalink / raw) To: Paul Eggert; +Cc: Stefan Monnier, emacs-devel >>>>> "Paul" == Paul Eggert <eggert@cs.ucla.edu> writes: Paul> It's simple to put easserts into macros, but as you know this has Paul> problems that make the macros hard to use. They cannot be invoked Paul> from GDB, which complicates debugging. Why is that? I haven't been following this, but I wonder if there is a gdb deficiency we can easily address. Paul> More important, callers sometimes cannot specify arguments with Paul> side effects, and this restriction makes callers error-prone. Paul> We've tried to address some of the side-effect issues with this Paul> hack: Paul> /* The IDX==IDX tries to detect when the macro argument is side-effecting. */ Paul> #define ASET(ARRAY, IDX, VAL) \ Paul> (eassert ((IDX) == (IDX)), \ I've often wished that GCC had a __builtin_assert_no_side_effects for use in macros. Tom ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: inlinable functions instead of macros 2012-08-25 1:38 ` Tom Tromey @ 2012-08-25 2:58 ` Paul Eggert 2012-08-25 5:35 ` Eli Zaretskii 0 siblings, 1 reply; 34+ messages in thread From: Paul Eggert @ 2012-08-25 2:58 UTC (permalink / raw) To: Tom Tromey; +Cc: emacs-devel On 08/24/2012 06:38 PM, Tom Tromey wrote: > I wonder if there is a gdb deficiency we can easily address. I looked into this and it turns out that it can be addressed, on the platforms that support "gcc -g3". -g3 is not the default, which means random Emacs builds are unlikely to work with GDB and macros. Still, gcc -g3 works on my main platform, which removes much of that issue with macros. Macros' main problems are their gotchas with side effects and scoping, alas, which gcc -g3 doesn't fix. ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: inlinable functions instead of macros 2012-08-25 2:58 ` Paul Eggert @ 2012-08-25 5:35 ` Eli Zaretskii 2012-08-25 20:13 ` Tom Tromey 0 siblings, 1 reply; 34+ messages in thread From: Eli Zaretskii @ 2012-08-25 5:35 UTC (permalink / raw) To: Paul Eggert; +Cc: tromey, emacs-devel > Date: Fri, 24 Aug 2012 19:58:10 -0700 > From: Paul Eggert <eggert@cs.ucla.edu> > Cc: emacs-devel@gnu.org > > On 08/24/2012 06:38 PM, Tom Tromey wrote: > > I wonder if there is a gdb deficiency we can easily address. > > I looked into this and it turns out that it can be addressed, > on the platforms that support "gcc -g3". -g3 is not the default, > which means random Emacs builds are unlikely to work with GDB and > macros. FWIW, I always wondered why we don't use "-ggdb -g3" by default, when GCC is the compiler. ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: inlinable functions instead of macros 2012-08-25 5:35 ` Eli Zaretskii @ 2012-08-25 20:13 ` Tom Tromey 2012-08-26 4:42 ` Paul Eggert 0 siblings, 1 reply; 34+ messages in thread From: Tom Tromey @ 2012-08-25 20:13 UTC (permalink / raw) To: Eli Zaretskii; +Cc: Paul Eggert, emacs-devel Eli> FWIW, I always wondered why we don't use "-ggdb -g3" by default, when Eli> GCC is the compiler. Just '-g3' is enough (I don't think -ggdb has done anything important in a long time). I think it would be a good change. I build Emacs (and gdb and gcc) with -g3 routinely, as it makes debugging much simpler. Tom ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: inlinable functions instead of macros 2012-08-25 20:13 ` Tom Tromey @ 2012-08-26 4:42 ` Paul Eggert 0 siblings, 0 replies; 34+ messages in thread From: Paul Eggert @ 2012-08-26 4:42 UTC (permalink / raw) To: Tom Tromey; +Cc: Eli Zaretskii, emacs-devel On 08/25/2012 01:13 PM, Tom Tromey wrote: > Just '-g3' is enough (I don't think -ggdb has done anything important in > a long time). I think it would be a good change. OK, thanks, I gave it a shot by having 'configure' default to -g3 instead of plain -g, if -g3 works. This affects the build only when the user of 'configure' does not specify CFLAGS. I installed this as trunk bzr 109779. ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: inlinable functions instead of macros 2012-08-24 21:37 ` Paul Eggert 2012-08-25 1:38 ` Tom Tromey @ 2012-08-25 2:05 ` Stefan Monnier 2012-08-26 5:31 ` Paul Eggert 1 sibling, 1 reply; 34+ messages in thread From: Stefan Monnier @ 2012-08-25 2:05 UTC (permalink / raw) To: Paul Eggert; +Cc: emacs-devel > problems that make the macros hard to use. They cannot be invoked > from GDB, which complicates debugging. More important, callers Most of those macros don't need to be used from GDB (e.g. I've never needed to use ASET from GDB). > restriction makes callers error-prone. We've tried to address some of > the side-effect issues with this hack: > /* The IDX==IDX tries to detect when the macro argument is side-effecting. */ > #define ASET(ARRAY, IDX, VAL) \ > (eassert ((IDX) == (IDX)), \ This should be removed indeed. I added it temporarily and in any case it doesn't work (e.g. "i++ == i++" will always return true in my experience). > + Reimplement in terms of eassert_f, blv_found_f, set_blv_found_f. > + (eassert_f, blv_found_f, set_blv_found_f): New macros or functions, > + containing the guts of the old, but which let the caller specify > + file and line number. I'd rather avoid these kinds of efforts, Stefan ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: inlinable functions instead of macros 2012-08-25 2:05 ` Stefan Monnier @ 2012-08-26 5:31 ` Paul Eggert 0 siblings, 0 replies; 34+ messages in thread From: Paul Eggert @ 2012-08-26 5:31 UTC (permalink / raw) To: Stefan Monnier; +Cc: emacs-devel On 08/24/2012 07:05 PM, Stefan Monnier wrote: > I've never needed to use ASET from GDB I guess our debugging styles differ. But this issue is moot now that -g3 is the default. >> /* The IDX==IDX tries to detect when the macro argument is side-effecting. */ >> #define ASET(ARRAY, IDX, VAL) \ >> (eassert ((IDX) == (IDX)), \ > > This should be removed indeed. That's easy, and I did that in trunk bzr 109780. >> + Reimplement in terms of eassert_f,... > > I'd rather avoid these kinds of efforts, Me too. I'd rather rely on the new execinfo stuff. Perhaps after you've had a chance to give execinfo a try, we can see whether we can improve matters further in this area. ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: inlinable functions instead of macros 2012-08-22 9:28 ` inlinable functions instead of macros Paul Eggert 2012-08-22 9:48 ` Andreas Schwab 2012-08-22 14:31 ` Stefan Monnier @ 2012-08-22 16:03 ` Eli Zaretskii 2 siblings, 0 replies; 34+ messages in thread From: Eli Zaretskii @ 2012-08-22 16:03 UTC (permalink / raw) To: Paul Eggert; +Cc: monnier, emacs-devel > Date: Wed, 22 Aug 2012 02:28:52 -0700 > From: Paul Eggert <eggert@cs.ucla.edu> > Cc: emacs-devel@gnu.org > > alloc.c:800: Emacs fatal error: assertion failed: 0 <= nitems && 0 < item_size > ./temacs[0x5bbdaf] > ./temacs[0x5bf5e2] > ./temacs[0x549cbd] > ./temacs[0x417681] > /lib64/libc.so.6(__libc_start_main+0xed)[0x3b3082135d] > ./temacs[0x418d85] > > You can then find out the source locations corresponding to the > backtrace by giving those addresses to the debugger, like this: > > $ gdb temacs > ... > (gdb) b *0x5bbdaf > Breakpoint 2 at 0x5bbdaf: file alloc.c, line 6698. > (gdb) b *0x5bf5e2 > Breakpoint 3 at 0x5bf5e2: file alloc.c, line 802. > (gdb) b *0x549cbd > Breakpoint 4 at 0x549cbd: file emacs.c, line 1799. Also, "l *0x5bbdaf" etc. will show source lines around the offending line (6698 etc.). ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: inlinable functions instead of macros 2012-08-17 23:48 inlinable functions instead of macros Stefan Monnier 2012-08-17 23:59 ` Paul Eggert @ 2012-08-18 19:08 ` Richard Stallman 2012-08-18 20:57 ` Paul Eggert 2012-08-18 21:14 ` Óscar Fuentes 2012-08-19 17:28 ` Florian Weimer 2012-08-20 20:38 ` Sam Steingold 3 siblings, 2 replies; 34+ messages in thread From: Richard Stallman @ 2012-08-18 19:08 UTC (permalink / raw) To: Stefan Monnier; +Cc: emacs-devel lisp.h:2416: Emacs fatal error: assertion failed: found == !EQ (blv->defcell, blv->valcell) I.e. the file&line info is always the same rather than giving the file&line where the inlinable function was called. Could a GCC improvement be a good fix for this? For instance, if it allowed the assertion failure to find and mention the source location of the function's caller? -- Dr Richard Stallman President, Free Software Foundation 51 Franklin St Boston MA 02110 USA www.fsf.org www.gnu.org Skype: No way! That's nonfree (freedom-denying) software. Use Ekiga or an ordinary phone call ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: inlinable functions instead of macros 2012-08-18 19:08 ` Richard Stallman @ 2012-08-18 20:57 ` Paul Eggert 2012-08-18 21:03 ` Eli Zaretskii 2012-08-18 21:14 ` Óscar Fuentes 1 sibling, 1 reply; 34+ messages in thread From: Paul Eggert @ 2012-08-18 20:57 UTC (permalink / raw) To: rms; +Cc: Stefan Monnier, emacs-devel On 08/18/2012 12:08 PM, Richard Stallman wrote: > Could a GCC improvement be a good fix for this? For instance, if it > allowed the assertion failure to find and mention the source location > of the function's caller? That could be done via GCC. It should also be doable without changing the compiler, by enhancing glibc's 'backtrace' function. GNU binutils contains code to do this, in its addr2line command, and presumably we could use similar code inside Emacs. Emacs could link to libbfd on systems that have it, or it could bundle libbfd, or it could invoke the command-line tool addr2line to get the relevant info. Whichever's easiest, presumably. ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: inlinable functions instead of macros 2012-08-18 20:57 ` Paul Eggert @ 2012-08-18 21:03 ` Eli Zaretskii 2012-08-18 21:31 ` Paul Eggert 0 siblings, 1 reply; 34+ messages in thread From: Eli Zaretskii @ 2012-08-18 21:03 UTC (permalink / raw) To: Paul Eggert; +Cc: emacs-devel, rms, monnier > Date: Sat, 18 Aug 2012 13:57:58 -0700 > From: Paul Eggert <eggert@cs.ucla.edu> > Cc: Stefan Monnier <monnier@IRO.UMontreal.CA>, emacs-devel@gnu.org > > On 08/18/2012 12:08 PM, Richard Stallman wrote: > > Could a GCC improvement be a good fix for this? For instance, if it > > allowed the assertion failure to find and mention the source location > > of the function's caller? > > That could be done via GCC. What do __FUNCTION__ and __func__ hold for an inlined function? ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: inlinable functions instead of macros 2012-08-18 21:03 ` Eli Zaretskii @ 2012-08-18 21:31 ` Paul Eggert 2012-08-19 2:45 ` Eli Zaretskii 0 siblings, 1 reply; 34+ messages in thread From: Paul Eggert @ 2012-08-18 21:31 UTC (permalink / raw) To: Eli Zaretskii; +Cc: emacs-devel On 08/18/2012 02:03 PM, Eli Zaretskii wrote: > What do __FUNCTION__ and __func__ hold for an inlined function? The name of the inlined function itself, which is not what's wanted here. ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: inlinable functions instead of macros 2012-08-18 21:31 ` Paul Eggert @ 2012-08-19 2:45 ` Eli Zaretskii 0 siblings, 0 replies; 34+ messages in thread From: Eli Zaretskii @ 2012-08-19 2:45 UTC (permalink / raw) To: Paul Eggert; +Cc: emacs-devel > Date: Sat, 18 Aug 2012 14:31:13 -0700 > From: Paul Eggert <eggert@cs.ucla.edu> > CC: emacs-devel@gnu.org > > On 08/18/2012 02:03 PM, Eli Zaretskii wrote: > > What do __FUNCTION__ and __func__ hold for an inlined function? > > The name of the inlined function itself, which is not what's > wanted here. Sounds like this could be a starting point for a GCC feature that is wanted here. ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: inlinable functions instead of macros 2012-08-18 19:08 ` Richard Stallman 2012-08-18 20:57 ` Paul Eggert @ 2012-08-18 21:14 ` Óscar Fuentes 1 sibling, 0 replies; 34+ messages in thread From: Óscar Fuentes @ 2012-08-18 21:14 UTC (permalink / raw) To: emacs-devel Richard Stallman <rms@gnu.org> writes: > lisp.h:2416: Emacs fatal error: assertion failed: found == !EQ (blv->defcell, blv->valcell) > > I.e. the file&line info is always the same rather than giving the > file&line where the inlinable function was called. > > Could a GCC improvement be a good fix for this? For instance, if it > allowed the assertion failure to find and mention the source location > of the function's caller? It looks quite difficult to even give a sound specification for such feature. And I bet that the implementation, if possible at all, would be very complex and extensive. Either print a backtrace or revert to #define's. ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: inlinable functions instead of macros 2012-08-17 23:48 inlinable functions instead of macros Stefan Monnier 2012-08-17 23:59 ` Paul Eggert 2012-08-18 19:08 ` Richard Stallman @ 2012-08-19 17:28 ` Florian Weimer 2012-08-20 20:38 ` Sam Steingold 3 siblings, 0 replies; 34+ messages in thread From: Florian Weimer @ 2012-08-19 17:28 UTC (permalink / raw) To: Stefan Monnier; +Cc: emacs-devel * Stefan Monnier: > While inlinable functions are much cleaner than macros, they have > a very serious downside: you just end up with > > lisp.h:2416: Emacs fatal error: assertion failed: found == !EQ (blv->defcell, blv->valcell) > > I.e. the file&line info is always the same rather than giving the > file&line where the inlinable function was called. Coincidentally, there's a related discussion on the gcc mailing list: <http://gcc.gnu.org/ml/gcc/2012-08/msg00172.html> The approach we settled for is C++-specific, though. ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: inlinable functions instead of macros 2012-08-17 23:48 inlinable functions instead of macros Stefan Monnier ` (2 preceding siblings ...) 2012-08-19 17:28 ` Florian Weimer @ 2012-08-20 20:38 ` Sam Steingold 3 siblings, 0 replies; 34+ messages in thread From: Sam Steingold @ 2012-08-20 20:38 UTC (permalink / raw) To: emacs-devel > * Stefan Monnier <zbaavre@VEB.HZbagerny.PN> [2012-08-17 19:48:50 -0400]: > > While inlinable functions are much cleaner than macros, they have > a very serious downside: you just end up with > > lisp.h:2416: Emacs fatal error: assertion failed: found == !EQ (blv->defcell, blv->valcell) > > I.e. the file&line info is always the same rather than giving the > file&line where the inlinable function was called. a simple solution is this: inline void my_inlinable_function (....., char* file, int line) {....} #deinfe my_inlinable_function(.....) (my_inlinable_function)(.....,FILE,LINE) -- Sam Steingold (http://sds.podval.org/) on Ubuntu 12.04 (precise) X 11.0.11103000 http://www.childpsy.net/ http://pmw.org.il http://www.PetitionOnline.com/tap12009/ http://dhimmi.com http://jihadwatch.org A man paints with his brains and not with his hands. ^ permalink raw reply [flat|nested] 34+ messages in thread
end of thread, other threads:[~2012-08-26 5:31 UTC | newest] Thread overview: 34+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-08-17 23:48 inlinable functions instead of macros Stefan Monnier 2012-08-17 23:59 ` Paul Eggert 2012-08-18 22:15 ` Daniel Colascione 2012-08-18 22:55 ` John Yates 2012-08-19 4:22 ` Richard Stallman 2012-08-21 17:50 ` Stefan Monnier 2012-08-22 9:24 ` C backtraces for Emacs Paul Eggert 2012-08-22 16:01 ` Eli Zaretskii 2012-08-22 16:39 ` Paul Eggert 2012-08-23 0:40 ` Daniel Colascione 2012-08-23 16:55 ` Eli Zaretskii 2012-08-24 10:48 ` Paul Eggert 2012-08-25 4:41 ` Paul Eggert 2012-08-25 5:57 ` Eli Zaretskii 2012-08-22 9:28 ` inlinable functions instead of macros Paul Eggert 2012-08-22 9:48 ` Andreas Schwab 2012-08-22 14:31 ` Stefan Monnier 2012-08-24 21:37 ` Paul Eggert 2012-08-25 1:38 ` Tom Tromey 2012-08-25 2:58 ` Paul Eggert 2012-08-25 5:35 ` Eli Zaretskii 2012-08-25 20:13 ` Tom Tromey 2012-08-26 4:42 ` Paul Eggert 2012-08-25 2:05 ` Stefan Monnier 2012-08-26 5:31 ` Paul Eggert 2012-08-22 16:03 ` Eli Zaretskii 2012-08-18 19:08 ` Richard Stallman 2012-08-18 20:57 ` Paul Eggert 2012-08-18 21:03 ` Eli Zaretskii 2012-08-18 21:31 ` Paul Eggert 2012-08-19 2:45 ` Eli Zaretskii 2012-08-18 21:14 ` Óscar Fuentes 2012-08-19 17:28 ` Florian Weimer 2012-08-20 20:38 ` Sam Steingold
Code repositories for project(s) associated with this external index https://git.savannah.gnu.org/cgit/emacs.git https://git.savannah.gnu.org/cgit/emacs/org-mode.git This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.