Subject: bug#12316: closed (Re: Simplify redefinition of 'abort'.)

From: [email-masked] (GNU bug Tracking System)
Date: Tue, 04 Sep 2012 17:37:02 +0000

Your bug report

#12316: Simplify redefinition of 'abort'.

which was filed against the emacs package, has been closed.

The explanation is attached below, along with your original report.
If you require more details, please reply to [email-masked].

-- 
12316: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=12316
GNU Bug Tracking System
Contact [email-masked] with problems

Subject: Re: Simplify redefinition of 'abort'.
From: Paul Eggert <[email-masked]>
To: Eli Zaretskii <[email-masked]>
Cc: [email-masked], [email-masked]
Date: Tue, 04 Sep 2012 10:36:17 -0700
List-Id: <debbugs-submit.debbugs.gnu.org>

I've installed that patch and am marking this bug as done.

Subject: Simplify redefinition of 'abort'.
From: Paul Eggert <[email-masked]>
To: [email-masked]
Cc: Juanma Barranquero <[email-masked]>, Eli Zaretskii <[email-masked]>
Date: Thu, 30 Aug 2012 20:14:51 -0700
List-Id: <debbugs-submit.debbugs.gnu.org>

Tags: patch

Here's a patch I'd like to install after a bit more testing.
It simplifies how 'abort' is configured, on POSIX hosts.
This affects the Windows port but I haven't tested that part.
I'm CC'ing this to Eli and Juanma to give them a heads-up.

=== modified file 'ChangeLog'
--- ChangeLog	2012-08-28 16:01:59 +0000
+++ ChangeLog	2012-08-31 03:06:13 +0000
@@ -1,3 +1,8 @@
+2012-08-31  Paul Eggert  <[email-masked]>
+
+	Simplify redefinition of 'abort'.
+	* configure.ac (NO_ABRT): Remove.
+
 2012-08-26  Paul Eggert  <[email-masked]>
 
 	* configure.ac (CFLAGS): Prefer -g3 to -g if -g3 works

=== modified file 'admin/CPP-DEFINES'
--- admin/CPP-DEFINES	2012-08-18 21:36:13 +0000
+++ admin/CPP-DEFINES	2012-08-31 03:06:13 +0000
@@ -428,7 +428,6 @@
 MAIL_USE_SYSTEM_LOCK
 MAXPATHLEN
 NLIST_STRUCT
-NO_ABORT
 NO_EDITRES
 NO_MATHERR
 NO_TERMIO

=== modified file 'admin/ChangeLog'
--- admin/ChangeLog	2012-08-28 19:26:41 +0000
+++ admin/ChangeLog	2012-08-31 03:06:13 +0000
@@ -1,3 +1,8 @@
+2012-08-31  Paul Eggert  <[email-masked]>
+
+	Simplify redefinition of 'abort'.
+	* CPP-DEFINES (NO_ABORT): Remove.
+
 2012-08-28  Glenn Morris  <[email-masked]>
 
 	* bzrmerge.el (bzrmerge-merges): Allow unversioned files in the tree.

=== modified file 'configure.ac'
--- configure.ac	2012-08-28 16:01:59 +0000
+++ configure.ac	2012-08-31 03:06:13 +0000
@@ -3321,12 +3321,6 @@
     AC_DEFINE(BROKEN_PTY_READ_AFTER_EAGAIN, 1, [Define on FreeBSD to
       work around an issue when reading from a PTY.])
     ;;
-
-  dnl Define the following so emacs symbols will not conflict with those
-  dnl in the System framework.  Otherwise -prebind will not work.
-  darwin)
-    AC_DEFINE(NO_ABORT, 1, [Do not define abort in emacs.c.])
-    ;;
 esac
 
 case $opsys in

=== modified file 'nt/ChangeLog'
--- nt/ChangeLog	2012-08-28 16:01:59 +0000
+++ nt/ChangeLog	2012-08-31 03:06:13 +0000
@@ -1,3 +1,9 @@
+2012-08-31  Paul Eggert  <[email-masked]>
+
+	Simplify redefinition of 'abort'.
+	* inc/ms-w32.h (w32_abort): Remove.
+	* inc/unistd.h: Include <stdlib.h>, for 'environ'.
+
 2012-08-22  Juanma Barranquero  <[email-masked]>
 
 	* config.nt: Sync with autogen/config.in.

=== modified file 'nt/inc/ms-w32.h'
--- nt/inc/ms-w32.h	2012-08-07 11:03:48 +0000
+++ nt/inc/ms-w32.h	2012-08-31 03:06:13 +0000
@@ -334,16 +334,7 @@
 #include <malloc.h>
 #endif
 
-/* stdlib.h must be included after redefining malloc & friends, but
-   before redefining abort.  Isn't library redefinition funny?  */
 #include <stdlib.h>
-
-/* Redefine abort.  */
-#ifdef HAVE_NTGUI
-#define abort	w32_abort
-extern _Noreturn void w32_abort (void);
-#endif
-
 #include <sys/stat.h>
 
 /* Define for those source files that do not include enough NT system files.  */

=== modified file 'nt/inc/unistd.h'
--- nt/inc/unistd.h	2011-02-27 19:48:31 +0000
+++ nt/inc/unistd.h	2012-08-31 03:06:13 +0000
@@ -3,8 +3,9 @@
 #ifndef _UNISTD_H
 #define _UNISTD_H
 
+#include <stdlib.h> /* for 'environ' */
+
 extern ssize_t readlink (const char *, char *, size_t);
 extern int symlink (char const *, char const *);
 
 #endif	/* _UNISTD_H */
-

