[patch] Do not disappoint on "Create a core file of GDB?"

[patch] Do not disappoint on "Create a core file of GDB?"

[Jan Kratochvil]

Hi,

this is a FAQ, GDB asks "Create a core file of GDB?", user types "y" but
a core file is never created.

It is because distros (at least Fedora) commonly have:
	ulimit -S -c 0
	ulimit -H -c unlimited

and GDB does not raise its soft limit to hard limit.

But common applications also do not raise it so why should GDB?  Soft limit is
there to act as default value for the application.  GDB is different as it
explicitly asks the user - in such case it should IMO raise the core limit.

Even in the case of "maint set internal-error corefile yes" it had to be set
explicitly by the user as the default is "ask", therefore it should override
the soft core limit.

setrlimit is a POSIX function so I hope it does not need autoconf magic:
	http://www.opengroup.org/onlinepubs/009695399/functions/setrlimit.html

No regressions on {x86_64,x86_64-m32,i686}-fedora12-linux-gnu.


Thanks,
Jan


2010-01-11  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* utils.c: Include sys/resource.h.
	(dump_core, can_dump_core): New.
	(internal_vproblem): Update the comment.  Check can_dump_core while
	setting dump_core_p.  Replace two abort calls by dump_core calls.

--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -26,6 +26,7 @@
 #include "event-top.h"
 #include "exceptions.h"
 #include "gdbthread.h"
+#include <sys/resource.h>
 
 #ifdef TUI
 #include "tui/tui.h"		/* For tui_get_command_dimension.   */
@@ -843,6 +844,40 @@ error_stream (struct ui_file *stream)
   error (("%s"), message);
 }
 
+/* Dump core trying to increase the core soft limit to hard limit first.  */
+
+static void
+dump_core (void)
+{
+  struct rlimit rlim = { RLIM_INFINITY, RLIM_INFINITY };
+
+  setrlimit (RLIMIT_CORE, &rlim);
+
+  abort ();		/* NOTE: GDB has only three calls to abort().  */
+}
+
+/* Check whether GDB will be able to dump core using the dump_core function.  */
+
+static int
+can_dump_core (const char *reason)
+{
+  struct rlimit rlim;
+
+  /* Be quiet and assume we can dump if an error is returned.  */
+  if (getrlimit (RLIMIT_CORE, &rlim) != 0)
+    return 1;
+
+  if (rlim.rlim_max == 0)
+    {
+      fprintf_unfiltered (gdb_stderr,
+			  _("%s\nUnable to dump core, use `ulimit -c unlimited'"
+			    " before executing GDB next time.\n"), reason);
+      return 0;
+    }
+
+  return 1;
+}
+
 /* Allow the user to configure the debugger behavior with respect to
    what to do when an internal problem is detected.  */
 
@@ -893,7 +928,7 @@ internal_vproblem (struct internal_problem *problem,
       case 1:
 	dejavu = 2;
 	fputs_unfiltered (msg, gdb_stderr);
-	abort ();	/* NOTE: GDB has only four calls to abort().  */
+	abort ();	/* NOTE: GDB has only three calls to abort().  */
       default:
 	dejavu = 3;
         /* Newer GLIBC versions put the warn_unused_result attribute
@@ -902,7 +937,7 @@ internal_vproblem (struct internal_problem *problem,
            does not fix this problem.  This is the solution suggested
            at http://gcc.gnu.org/bugzilla/show_bug.cgi?id=25509.  */
 	if (write (STDERR_FILENO, msg, sizeof (msg)) != sizeof (msg))
-          abort (); /* NOTE: GDB has only four calls to abort().  */
+          abort (); /* NOTE: GDB has only three calls to abort().  */
 	exit (1);
       }
   }
@@ -951,13 +986,18 @@ further debugging may prove unreliable.", file, line, problem->name, msg);
 
   if (problem->should_dump_core == internal_problem_ask)
     {
-      /* Default (yes/batch case) is to dump core.  This leaves a GDB
-         `dropping' so that it is easier to see that something went
-         wrong in GDB.  */
-      dump_core_p = query (_("%s\nCreate a core file of GDB? "), reason);
+      if (!can_dump_core (reason))
+	dump_core_p = 0;
+      else
+	{
+	  /* Default (yes/batch case) is to dump core.  This leaves a GDB
+	     `dropping' so that it is easier to see that something went
+	     wrong in GDB.  */
+	  dump_core_p = query (_("%s\nCreate a core file of GDB? "), reason);
+	}
     }
   else if (problem->should_dump_core == internal_problem_yes)
-    dump_core_p = 1;
+    dump_core_p = can_dump_core (reason);
   else if (problem->should_dump_core == internal_problem_no)
     dump_core_p = 0;
   else
@@ -966,7 +1006,7 @@ further debugging may prove unreliable.", file, line, problem->name, msg);
   if (quit_p)
     {
       if (dump_core_p)
-	abort ();		/* NOTE: GDB has only four calls to abort().  */
+	dump_core ();
       else
 	exit (1);
     }
@@ -976,7 +1016,7 @@ further debugging may prove unreliable.", file, line, problem->name, msg);
 	{
 #ifdef HAVE_WORKING_FORK
 	  if (fork () == 0)
-	    abort ();		/* NOTE: GDB has only four calls to abort().  */
+	    dump_core ();
 #endif
 	}
     }

Re: [patch] Do not disappoint on "Create a core file of GDB?"

[Tristan Gingold]

On Jan 11, 2010, at 5:02 PM, Jan Kratochvil wrote:
Hi,

sorry for the nit but:

> setrlimit is a POSIX function so I hope it does not need autoconf magic:
> 	http://www.opengroup.org/onlinepubs/009695399/functions/setrlimit.html

Sure, but windows is not POSIX compliant, so I think you can't avoid some autoconf stuff.

Tristan.

Re: [patch] Do not disappoint on "Create a core file of GDB?"

[Jan Kratochvil]

On Mon, 11 Jan 2010 17:07:30 +0100, Tristan Gingold wrote:
> 
> On Jan 11, 2010, at 5:02 PM, Jan Kratochvil wrote:
> Hi,
> 
> sorry for the nit but:
> 
> > setrlimit is a POSIX function so I hope it does not need autoconf magic:
> > 	http://www.opengroup.org/onlinepubs/009695399/functions/setrlimit.html
> 
> Sure, but windows is not POSIX compliant, so I think you can't avoid some
> autoconf stuff.

OK.

The patch has now no effect on platforms without setrlimit / getrlimit.


Thanks,
Jan


2010-01-11  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* configure.ac (AC_CHECK_FUNCS): Check for setrlimit and getrlimit.
	* configure: Regenerate.
	* config.in: Regenerate.
	* utils.c: Include sys/resource.h.
	(dump_core, can_dump_core): New.
	(internal_vproblem): Update the comment.  Check can_dump_core while
	setting dump_core_p.  Replace two abort calls by dump_core calls.

--- a/gdb/configure.ac
+++ b/gdb/configure.ac
@@ -801,7 +801,8 @@ AC_FUNC_VFORK
 AC_CHECK_FUNCS([canonicalize_file_name realpath getrusage getuid \
 		getgid pipe poll pread64 sbrk setpgid setpgrp setsid \
 		sigaction sigprocmask sigsetmask socketpair syscall \
-		ttrace wborder setlocale iconvlist libiconvlist btowc])
+		ttrace wborder setlocale iconvlist libiconvlist btowc \
+		setrlimit getrlimit])
 AM_LANGINFO_CODESET
 
 # Check the return and argument types of ptrace.  No canned test for
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -26,6 +26,9 @@
 #include "event-top.h"
 #include "exceptions.h"
 #include "gdbthread.h"
+#ifdef HAVE_SYS_RESOURCE_H
+#include <sys/resource.h>
+#endif /* HAVE_SYS_RESOURCE_H */
 
 #ifdef TUI
 #include "tui/tui.h"		/* For tui_get_command_dimension.   */
@@ -843,6 +844,44 @@ error_stream (struct ui_file *stream)
   error (("%s"), message);
 }
 
+/* Dump core trying to increase the core soft limit to hard limit first.  */
+
+static void
+dump_core (void)
+{
+#ifdef HAVE_SETRLIMIT
+  struct rlimit rlim = { RLIM_INFINITY, RLIM_INFINITY };
+
+  setrlimit (RLIMIT_CORE, &rlim);
+#endif /* HAVE_SETRLIMIT */
+
+  abort ();		/* NOTE: GDB has only three calls to abort().  */
+}
+
+/* Check whether GDB will be able to dump core using the dump_core function.  */
+
+static int
+can_dump_core (const char *reason)
+{
+#ifdef HAVE_GETRLIMIT
+  struct rlimit rlim;
+
+  /* Be quiet and assume we can dump if an error is returned.  */
+  if (getrlimit (RLIMIT_CORE, &rlim) != 0)
+    return 1;
+
+  if (rlim.rlim_max == 0)
+    {
+      fprintf_unfiltered (gdb_stderr,
+			  _("%s\nUnable to dump core, use `ulimit -c unlimited'"
+			    " before executing GDB next time.\n"), reason);
+      return 0;
+    }
+#endif /* HAVE_GETRLIMIT */
+
+  return 1;
+}
+
 /* Allow the user to configure the debugger behavior with respect to
    what to do when an internal problem is detected.  */
 