=== modified file 'src/.gdbinit'
--- src/.gdbinit	2012-08-20 17:32:31 +0000
+++ src/.gdbinit	2012-08-31 03:06:13 +0000
@@ -1222,14 +1222,9 @@
   set $tem = (struct Lisp_String *) $ptr
   set $tem = (char *) $tem->data
 
-  # Don't let abort actually run, as it will make stdio stop working and
-  # therefore the `pr' command above as well.
-  if $tem[0] == 'w' && $tem[1] == 'i' && $tem[2] == 'n' && $tem[3] == 'd'
-    # The windows-nt build replaces abort with its own function.
-    break w32_abort
-  else
-    break abort
-  end
+  # Don't let emacs_abort actually run, as it will make stdio stop
+  # working and therefore the 'pr' command above as well.
+  break emacs_abort
 end
 
 # x_error_quitter is defined only on X.  But window-system is set up

=== modified file 'src/ChangeLog'
--- src/ChangeLog	2012-08-30 16:07:44 +0000
+++ src/ChangeLog	2012-08-31 03:06:13 +0000
@@ -1,3 +1,23 @@
+2012-08-31  Paul Eggert  <[email-masked]>
+
+	Simplify redefinition of 'abort'.
+	Do not try to redefine the 'abort' function.  Instead, redo
+	'emacs' so that it calls 'emacs_abort' rather than 'abort'.
+	This removes the need for the NO_ABORT configure-time macro
+	and makes it easier to change the abort code to do a backtrace.
+	* .gdbinit: Just stop at emacs_abort, not at w32_abort or abort.
+	* conf_post.h: Include <string.h> and <stdlib.h> toward the end,
+	so that we can use NO_INLINE here and can define 'abort' cleanly.
+	(emacs_abort) [emacs]: New decl.
+	(abort) [emacs]: New macro.
+	* emacs.c (abort) [!DOS_NT && !NO_ABORT]:
+	Remove; sysdep.c's emacs_abort now takes its place.
+	* msdos.c (dos_abort) [defined abort]: Remove; not used.
+	(abort) [!defined abort]: Rename to ...
+	(emacs_abort) [HAVE_NTGUI]: ... new name.
+	* sysdep.c (emacs_abort) [!HAVE_NTGUI]: New function, taking
+	the place of the old 'abort' in emacs.c.
+
 2012-08-30  Jan Djärv  <[email-masked]>
 
 	* gtkutil.c (x_wm_set_size_hint): Use 1 col for base_width so it

=== modified file 'src/conf_post.h'
--- src/conf_post.h	2012-08-20 16:48:10 +0000
+++ src/conf_post.h	2012-08-31 03:06:13 +0000
@@ -177,12 +177,6 @@
 #endif
 #endif
 
-#include <string.h>
-/* If you think about removing the line below, note that the
-   MS-Windows build relies on it for declaration of 'environ' needed
-   by a few source files.  */
-#include <stdlib.h>
-
 #if __GNUC__ >= 3  /* On GCC 3.0 we might get a warning.  */
 #define NO_INLINE __attribute__((noinline))
 #else
@@ -233,4 +227,14 @@
 # define lint_assume(cond) ((void) (0 && (cond)))
 #endif
 
+#include <string.h>
+#include <stdlib.h>
+/* Under Emacs, arrange for 'abort ()' to call a function that is
+   breakpointable via GDB.  */
+#ifdef emacs
+extern _Noreturn void emacs_abort (void) NO_INLINE;
+# undef abort
+# define abort() emacs_abort ()
+#endif
+
 /* conf_post.h ends here */

=== modified file 'src/emacs.c'
--- src/emacs.c	2012-08-25 06:38:43 +0000
+++ src/emacs.c	2012-08-31 03:10:04 +0000
@@ -344,22 +344,6 @@
   force_auto_save_soon ();
 }
 #endif
-
-/* We define abort, rather than using it from the library,
-   so that GDB can return from a breakpoint here.
-   MSDOS has its own definition in msdos.c.  */
-
-#if ! defined (DOS_NT) && ! defined (NO_ABORT)
-
-void
-abort (void)
-{
-  kill (getpid (), SIGABRT);
-  /* This shouldn't be executed, but it prevents a warning.  */
-  exit (1);
-}
-#endif
-
 
 /* Code for dealing with Lisp access to the Unix command line.  */

=== modified file 'src/msdos.c'
--- src/msdos.c	2012-08-21 10:21:04 +0000
+++ src/msdos.c	2012-08-31 03:06:13 +0000
@@ -4215,26 +4215,9 @@
 }
 #endif
 
-#ifdef abort
-#undef abort
-void
-dos_abort (char *file, int line)
-{
-  char buffer1[200], buffer2[400];
-  int i, j;
-
-  sprintf (buffer1, "<EMACS FATAL ERROR IN %s LINE %d>", file, line);
-  for (i = j = 0; buffer1[i]; i++) {
-    buffer2[j++] = buffer1[i];
-    buffer2[j++] = 0x70;
-  }
-  dosmemput (buffer2, j, (int)ScreenPrimary);
-  ScreenSetCursor (2, 0);
-  abort ();
-}
-#else
-void
-abort (void)
+#ifdef HAVE_NTGUI
+void
+emacs_abort (void)
 {
   dos_ttcooked ();
   ScreenSetCursor (10, 0);

=== modified file 'src/sysdep.c'
--- src/sysdep.c	2012-08-18 02:49:24 +0000
+++ src/sysdep.c	2012-08-31 03:06:13 +0000
@@ -1838,6 +1838,15 @@
 }
 #endif
 
+#ifndef HAVE_NTGUI
+/* Using emacs_abort lets GDB return from a breakpoint here.  */
+void
+emacs_abort (void)
+{
+  (abort) ();
+}
+#endif
+
 int
 emacs_open (const char *path, int oflag, int mode)
 {