@@ -893,7 +932,7 @@ internal_vproblem (struct internal_problem *problem,
       case 1:
 	dejavu = 2;
 	fputs_unfiltered (msg, gdb_stderr);
-	abort ();	/* NOTE: GDB has only four calls to abort().  */
+	abort ();	/* NOTE: GDB has only three calls to abort().  */
       default:
 	dejavu = 3;
         /* Newer GLIBC versions put the warn_unused_result attribute
@@ -902,7 +941,7 @@ internal_vproblem (struct internal_problem *problem,
            does not fix this problem.  This is the solution suggested
            at http://gcc.gnu.org/bugzilla/show_bug.cgi?id=25509.  */
 	if (write (STDERR_FILENO, msg, sizeof (msg)) != sizeof (msg))
-          abort (); /* NOTE: GDB has only four calls to abort().  */
+          abort (); /* NOTE: GDB has only three calls to abort().  */
 	exit (1);
       }
   }
@@ -951,13 +990,18 @@ further debugging may prove unreliable.", file, line, problem->name, msg);
 
   if (problem->should_dump_core == internal_problem_ask)
     {
-      /* Default (yes/batch case) is to dump core.  This leaves a GDB
-         `dropping' so that it is easier to see that something went
-         wrong in GDB.  */
-      dump_core_p = query (_("%s\nCreate a core file of GDB? "), reason);
+      if (!can_dump_core (reason))
+	dump_core_p = 0;
+      else
+	{
+	  /* Default (yes/batch case) is to dump core.  This leaves a GDB
+	     `dropping' so that it is easier to see that something went
+	     wrong in GDB.  */
+	  dump_core_p = query (_("%s\nCreate a core file of GDB? "), reason);
+	}
     }
   else if (problem->should_dump_core == internal_problem_yes)
-    dump_core_p = 1;
+    dump_core_p = can_dump_core (reason);
   else if (problem->should_dump_core == internal_problem_no)
     dump_core_p = 0;
   else
@@ -966,7 +1010,7 @@ further debugging may prove unreliable.", file, line, problem->name, msg);
   if (quit_p)
     {
       if (dump_core_p)
-	abort ();		/* NOTE: GDB has only four calls to abort().  */
+	dump_core ();
       else
 	exit (1);
     }
@@ -976,7 +1020,7 @@ further debugging may prove unreliable.", file, line, problem->name, msg);
 	{
 #ifdef HAVE_WORKING_FORK
 	  if (fork () == 0)
-	    abort ();		/* NOTE: GDB has only four calls to abort().  */
+	    dump_core ();
 #endif
 	}
     }

Re: [patch] Do not disappoint on "Create a core file of GDB?"

[Eli Zaretskii]

> From: Tristan Gingold <gingold@adacore.com>
> Date: Mon, 11 Jan 2010 17:07:30 +0100
> Cc: gdb-patches@sourceware.org
> 
> 
> > setrlimit is a POSIX function so I hope it does not need autoconf magic:
> > 	http://www.opengroup.org/onlinepubs/009695399/functions/setrlimit.html
> 
> Sure, but windows is not POSIX compliant, so I think you can't avoid some autoconf stuff.

Which "windows" do you have in mind?  If it's Cygwin, I'd imagine it
does have setrlimit.  If you are talking about native Windows, then it
cannot dump core, anyway, can it?

Re: [patch] Do not disappoint on "Create a core file of GDB?"

[Tristan Gingold]

On Jan 11, 2010, at 8:15 PM, Eli Zaretskii wrote:

>> From: Tristan Gingold <gingold@adacore.com>
>> Date: Mon, 11 Jan 2010 17:07:30 +0100
>> Cc: gdb-patches@sourceware.org
>> 
>> 
>>> setrlimit is a POSIX function so I hope it does not need autoconf magic:
>>> 	http://www.opengroup.org/onlinepubs/009695399/functions/setrlimit.html
>> 
>> Sure, but windows is not POSIX compliant, so I think you can't avoid some autoconf stuff.
> 
> Which "windows" do you have in mind?  If it's Cygwin, I'd imagine it
> does have setrlimit.  If you are talking about native Windows, then it
> cannot dump core, anyway, can it?

Sure.  But it doesn't have setrlimit/getrlimit.

Re: [patch] Do not disappoint on "Create a core file of GDB?"

[Tom Tromey]

Jan> this is a FAQ, GDB asks "Create a core file of GDB?", user types
Jan> "y" but a core file is never created.
[...]
Jan> Even in the case of "maint set internal-error corefile yes" it had
Jan> to be set explicitly by the user as the default is "ask", therefore
Jan> it should override the soft core limit.

This seems reasonable.

Jan> 2010-01-11  Jan Kratochvil  <jan.kratochvil@redhat.com>
Jan> 	* configure.ac (AC_CHECK_FUNCS): Check for setrlimit and getrlimit.
Jan> 	* configure: Regenerate.
Jan> 	* config.in: Regenerate.
Jan> 	* utils.c: Include sys/resource.h.
Jan> 	(dump_core, can_dump_core): New.
Jan> 	(internal_vproblem): Update the comment.  Check can_dump_core while
Jan> 	setting dump_core_p.  Replace two abort calls by dump_core calls.

Looks good to me.
Ok.

Tom

Re: [patch] Do not disappoint on "Create a core file of GDB?"

[Eli Zaretskii]

> From: Tristan Gingold <gingold@adacore.com>
> Date: Tue, 12 Jan 2010 09:45:29 +0100
> Cc: jan.kratochvil@redhat.com,  gdb-patches@sourceware.org
> 
> 
> On Jan 11, 2010, at 8:15 PM, Eli Zaretskii wrote:
> 
> >> From: Tristan Gingold <gingold@adacore.com>
> >> Date: Mon, 11 Jan 2010 17:07:30 +0100
> >> Cc: gdb-patches@sourceware.org
> >> 
> >> 
> >>> setrlimit is a POSIX function so I hope it does not need autoconf magic:
> >>> 	http://www.opengroup.org/onlinepubs/009695399/functions/setrlimit.html
> >> 
> >> Sure, but windows is not POSIX compliant, so I think you can't avoid some autoconf stuff.
> > 
> > Which "windows" do you have in mind?  If it's Cygwin, I'd imagine it
> > does have setrlimit.  If you are talking about native Windows, then it
> > cannot dump core, anyway, can it?
> 
> Sure.  But it doesn't have setrlimit/getrlimit.

But of course.

In case I wasn't clear, I was actually wondering how come we compile a
source file that's needed for reading and writing core files on a
system where core files aren't supported.  We shouldn't compile it,
and this problem should not exist.

Re: [patch] Do not disappoint on "Create a core file of GDB?"

[Jan Kratochvil]

On Tue, 12 Jan 2010 20:37:50 +0100, Eli Zaretskii wrote:
> In case I wasn't clear, I was actually wondering how come we compile a
> source file that's needed for reading and writing core files on a
> system where core files aren't supported.  We shouldn't compile it,
> and this problem should not exist.

This code is about invoking system default SIGABRT action which is usually to
dump a core.  Calling abort() is simple enough it IMO does not make sense to
move it to some host/target-dependent file.

You are probably suggesting that GDB could fork, attaching to its copy (in
a new inferior) and invoke gcore on the copy.  This would be a functionality
enabled only for native targets (*-nat.c) so that it could dump core even on
operating systems not supporting such operation.

It would be a feature currently unsupported by GDB and out of the scope of
this patch.  Still the system core dumping would be preferred over this
GDB-native dump as it is more safe for half-crashed GDB.  Therefore such
functionality would have limited audience.


Regards,
Jan

Re: [patch] Do not disappoint on "Create a core file of GDB?"

[Jan Kratochvil]

On Tue, 12 Jan 2010 17:58:48 +0100, Tom Tromey wrote:
> Jan> 2010-01-11  Jan Kratochvil  <jan.kratochvil@redhat.com>
> Jan> 	* configure.ac (AC_CHECK_FUNCS): Check for setrlimit and getrlimit.
> Jan> 	* configure: Regenerate.
> Jan> 	* config.in: Regenerate.
> Jan> 	* utils.c: Include sys/resource.h.
> Jan> 	(dump_core, can_dump_core): New.
> Jan> 	(internal_vproblem): Update the comment.  Check can_dump_core while
> Jan> 	setting dump_core_p.  Replace two abort calls by dump_core calls.
> 
> Looks good to me.
> Ok.

Checked-in.


Thanks,
Jan


http://sourceware.org/ml/gdb-cvs/2010-01/msg00131.html

--- src/gdb/ChangeLog	2010/01/14 21:24:04	1.11253
+++ src/gdb/ChangeLog	2010/01/15 00:34:37	1.11254
@@ -1,3 +1,13 @@
+2010-01-15  Jan Kratochvil  <jan.kratochvil@redhat.com>
+
+	* configure.ac (AC_CHECK_FUNCS): Check for setrlimit and getrlimit.
+	* configure: Regenerate.
+	* config.in: Regenerate.
+	* utils.c: Include sys/resource.h.
+	(dump_core, can_dump_core): New.
+	(internal_vproblem): Update the comment.  Check can_dump_core while
+	setting dump_core_p.  Replace two abort calls by dump_core calls.
+
 2010-01-14  Jan Kratochvil  <jan.kratochvil@redhat.com>
 	    Eli Zaretskii  <eliz@gnu.org>
 
--- src/gdb/config.in	2009/11/11 04:42:39	1.114
+++ src/gdb/config.in	2010/01/15 00:34:37	1.115
@@ -165,6 +165,9 @@
 /* Define to 1 if you have the `getpagesize' function. */
 #undef HAVE_GETPAGESIZE
 
+/* Define to 1 if you have the `getrlimit' function. */
+#undef HAVE_GETRLIMIT
+
 /* Define to 1 if you have the `getrusage' function. */
 #undef HAVE_GETRUSAGE
 
@@ -366,6 +369,9 @@
 /* Define to 1 if you have the `setpgrp' function. */
 #undef HAVE_SETPGRP
 
+/* Define to 1 if you have the `setrlimit' function. */
+#undef HAVE_SETRLIMIT
+
 /* Define to 1 if you have the `setsid' function. */
 #undef HAVE_SETSID
 
--- src/gdb/configure	2010/01/12 12:22:52	1.296
+++ src/gdb/configure	2010/01/15 00:34:37	1.297
@@ -11233,7 +11233,8 @@
 for ac_func in canonicalize_file_name realpath getrusage getuid \
 		getgid pipe poll pread64 sbrk setpgid setpgrp setsid \
 		sigaction sigprocmask sigsetmask socketpair syscall \
-		ttrace wborder setlocale iconvlist libiconvlist btowc
+		ttrace wborder setlocale iconvlist libiconvlist btowc \
+		setrlimit getrlimit
 do :
   as_ac_var=`$as_echo "ac_cv_func_$ac_func" | $as_tr_sh`
 ac_fn_c_check_func "$LINENO" "$ac_func" "$as_ac_var"
--- src/gdb/configure.ac	2010/01/01 09:44:06	1.111
+++ src/gdb/configure.ac	2010/01/15 00:34:37	1.112
@@ -801,7 +801,8 @@
 AC_CHECK_FUNCS([canonicalize_file_name realpath getrusage getuid \
 		getgid pipe poll pread64 sbrk setpgid setpgrp setsid \
 		sigaction sigprocmask sigsetmask socketpair syscall \
-		ttrace wborder setlocale iconvlist libiconvlist btowc])
+		ttrace wborder setlocale iconvlist libiconvlist btowc \
+		setrlimit getrlimit])
 AM_LANGINFO_CODESET
 
 # Check the return and argument types of ptrace.  No canned test for
--- src/gdb/utils.c	2010/01/12 21:40:24	1.221
+++ src/gdb/utils.c	2010/01/15 00:34:37	1.222
@@ -26,6 +26,9 @@
 #include "event-top.h"
 #include "exceptions.h"
 #include "gdbthread.h"
+#ifdef HAVE_SYS_RESOURCE_H
+#include <sys/resource.h>
+#endif /* HAVE_SYS_RESOURCE_H */
 
 #ifdef TUI
 #include "tui/tui.h"		/* For tui_get_command_dimension.   */
@@ -843,6 +846,44 @@
   error (("%s"), message);
 }
 
+/* Dump core trying to increase the core soft limit to hard limit first.  */
+
+static void
+dump_core (void)
+{
+#ifdef HAVE_SETRLIMIT
+  struct rlimit rlim = { RLIM_INFINITY, RLIM_INFINITY };
+
+  setrlimit (RLIMIT_CORE, &rlim);
+#endif /* HAVE_SETRLIMIT */
+
+  abort ();		/* NOTE: GDB has only three calls to abort().  */
+}
+
+/* Check whether GDB will be able to dump core using the dump_core function.  */
+
+static int
+can_dump_core (const char *reason)
+{
+#ifdef HAVE_GETRLIMIT
+  struct rlimit rlim;
+
+  /* Be quiet and assume we can dump if an error is returned.  */
+  if (getrlimit (RLIMIT_CORE, &rlim) != 0)
+    return 1;
+
+  if (rlim.rlim_max == 0)
+    {
+      fprintf_unfiltered (gdb_stderr,
+			  _("%s\nUnable to dump core, use `ulimit -c unlimited'"
+			    " before executing GDB next time.\n"), reason);
+      return 0;
+    }
+#endif /* HAVE_GETRLIMIT */
+
+  return 1;
+}
+
 /* Allow the user to configure the debugger behavior with respect to
    what to do when an internal problem is detected.  */
 
@@ -893,7 +934,7 @@
       case 1:
 	dejavu = 2;
 	fputs_unfiltered (msg, gdb_stderr);
-	abort ();	/* NOTE: GDB has only four calls to abort().  */
+	abort ();	/* NOTE: GDB has only three calls to abort().  */
       default:
 	dejavu = 3;
         /* Newer GLIBC versions put the warn_unused_result attribute
@@ -902,7 +943,7 @@
            does not fix this problem.  This is the solution suggested
            at http://gcc.gnu.org/bugzilla/show_bug.cgi?id=25509.  */
 	if (write (STDERR_FILENO, msg, sizeof (msg)) != sizeof (msg))
-          abort (); /* NOTE: GDB has only four calls to abort().  */
+          abort (); /* NOTE: GDB has only three calls to abort().  */
 	exit (1);
       }
   }
@@ -951,13 +992,18 @@
 
   if (problem->should_dump_core == internal_problem_ask)
     {
-      /* Default (yes/batch case) is to dump core.  This leaves a GDB
-         `dropping' so that it is easier to see that something went
-         wrong in GDB.  */
-      dump_core_p = query (_("%s\nCreate a core file of GDB? "), reason);
+      if (!can_dump_core (reason))
+	dump_core_p = 0;
+      else
+	{
+	  /* Default (yes/batch case) is to dump core.  This leaves a GDB
+	     `dropping' so that it is easier to see that something went
+	     wrong in GDB.  */
+	  dump_core_p = query (_("%s\nCreate a core file of GDB? "), reason);
+	}
     }
   else if (problem->should_dump_core == internal_problem_yes)
-    dump_core_p = 1;
+    dump_core_p = can_dump_core (reason);
   else if (problem->should_dump_core == internal_problem_no)
     dump_core_p = 0;
   else
@@ -966,7 +1012,7 @@
   if (quit_p)
     {
       if (dump_core_p)
-	abort ();		/* NOTE: GDB has only four calls to abort().  */
+	dump_core ();
       else
 	exit (1);
     }
@@ -976,7 +1022,7 @@
 	{
 #ifdef HAVE_WORKING_FORK
 	  if (fork () == 0)
-	    abort ();		/* NOTE: GDB has only four calls to abort().  */
+	    dump_core ();
 #endif
 	}
     }

Re: [patch] Do not disappoint on "Create a core file of GDB?"

[Eli Zaretskii]

> Date: Fri, 15 Jan 2010 01:28:05 +0100
> From: Jan Kratochvil <jan.kratochvil@redhat.com>
> Cc: Tristan Gingold <gingold@adacore.com>, gdb-patches@sourceware.org
> 
> On Tue, 12 Jan 2010 20:37:50 +0100, Eli Zaretskii wrote:
> > In case I wasn't clear, I was actually wondering how come we compile a
> > source file that's needed for reading and writing core files on a
> > system where core files aren't supported.  We shouldn't compile it,
> > and this problem should not exist.
> 
> This code is about invoking system default SIGABRT action which is usually to
> dump a core.  Calling abort() is simple enough it IMO does not make sense to
> move it to some host/target-dependent file.

I don't want to start a dispute, just to make sure my intent is clear.

We are calling `abort' here with a very specific purpose: producing a
core file.  On systems that don't support core files, calling `abort'
in this context does not make sense, even though `bort' is an ANSI
function and should be supported by any system where GDB can be built.
So the code which does that should not be compiled on such systems,
IMO.  Ifdef'ing it away conditioned by such systems is much easier
than introducing configury, whose results we know in advance.

Re: [patch] Do not disappoint on "Create a core file of GDB?"

[Mark Kettenis]

> Date: Fri, 15 Jan 2010 01:35:29 +0100
> From: Jan Kratochvil <jan.kratochvil@redhat.com>
> 
> On Tue, 12 Jan 2010 17:58:48 +0100, Tom Tromey wrote:
> > Jan> 2010-01-11  Jan Kratochvil  <jan.kratochvil@redhat.com>
> > Jan> 	* configure.ac (AC_CHECK_FUNCS): Check for setrlimit and getrlimit.
> > Jan> 	* configure: Regenerate.
> > Jan> 	* config.in: Regenerate.
> > Jan> 	* utils.c: Include sys/resource.h.
> > Jan> 	(dump_core, can_dump_core): New.
> > Jan> 	(internal_vproblem): Update the comment.  Check can_dump_core while
> > Jan> 	setting dump_core_p.  Replace two abort calls by dump_core calls.
> > 
> > Looks good to me.
> > Ok.
> 
> Checked-in.

I fear that after you did that the "NOTE: GDB has only three calls to
abort()." comment is no longer true.

Can you fix that?

Re: [patch] Do not disappoint on "Create a core file of GDB?"

[Jan Kratochvil]

On Sat, 30 Jan 2010 14:05:06 +0100, Mark Kettenis wrote:
> I fear that after you did that the "NOTE: GDB has only three calls to
> abort()." comment is no longer true.

There were four calls and there are three calls now.  The message has been
updated:
http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/gdb/utils.c.diff?cvsroot=src&r1=1.221&r2=1.222
+  abort ();		/* NOTE: GDB has only three calls to abort().  */
-	abort ();	/* NOTE: GDB has only four calls to abort().  */
+	abort ();	/* NOTE: GDB has only three calls to abort().  */
-          abort (); /* NOTE: GDB has only four calls to abort().  */
+          abort (); /* NOTE: GDB has only three calls to abort().  */
-	abort ();		/* NOTE: GDB has only four calls to abort().  */
-	abort ();		/* NOTE: GDB has only four calls to abort().  */

and current HEAD:
$ grep 'GDB has only.*abort' *
utils.c:  abort ();		/* NOTE: GDB has only three calls to abort().  */
utils.c:	abort ();	/* NOTE: GDB has only three calls to abort().  */
utils.c:          abort (); /* NOTE: GDB has only three calls to abort().  */

The ChangeLog entry
http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/gdb/ChangeLog.diff?cvsroot=src&r1=1.11253&r2=1.11254
describes thise "four"->"three" word change by:
	(internal_vproblem): Update the comment.
which may have been insufficient, sorry.


Thanks,
Jan

Re: [patch] Do not disappoint on "Create a core file of GDB?"

[Mark Kettenis]

> Date: Sat, 30 Jan 2010 15:47:23 +0100
> From: Jan Kratochvil <jan.kratochvil@redhat.com>
> 
> On Sat, 30 Jan 2010 14:05:06 +0100, Mark Kettenis wrote:
> > I fear that after you did that the "NOTE: GDB has only three calls to
> > abort()." comment is no longer true.
> 
> There were four calls and there are three calls now.  The message has been
> updated:
> http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/gdb/utils.c.diff?cvsroot=src&r1=1.221&r2=1.222
> +  abort ();		/* NOTE: GDB has only three calls to abort().  */
> -	abort ();	/* NOTE: GDB has only four calls to abort().  */
> +	abort ();	/* NOTE: GDB has only three calls to abort().  */
> -          abort (); /* NOTE: GDB has only four calls to abort().  */
> +          abort (); /* NOTE: GDB has only three calls to abort().  */
> -	abort ();		/* NOTE: GDB has only four calls to abort().  */
> -	abort ();		/* NOTE: GDB has only four calls to abort().  */
> 
> and current HEAD:
> $ grep 'GDB has only.*abort' *
> utils.c:  abort ();		/* NOTE: GDB has only three calls to abort().  */
> utils.c:	abort ();	/* NOTE: GDB has only three calls to abort().  */
> utils.c:          abort (); /* NOTE: GDB has only three calls to abort().  */
> 
> The ChangeLog entry
> http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/gdb/ChangeLog.diff?cvsroot=src&r1=1.11253&r2=1.11254
> describes thise "four"->"three" word change by:
> 	(internal_vproblem): Update the comment.
> which may have been insufficient, sorry.

Hmm, my diff-reading skills may have been affected by jetlag.  Sorry
for the noise!

RE: [patch] Do not disappoint on "Create a core file of GDB?"

[Pierre Muller]

Nevertheless, the patch
http://sourceware.org/ml/gdb-cvs/2010-01/msg00131.html
caused a re-entry of 'abort' rule in the ARI critical at
http://sourceware.org/gdb/current/ari/

Because the script gdb_ari.sh expects exactly 4 occurences of abort()
in gdb/utils.c source.

 As the fix is rather obvious (see below),
I committed it as obvious.

Pierre Muller 
as ARI maintainer...
(not very active on that part lately :().

===================================================================
RCS file: /cvs/gdbadmin/ss/gdb_ari.sh,v
retrieving revision 1.99
diff -c -r1.99 gdb_ari.sh
*** gdb_ari.sh  6 Oct 2009 21:15:08 -0000       1.99
--- gdb_ari.sh  31 Mar 2010 13:27:38 -0000
***************
*** 637,643 ****
  BEGIN { doc["abort"] = "\
  Do not use abort, instead use internal_error; GDB should never abort"
      category["abort"] = ari_regression
!     fix("abort", "gdb/utils.c", 4)
  }
  /(^|[^_[:alnum:]])abort[[:space:]]*\(/ {
      fail("abort")
--- 637,643 ----
  BEGIN { doc["abort"] = "\
  Do not use abort, instead use internal_error; GDB should never abort"
      category["abort"] = ari_regression
!     fix("abort", "gdb/utils.c", 3)
  }
  /(^|[^_[:alnum:]])abort[[:space:]]*\(/ {
      fail("abort")

> -----Message d'origine-----
> De : gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] De la part de Mark Kettenis
> Envoyé : Sunday, January 31, 2010 3:13 PM
> À : jan.kratochvil@redhat.com
> Cc : gdb-patches@sourceware.org
> Objet : Re: [patch] Do not disappoint on "Create a core file of GDB?"
> 
> > Date: Sat, 30 Jan 2010 15:47:23 +0100
> > From: Jan Kratochvil <jan.kratochvil@redhat.com>
> >
> > On Sat, 30 Jan 2010 14:05:06 +0100, Mark Kettenis wrote:
> > > I fear that after you did that the "NOTE: GDB has only three calls
> to
> > > abort()." comment is no longer true.
> >
> > There were four calls and there are three calls now.  The message has
> been
> > updated:
> > http://sources.redhat.com/cgi-
> bin/cvsweb.cgi/src/gdb/utils.c.diff?cvsroot=src&r1=1.221&r2=1.222
> > +  abort ();		/* NOTE: GDB has only three calls to
abort().
> */
> > -	abort ();	/* NOTE: GDB has only four calls to abort().  */
> > +	abort ();	/* NOTE: GDB has only three calls to abort().  */
> > -          abort (); /* NOTE: GDB has only four calls to abort().  */
> > +          abort (); /* NOTE: GDB has only three calls to abort().
> */
> > -	abort ();		/* NOTE: GDB has only four calls to abort().
> */
> > -	abort ();		/* NOTE: GDB has only four calls to abort().
> */
> >
> > and current HEAD:
> > $ grep 'GDB has only.*abort' *
> > utils.c:  abort ();		/* NOTE: GDB has only three calls to
> abort().  */
> > utils.c:	abort ();	/* NOTE: GDB has only three calls to
abort().
> */
> > utils.c:          abort (); /* NOTE: GDB has only three calls to
> abort().  */
> >
> > The ChangeLog entry
> > http://sources.redhat.com/cgi-
> bin/cvsweb.cgi/src/gdb/ChangeLog.diff?cvsroot=src&r1=1.11253&r2=1.11254
> > describes thise "four"->"three" word change by:
> > 	(internal_vproblem): Update the comment.
> > which may have been insufficient, sorry.
> 
> Hmm, my diff-reading skills may have been affected by jetlag.  Sorry
> for the noise!