RFA: Building GDB under GLIBC 2.8

RFA: Building GDB under GLIBC 2.8

[Jim Blandy]

[-- Attachment #1: Type: text/plain, Size: 450 bytes --]

The latest version of Ubuntu (Intrepid Ibex) includes a GLIBC that
attaches the warn_unused_result attribute to many C library functions.
 The archer project doesn't use the -Wno-unused flag, so I needed to
fix these to build archer.  It seemed to me that submitting the patch
upstream would be the most helpful thing to do.

The first patch simply adds return value checks.  The second patch
actually removes some code which I think is unnecessary.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-gdb.patch --]
[-- Type: text/x-patch; name=0001-gdb.patch, Size: 817 bytes --]

From 95438deda1a4dc5be64d31ed5dd3495060b76147 Mon Sep 17 00:00:00 2001
From: Jim Blandy <jimb@red-bean.com>
Date: Fri, 12 Dec 2008 16:54:06 -0800
Subject: [PATCH] gdb
 	* top.c (gdb_init): Don't set the current directory here; that's
 	already been done in captured_main.

---
 gdb/ChangeLog |    5 +++++
 gdb/top.c     |    3 ---
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/gdb/top.c b/gdb/top.c
index d0ba466..655b7c9 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -1643,9 +1643,6 @@ gdb_init (char *argv0)
 
   /* Run the init function of each source file */
 
-  getcwd (gdb_dirbuf, sizeof (gdb_dirbuf));
-  current_directory = gdb_dirbuf;
-
 #ifdef __MSDOS__
   /* Make sure we return to the original directory upon exit, come
      what may, since the OS doesn't do that for us.  */
-- 
1.5.6.3


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-gdb.patch --]
[-- Type: text/x-patch; name=0002-gdb.patch, Size: 7021 bytes --]

From 708c45c5f92994cd7bc1203f1b804c8d5c1ad90e Mon Sep 17 00:00:00 2001
From: Jim Blandy <jimb@red-bean.com>
Date: Fri, 12 Dec 2008 17:47:01 -0800
Subject: [PATCH] gdb
 	Check return values of functions declared with warn_unused_result
 	attribute in GLIBC 2.8.
 	* cli/cli-cmds.c (pwd_command): Check return value from getcwd.
 	* inflow.c (check_syscall): New function.
 	(new_tty): Use check_syscall to check return values from open and dup.
 	* linux-nat.c (linux_nat_info_proc_cmd): Check return value from fgets.
 	* main.c (captured_main): Call cwd after setting up gdb_stderr;
 	check for errors from getcwd.
 	* mi/mi-cmd-env.c (mi_cmd_env_pwd): Check return value from getcwd.
 	* ui-file.c (stdio_file_write): Check return value from fwrite.
 	(stdio_file_fputs): Check return value from fputs.
 	* utils.c (internal_vproblem): abort if last-ditch error message
 	write fails.

---
 gdb/ChangeLog       |   14 ++++++++++++++
 gdb/cli/cli-cmds.c  |    4 +++-
 gdb/inflow.c        |   22 ++++++++++++++--------
 gdb/linux-nat.c     |    6 ++++--
 gdb/main.c          |   12 +++++++++---
 gdb/mi/mi-cmd-env.c |    5 ++++-
 gdb/ui-file.c       |    6 ++++--
 gdb/utils.c         |    8 +++++++-
 8 files changed, 59 insertions(+), 18 deletions(-)

diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c
index 806a68a..b80bdfc 100644
--- a/gdb/cli/cli-cmds.c
+++ b/gdb/cli/cli-cmds.c
@@ -323,7 +323,9 @@ pwd_command (char *args, int from_tty)
 {
   if (args)
     error (_("The \"pwd\" command does not take an argument: %s"), args);
-  getcwd (gdb_dirbuf, sizeof (gdb_dirbuf));
+  if (! getcwd (gdb_dirbuf, sizeof (gdb_dirbuf)))
+    error (_("Error finding name of working directory: %s"),
+           safe_strerror (errno));
 
   if (strcmp (gdb_dirbuf, current_directory) != 0)
     printf_unfiltered (_("Working directory %s\n (canonically %s).\n"),
diff --git a/gdb/inflow.c b/gdb/inflow.c
index e82514e..7ecb5ab 100644
--- a/gdb/inflow.c
+++ b/gdb/inflow.c
@@ -523,6 +523,16 @@ new_tty_prefork (const char *ttyname)
   inferior_thisrun_terminal = ttyname;
 }
 
+static void
+check_syscall (const char *msg, int result)
+{
+  if (result < 0)
+    {
+      print_sys_errmsg (msg, errno);
+      _exit (1);
+    }
+}
+
 void
 new_tty (void)
 {
@@ -549,27 +559,23 @@ new_tty (void)
 
   /* Now open the specified new terminal.  */
   tty = open (inferior_thisrun_terminal, O_RDWR | O_NOCTTY);
-  if (tty == -1)
-    {
-      print_sys_errmsg (inferior_thisrun_terminal, errno);
-      _exit (1);
-    }
+  check_syscall (inferior_thisrun_terminal, tty);
 
   /* Avoid use of dup2; doesn't exist on all systems.  */
   if (tty != 0)
     {
       close (0);
-      dup (tty);
+      check_syscall ("dup'ing tty into fd 0", dup (tty));
     }
   if (tty != 1)
     {
       close (1);
-      dup (tty);
+      check_syscall ("dup'ing tty into fd 1", dup (tty));
     }
   if (tty != 2)
     {
       close (2);
-      dup (tty);
+      check_syscall ("dup'ing tty into fd 2", dup (tty));
     }
 
 #ifdef TIOCSCTTY
diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index 913bfec..a829eb8 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -3666,8 +3666,10 @@ linux_nat_info_proc_cmd (char *args, int from_tty)
       if ((procfile = fopen (fname1, "r")) != NULL)
 	{
 	  struct cleanup *cleanup = make_cleanup_fclose (procfile);
-	  fgets (buffer, sizeof (buffer), procfile);
-	  printf_filtered ("cmdline = '%s'\n", buffer);
+          if (fgets (buffer, sizeof (buffer), procfile))
+            printf_filtered ("cmdline = '%s'\n", buffer);
+          else
+            warning (_("unable to read '/proc/%lld/cmdline'"), pid);
 	  do_cleanups (cleanup);
 	}
       else
diff --git a/gdb/main.c b/gdb/main.c
index a53002d..a9fd988 100644
--- a/gdb/main.c
+++ b/gdb/main.c
@@ -195,9 +195,6 @@ captured_main (void *data)
   line[0] = '\0';		/* Terminate saved (now empty) cmd line */
   instream = stdin;
 
-  getcwd (gdb_dirbuf, sizeof (gdb_dirbuf));
-  current_directory = gdb_dirbuf;
-
   gdb_stdout = stdio_fileopen (stdout);
   gdb_stderr = stdio_fileopen (stderr);
   gdb_stdlog = gdb_stderr;	/* for moment */
@@ -206,6 +203,15 @@ captured_main (void *data)
   gdb_stdtargerr = gdb_stderr;	/* for moment */
   gdb_stdtargin = gdb_stdin;	/* for moment */
 
+  if (! getcwd (gdb_dirbuf, sizeof (gdb_dirbuf)))
+    /* Don't use *_filtered or warning() (which relies on
+       current_target) until after initialize_all_files(). */
+    fprintf_unfiltered (gdb_stderr,
+                        _("%s: warning: error finding working directory: %s\n"),
+                        argv[0], safe_strerror (errno));
+    
+  current_directory = gdb_dirbuf;
+
   /* Set the sysroot path.  */
 #ifdef TARGET_SYSTEM_ROOT_RELOCATABLE
   gdb_sysroot = make_relative_prefix (argv[0], BINDIR, TARGET_SYSTEM_ROOT);
diff --git a/gdb/mi/mi-cmd-env.c b/gdb/mi/mi-cmd-env.c
index 327ddc5..0103153 100644
--- a/gdb/mi/mi-cmd-env.c
+++ b/gdb/mi/mi-cmd-env.c
@@ -78,7 +78,10 @@ mi_cmd_env_pwd (char *command, char **argv, int argc)
      
   /* Otherwise the mi level is 2 or higher.  */
 
-  getcwd (gdb_dirbuf, sizeof (gdb_dirbuf));
+  if (! getcwd (gdb_dirbuf, sizeof (gdb_dirbuf)))
+    error (_("mi_cmd_env_pwd: error finding name of working directory: %s"),
+           safe_strerror (errno));
+    
   ui_out_field_string (uiout, "cwd", gdb_dirbuf);
 }
 
diff --git a/gdb/ui-file.c b/gdb/ui-file.c
index 9a1d892..2ed304f 100644
--- a/gdb/ui-file.c
+++ b/gdb/ui-file.c
@@ -481,7 +481,8 @@ stdio_file_write (struct ui_file *file, const char *buf, long length_buf)
   if (stdio->magic != &stdio_file_magic)
     internal_error (__FILE__, __LINE__,
 		    _("stdio_file_write: bad magic number"));
-  fwrite (buf, length_buf, 1, stdio->file);
+  if (fwrite (buf, length_buf, 1, stdio->file) != 1)
+    error ("stdio_file_write: %s", safe_strerror (errno));
 }
 
 static void
@@ -491,7 +492,8 @@ stdio_file_fputs (const char *linebuffer, struct ui_file *file)
   if (stdio->magic != &stdio_file_magic)
     internal_error (__FILE__, __LINE__,
 		    _("stdio_file_fputs: bad magic number"));
-  fputs (linebuffer, stdio->file);
+  if (fputs (linebuffer, stdio->file) == EOF)
+    error ("stdio_file_fputs: %s", safe_strerror (errno));
 }
 
 static int
diff --git a/gdb/utils.c b/gdb/utils.c
index d14009f..725f00b 100644
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -865,7 +865,13 @@ internal_vproblem (struct internal_problem *problem,
 	abort ();	/* NOTE: GDB has only three calls to abort().  */
       default:
 	dejavu = 3;
-	write (STDERR_FILENO, msg, sizeof (msg));
+        /* Newer GLIBC versions put the warn_unused_result attribute
+           on write, but this is one of those rare cases where
+           ignoring the return value is correct.  Casting to (void)
+           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 ();
 	exit (1);
       }
   }
-- 
1.5.6.3

Re: RFA: Building GDB under GLIBC 2.8

[Mark Kettenis]

> Date: Fri, 12 Dec 2008 18:04:47 -0800
> From: "Jim Blandy" <jimb@red-bean.com>
> 
> Content-Type: text/x-patch; name=0001-gdb.patch
> Content-Transfer-Encoding: base64
> X-Attachment-Id: f_fonmk7gy0
> Content-Disposition: attachment; filename=0001-gdb.patch
> 
> RnJvbSA5NTQzOGRlZGExYTRkYzViZTY0ZDMxZWQ1ZGQzNDk1MDYwYjc2MTQ3
> IE1vbiBTZXAgMTcgMDA6MDA6MDAgMjAwMQpGcm9tOiBKaW0gQmxhbmR5IDxq

Jim, could you please try sending diffs such that they are readable in
a text-based mail reader?

Re: RFA: Building GDB under GLIBC 2.8

[Andreas Schwab]

Mark Kettenis <mark.kettenis@xs4all.nl> writes:

>> Date: Fri, 12 Dec 2008 18:04:47 -0800
>> From: "Jim Blandy" <jimb@red-bean.com>
>> 
>> Content-Type: text/x-patch; name=0001-gdb.patch
>> Content-Transfer-Encoding: base64
>> X-Attachment-Id: f_fonmk7gy0
>> Content-Disposition: attachment; filename=0001-gdb.patch
>> 
>> RnJvbSA5NTQzOGRlZGExYTRkYzViZTY0ZDMxZWQ1ZGQzNDk1MDYwYjc2MTQ3
>> IE1vbiBTZXAgMTcgMDA6MDA6MDAgMjAwMQpGcm9tOiBKaW0gQmxhbmR5IDxq
>
> Jim, could you please try sending diffs such that they are readable in
> a text-based mail reader?

Why should that be an obstacle for a text-based reader?

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: RFA: Building GDB under GLIBC 2.8

[Jim Blandy]

> Jim, could you please try sending diffs such that they are readable in
> a text-based mail reader?

I'm afraid gmail will mangle them if I don't include them as
attachments.  MIME attachments have been in widespread use for a very
long time; does your mail reader have difficulty displaying them?

Re: RFA: Building GDB under GLIBC 2.8 [resend w/the patch]

[Jan Kratochvil]

[-- Attachment #1: Type: text/plain, Size: 3887 bytes --]

Hi Jim,

[ attaching this time ]

FYI that part for "ui-file.c" crashes GDB on the testsuite as when the GDB is
existing it has already closed its output.  Testsuite results are not changed
(as GDB output is already ignored that time) but many core files start to be
generated.

Attaching a possible replacement for that ui-file.c part:  When GDB cannot
write to its output IMO some message about it is no longer of much use.


Regards,
Jan


Program terminated with signal 11, Segmentation fault.
[New process 9576]
#0  0x00000000006024ff in exceptions_state_mc (action=CATCH_THROWING) at exceptions.c:125
125       switch (current_catcher->state)
(gdb) p current_catcher
$1 = (struct catcher *) 0x0
(gdb) bt
#0  0x00000000006024ff in exceptions_state_mc (action=CATCH_THROWING) at exceptions.c:125
#1  0x0000000000602808 in throw_exception (exception=
      {reason = RETURN_ERROR, error = GENERIC_ERROR, message = 0x3776910 "stdio_file_fputs: Input/output error"}) at exceptions.c:235
#2  0x0000000000602c29 in throw_it (reason=RETURN_ERROR, error=GENERIC_ERROR, fmt=0xc8224b "stdio_file_fputs: %s", ap=0x7fff750c9f80)
    at exceptions.c:396
#3  0x0000000000602c51 in throw_verror (error=GENERIC_ERROR, fmt=0xc8224b "stdio_file_fputs: %s", ap=0x7fff750c9f80) at exceptions.c:402
#4  0x000000000047a466 in error (string=0xc8224b "stdio_file_fputs: %s") at utils.c:796
#5  0x000000000047f7a7 in stdio_file_fputs (linebuffer=0x376e900 '\225' <repeats 37 times>, "�\225\225�", file=0x2f31e20) at ui-file.c:496
#6  0x000000000047efbd in fputs_unfiltered (buf=0x376e900 '\225' <repeats 37 times>, "�\225\225�", file=0x2f31e20) at ui-file.c:211
#7  0x000000000047c36c in fputs_maybe_filtered (linebuffer=0x376e900 '\225' <repeats 37 times>, "�\225\225�", stream=0x2f31e20, filter=1)
    at utils.c:2028
#8  0x000000000047c5be in fputs_filtered (linebuffer=0x376e900 '\225' <repeats 37 times>, "�\225\225�", stream=0x2f31e20) at utils.c:2119
#9  0x0000000000602960 in print_exception (file=0x2f31e20, e=
      {reason = RETURN_ERROR, error = GENERIC_ERROR, message = 0x376e900 '\225' <repeats 37 times>, "�\225\225�"}) at exceptions.c:306
#10 0x0000000000602bc5 in print_any_exception (file=0x2f31e20, prefix=0xc7df05 "", e=
      {reason = RETURN_ERROR, error = GENERIC_ERROR, message = 0x376e900 '\225' <repeats 37 times>, "�\225\225�"}) at exceptions.c:373
#11 0x0000000000602fbd in catch_errors (func=0x46fc27 <captured_main>, func_args=0x7fff750ca200, errstring=0xc7df05 "", mask=6)
    at exceptions.c:518
#12 0x0000000000470cdd in gdb_main (args=0x7fff750ca200) at .././gdb/main.c:840
#13 0x000000000046fbe0 in main (argc=3, argv=0x7fff750ca2f8) at gdb.c:33


On Sat, 13 Dec 2008 03:04:47 +0100, Jim Blandy wrote:
> From: Jim Blandy <jimb@red-bean.com>
...
>  	* ui-file.c (stdio_file_write): Check return value from fwrite.
>  	(stdio_file_fputs): Check return value from fputs.
...
> diff --git a/gdb/ui-file.c b/gdb/ui-file.c
> index 9a1d892..2ed304f 100644
> --- a/gdb/ui-file.c
> +++ b/gdb/ui-file.c
> @@ -481,7 +481,8 @@ stdio_file_write (struct ui_file *file, const char *buf, long length_buf)
>    if (stdio->magic != &stdio_file_magic)
>      internal_error (__FILE__, __LINE__,
>  		    _("stdio_file_write: bad magic number"));
> -  fwrite (buf, length_buf, 1, stdio->file);
> +  if (fwrite (buf, length_buf, 1, stdio->file) != 1)
> +    error ("stdio_file_write: %s", safe_strerror (errno));
>  }
>  
>  static void
> @@ -491,7 +492,8 @@ stdio_file_fputs (const char *linebuffer, struct ui_file *file)
>    if (stdio->magic != &stdio_file_magic)
>      internal_error (__FILE__, __LINE__,
>  		    _("stdio_file_fputs: bad magic number"));
> -  fputs (linebuffer, stdio->file);
> +  if (fputs (linebuffer, stdio->file) == EOF)
> +    error ("stdio_file_fputs: %s", safe_strerror (errno));
>  }
>  
>  static int

[-- Attachment #2: 2j --]
[-- Type: text/plain, Size: 861 bytes --]

--- gdb/ui-file.c	1 Jan 2008 22:53:13 -0000	1.15
+++ gdb/ui-file.c	14 Dec 2008 18:16:56 -0000
@@ -481,7 +481,9 @@ stdio_file_write (struct ui_file *file, 
   if (stdio->magic != &stdio_file_magic)
     internal_error (__FILE__, __LINE__,
 		    _("stdio_file_write: bad magic number"));
-  fwrite (buf, length_buf, 1, stdio->file);
+  /* Calling error crashes when we are called from the exception framework.  */
+  if (fwrite (buf, length_buf, 1, stdio->file))
+    ;
 }
 
 static void
@@ -491,7 +493,9 @@ stdio_file_fputs (const char *linebuffer
   if (stdio->magic != &stdio_file_magic)
     internal_error (__FILE__, __LINE__,
 		    _("stdio_file_fputs: bad magic number"));
-  fputs (linebuffer, stdio->file);
+  /* Calling error crashes when we are called from the exception framework.  */
+  if (fputs (linebuffer, stdio->file))
+    ;
 }
 
 static int

Re: RFA: Building GDB under GLIBC 2.8

[Jan Kratochvil]

Hi Jim,

FYI that part for "ui-file.c" crashes GDB on the testsuite as when the GDB is
existing it has already closed its output.  Testsuite results are not changed
(as GDB output is already ignored that time) but many core files start to be
generated.

Attaching a possible replacement for that ui-file.c part:  When GDB cannot
write to its output IMO some message about it is no longer of much use.


Regards,
Jan


Program terminated with signal 11, Segmentation fault.
[New process 9576]
#0  0x00000000006024ff in exceptions_state_mc (action=CATCH_THROWING) at exceptions.c:125
125       switch (current_catcher->state)
(gdb) p current_catcher
$1 = (struct catcher *) 0x0
(gdb) bt
#0  0x00000000006024ff in exceptions_state_mc (action=CATCH_THROWING) at exceptions.c:125
#1  0x0000000000602808 in throw_exception (exception=
      {reason = RETURN_ERROR, error = GENERIC_ERROR, message = 0x3776910 "stdio_file_fputs: Input/output error"}) at exceptions.c:235
#2  0x0000000000602c29 in throw_it (reason=RETURN_ERROR, error=GENERIC_ERROR, fmt=0xc8224b "stdio_file_fputs: %s", ap=0x7fff750c9f80)
    at exceptions.c:396
#3  0x0000000000602c51 in throw_verror (error=GENERIC_ERROR, fmt=0xc8224b "stdio_file_fputs: %s", ap=0x7fff750c9f80) at exceptions.c:402
#4  0x000000000047a466 in error (string=0xc8224b "stdio_file_fputs: %s") at utils.c:796
#5  0x000000000047f7a7 in stdio_file_fputs (linebuffer=0x376e900 '\225' <repeats 37 times>, "�\225\225�", file=0x2f31e20) at ui-file.c:496
#6  0x000000000047efbd in fputs_unfiltered (buf=0x376e900 '\225' <repeats 37 times>, "�\225\225�", file=0x2f31e20) at ui-file.c:211
#7  0x000000000047c36c in fputs_maybe_filtered (linebuffer=0x376e900 '\225' <repeats 37 times>, "�\225\225�", stream=0x2f31e20, filter=1)
    at utils.c:2028
#8  0x000000000047c5be in fputs_filtered (linebuffer=0x376e900 '\225' <repeats 37 times>, "�\225\225�", stream=0x2f31e20) at utils.c:2119
#9  0x0000000000602960 in print_exception (file=0x2f31e20, e=
      {reason = RETURN_ERROR, error = GENERIC_ERROR, message = 0x376e900 '\225' <repeats 37 times>, "�\225\225�"}) at exceptions.c:306
#10 0x0000000000602bc5 in print_any_exception (file=0x2f31e20, prefix=0xc7df05 "", e=
      {reason = RETURN_ERROR, error = GENERIC_ERROR, message = 0x376e900 '\225' <repeats 37 times>, "�\225\225�"}) at exceptions.c:373
#11 0x0000000000602fbd in catch_errors (func=0x46fc27 <captured_main>, func_args=0x7fff750ca200, errstring=0xc7df05 "", mask=6)
    at exceptions.c:518
#12 0x0000000000470cdd in gdb_main (args=0x7fff750ca200) at .././gdb/main.c:840
#13 0x000000000046fbe0 in main (argc=3, argv=0x7fff750ca2f8) at gdb.c:33


On Sat, 13 Dec 2008 03:04:47 +0100, Jim Blandy wrote:
> From: Jim Blandy <jimb@red-bean.com>
...
>  	* ui-file.c (stdio_file_write): Check return value from fwrite.
>  	(stdio_file_fputs): Check return value from fputs.
...
> diff --git a/gdb/ui-file.c b/gdb/ui-file.c
> index 9a1d892..2ed304f 100644
> --- a/gdb/ui-file.c
> +++ b/gdb/ui-file.c
> @@ -481,7 +481,8 @@ stdio_file_write (struct ui_file *file, const char *buf, long length_buf)
>    if (stdio->magic != &stdio_file_magic)
>      internal_error (__FILE__, __LINE__,
>  		    _("stdio_file_write: bad magic number"));
> -  fwrite (buf, length_buf, 1, stdio->file);
> +  if (fwrite (buf, length_buf, 1, stdio->file) != 1)
> +    error ("stdio_file_write: %s", safe_strerror (errno));
>  }
>  
>  static void
> @@ -491,7 +492,8 @@ stdio_file_fputs (const char *linebuffer, struct ui_file *file)
>    if (stdio->magic != &stdio_file_magic)
>      internal_error (__FILE__, __LINE__,
>  		    _("stdio_file_fputs: bad magic number"));
> -  fputs (linebuffer, stdio->file);
> +  if (fputs (linebuffer, stdio->file) == EOF)
> +    error ("stdio_file_fputs: %s", safe_strerror (errno));
>  }
>  
>  static int

Re: RFA: Building GDB under GLIBC 2.8

[Mark Kettenis]

> Date: Sat, 13 Dec 2008 22:07:29 -0800
> From: "Jim Blandy" <jimb@red-bean.com>
> 
> > Jim, could you please try sending diffs such that they are readable in
> > a text-based mail reader?
> 
> I'm afraid gmail will mangle them if I don't include them as
> attachments.  MIME attachments have been in widespread use for a very
> long time; does your mail reader have difficulty displaying them?

Ugh, web-based email.  I keep wondering how people can write anything
but three-line mails with those.  I must be getting old...

Anyway, I use emacs's rmail to read my mail.  Have been for years, and
for me being able to have my mail inside my editor is fairly
essential.  Rmail isn't MIME-aware, which is great.  I don't get to
see most of the random crap that uneducated computer users attach to
their mails.  And if I really want to look at it, there's etach wich
detaches MIME attachments from my mail and saves them as a file.

Detaching attachments is fine for most things, but it is really
annoying for reviewing patches, since in many cases you want to make
remarks about particular lines of a patch, whcih is best done in line.
This is the reason why we used to have the (unwritten?) rule that
patches were supposed to be sent inline using a mail client that
didn't muck with the contents of the mail message.

Re: RFA: Building GDB under GLIBC 2.8

[Paul Pluzhnikov]

On Sun, Dec 14, 2008 at 11:56 AM, Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
>> Date: Sat, 13 Dec 2008 22:07:29 -0800
>> From: "Jim Blandy" <jimb@red-bean.com>
>>
>> > Jim, could you please try sending diffs such that they are readable in
>> > a text-based mail reader?
>>
>> I'm afraid gmail will mangle them if I don't include them as
>> attachments.

Gmail will actually do text/plain attachment with 7-bit encoding
if the attached filename ends in .txt

However, it appears that there is some other message rewriting
happening elsewhere, because my messages with text/plain/7-bit
attachments end up as text/plain/base64 by the time they reach
sourceware.org :-(

>> MIME attachments have been in widespread use for a very
>> long time; does your mail reader have difficulty displaying them?
>
> Ugh, web-based email.  I keep wondering how people can write anything
> but three-line mails with those.  I must be getting old...

There is an "It's all text" firefox plugin, which makes editing
the messages with a real editor easy. Unfortunately, whitespace
and line wrapping are still a problem.

> Anyway, I use emacs's rmail to read my mail.  Have been for years, and
> for me being able to have my mail inside my editor is fairly
> essential.  Rmail isn't MIME-aware, which is great.  I don't get to
> see most of the random crap that uneducated computer users attach to
> their mails.  And if I really want to look at it, there's etach wich
> detaches MIME attachments from my mail and saves them as a file.
>
> Detaching attachments is fine for most things, but it is really
> annoying for reviewing patches, since in many cases you want to make
> remarks about particular lines of a patch, whcih is best done in line.

Surely you can open the detached attachment in another buffer and
cut/paste the lines you want to comment on back into the reply?

> This is the reason why we used to have the (unwritten?) rule that
> patches were supposed to be sent inline using a mail client that
> didn't muck with the contents of the mail message.

Yes, I would very much like such a client. For a long time I've
used emacs VM mode, but there are several features of Gmail which
I do not know how to emulate in emacs-VM, so it isn't really
feasible for me to switch back to emacs :(

Base64 attachments appear not to cause too much trouble in this
list (you are the only one who complained recently), although
they are not tolerated in glibc lists.

Cheers,
-- 
Paul Pluzhnikov

Re: RFA: Building GDB under GLIBC 2.8

[Jim Blandy]

On Sun, Dec 14, 2008 at 10:25 AM, Jan Kratochvil
<jan.kratochvil@redhat.com> wrote:
> FYI that part for "ui-file.c" crashes GDB on the testsuite as when the GDB is
> existing it has already closed its output.  Testsuite results are not changed
> (as GDB output is already ignored that time) but many core files start to be
> generated.
>
> Attaching a possible replacement for that ui-file.c part:  When GDB cannot
> write to its output IMO some message about it is no longer of much use.

Ah, crud.  Thanks for catching that.  But, I didn't see an alternative
change in your message.

Re: RFA: Building GDB under GLIBC 2.8

[Tom Tromey]

>>>>> "Jim" == Jim Blandy <jimb@red-bean.com> writes:

Jim> The latest version of Ubuntu (Intrepid Ibex) includes a GLIBC that
Jim> attaches the warn_unused_result attribute to many C library functions.
Jim>  The archer project doesn't use the -Wno-unused flag, so I needed to
Jim> fix these to build archer.  It seemed to me that submitting the patch
Jim> upstream would be the most helpful thing to do.

Thanks.

One nit from me:
 
Jim> +static void
Jim> +check_syscall (const char *msg, int result)
Jim> +{

I believe that these days we're asking for intro comments on all new
functions.

I can't approve or reject this, but according to MAINTAINERS, you can :)

Tom

Re: RFA: Building GDB under GLIBC 2.8

[Pedro Alves]

On Saturday 13 December 2008 02:04:47, Jim Blandy wrote:
>         abort ();       /* NOTE: GDB has only three calls to abort().  */

                                                ^^^^^
<warning> incoming nit </warning>

Ooops, not three anymore, hey?  :-)

>        default:
>         dejavu = 3;
> -       write (STDERR_FILENO, msg, sizeof (msg));
> +        /* Newer GLIBC versions put the warn_unused_result attribute
> +           on write, but this is one of those rare cases where
> +           ignoring the return value is correct.  Casting to (void)
> +           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 ();
>         exit (1);
>        }

-- 
Pedro Alves

Re: RFA: Building GDB under GLIBC 2.8

[Jim Blandy]

[-- Attachment #1: Type: text/plain, Size: 64 bytes --]

Okay, I think this revision makes the changes people suggested.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: retval.patch --]
[-- Type: text/x-patch; name=retval.patch, Size: 8618 bytes --]

commit 01ff08caa6977858da014630578f708b86f84803
Author: Jim Blandy <jimb@red-bean.com>
Date:   Fri Dec 12 17:47:01 2008 -0800

    gdb
    	Check return values of functions declared with warn_unused_result
    	attribute in GLIBC 2.8.
    	* cli/cli-cmds.c (pwd_command): Check return value from getcwd.
    	* inflow.c (check_syscall): New function.
    	(new_tty): Use check_syscall to check return values from open and dup.
    	* linux-nat.c (linux_nat_info_proc_cmd): Check return value from fgets.
    	* main.c (captured_main): Call cwd after setting up gdb_stderr;
    	check for errors from getcwd.
    	* mi/mi-cmd-env.c (mi_cmd_env_pwd): Check return value from getcwd.
    	* ui-file.c (stdio_file_write): Ignore return value from fwrite.
    	(stdio_file_fputs): Same.
    	* utils.c (internal_vproblem): abort if last-ditch error message
    	write fails.

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index a688976..9680d8d 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,19 @@
 2008-12-19  Jim Blandy  <jimb@red-bean.com>
 
+	Check return values of functions declared with warn_unused_result
+	attribute in GLIBC 2.8.
+	* cli/cli-cmds.c (pwd_command): Check return value from getcwd.
+	* inflow.c (check_syscall): New function.
+	(new_tty): Use check_syscall to check return values from open and dup.
+	* linux-nat.c (linux_nat_info_proc_cmd): Check return value from fgets.
+	* main.c (captured_main): Call cwd after setting up gdb_stderr;
+	check for errors from getcwd.
+	* mi/mi-cmd-env.c (mi_cmd_env_pwd): Check return value from getcwd.
+	* ui-file.c (stdio_file_write): Ignore return value from fwrite.
+	(stdio_file_fputs): Same.
+	* utils.c (internal_vproblem): abort if last-ditch error message
+	write fails.
+
 	* top.c (gdb_init): Don't set the current directory here; that's
 	already been done in captured_main.
 
diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c
index 806a68a..b80bdfc 100644
--- a/gdb/cli/cli-cmds.c
+++ b/gdb/cli/cli-cmds.c
@@ -323,7 +323,9 @@ pwd_command (char *args, int from_tty)
 {
   if (args)
     error (_("The \"pwd\" command does not take an argument: %s"), args);
-  getcwd (gdb_dirbuf, sizeof (gdb_dirbuf));
+  if (! getcwd (gdb_dirbuf, sizeof (gdb_dirbuf)))
+    error (_("Error finding name of working directory: %s"),
+           safe_strerror (errno));
 
   if (strcmp (gdb_dirbuf, current_directory) != 0)
     printf_unfiltered (_("Working directory %s\n (canonically %s).\n"),
diff --git a/gdb/inflow.c b/gdb/inflow.c
index e82514e..a5a6d14 100644
--- a/gdb/inflow.c
+++ b/gdb/inflow.c
@@ -523,6 +523,20 @@ new_tty_prefork (const char *ttyname)
   inferior_thisrun_terminal = ttyname;
 }
 
+
+/* If RESULT, assumed to be the return value from a system call, is
+   negative, print the error message indicated by errno and exit.
+   MSG should identify the operation that failed.  */
+static void
+check_syscall (const char *msg, int result)
+{
+  if (result < 0)
+    {
+      print_sys_errmsg (msg, errno);
+      _exit (1);
+    }
+}
+
 void
 new_tty (void)
 {
@@ -549,27 +563,23 @@ new_tty (void)
 
   /* Now open the specified new terminal.  */
   tty = open (inferior_thisrun_terminal, O_RDWR | O_NOCTTY);
-  if (tty == -1)
-    {
-      print_sys_errmsg (inferior_thisrun_terminal, errno);
-      _exit (1);
-    }
+  check_syscall (inferior_thisrun_terminal, tty);
 
   /* Avoid use of dup2; doesn't exist on all systems.  */
   if (tty != 0)
     {
       close (0);
-      dup (tty);
+      check_syscall ("dup'ing tty into fd 0", dup (tty));
     }
   if (tty != 1)
     {
       close (1);
-      dup (tty);
+      check_syscall ("dup'ing tty into fd 1", dup (tty));
     }
   if (tty != 2)
     {
       close (2);
-      dup (tty);
+      check_syscall ("dup'ing tty into fd 2", dup (tty));
     }
 
 #ifdef TIOCSCTTY
diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index 913bfec..a829eb8 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -3666,8 +3666,10 @@ linux_nat_info_proc_cmd (char *args, int from_tty)
       if ((procfile = fopen (fname1, "r")) != NULL)
 	{
 	  struct cleanup *cleanup = make_cleanup_fclose (procfile);
-	  fgets (buffer, sizeof (buffer), procfile);
-	  printf_filtered ("cmdline = '%s'\n", buffer);
+          if (fgets (buffer, sizeof (buffer), procfile))
+            printf_filtered ("cmdline = '%s'\n", buffer);
+          else
+            warning (_("unable to read '/proc/%lld/cmdline'"), pid);
 	  do_cleanups (cleanup);
 	}
       else
diff --git a/gdb/main.c b/gdb/main.c
index a53002d..a9fd988 100644
--- a/gdb/main.c
+++ b/gdb/main.c
@@ -195,9 +195,6 @@ captured_main (void *data)
   line[0] = '\0';		/* Terminate saved (now empty) cmd line */
   instream = stdin;
 
-  getcwd (gdb_dirbuf, sizeof (gdb_dirbuf));
-  current_directory = gdb_dirbuf;
-
   gdb_stdout = stdio_fileopen (stdout);
   gdb_stderr = stdio_fileopen (stderr);
   gdb_stdlog = gdb_stderr;	/* for moment */
@@ -206,6 +203,15 @@ captured_main (void *data)
   gdb_stdtargerr = gdb_stderr;	/* for moment */
   gdb_stdtargin = gdb_stdin;	/* for moment */
 
+  if (! getcwd (gdb_dirbuf, sizeof (gdb_dirbuf)))
+    /* Don't use *_filtered or warning() (which relies on
+       current_target) until after initialize_all_files(). */
+    fprintf_unfiltered (gdb_stderr,
+                        _("%s: warning: error finding working directory: %s\n"),
+                        argv[0], safe_strerror (errno));
+    
+  current_directory = gdb_dirbuf;
+
   /* Set the sysroot path.  */
 #ifdef TARGET_SYSTEM_ROOT_RELOCATABLE
   gdb_sysroot = make_relative_prefix (argv[0], BINDIR, TARGET_SYSTEM_ROOT);
diff --git a/gdb/mi/mi-cmd-env.c b/gdb/mi/mi-cmd-env.c
index 327ddc5..0103153 100644
--- a/gdb/mi/mi-cmd-env.c
+++ b/gdb/mi/mi-cmd-env.c
@@ -78,7 +78,10 @@ mi_cmd_env_pwd (char *command, char **argv, int argc)
      
   /* Otherwise the mi level is 2 or higher.  */
 
-  getcwd (gdb_dirbuf, sizeof (gdb_dirbuf));
+  if (! getcwd (gdb_dirbuf, sizeof (gdb_dirbuf)))
+    error (_("mi_cmd_env_pwd: error finding name of working directory: %s"),
+           safe_strerror (errno));
+    
   ui_out_field_string (uiout, "cwd", gdb_dirbuf);
 }
 
diff --git a/gdb/ui-file.c b/gdb/ui-file.c
index 9a1d892..8bbe878 100644
--- a/gdb/ui-file.c
+++ b/gdb/ui-file.c
@@ -481,7 +481,9 @@ stdio_file_write (struct ui_file *file, const char *buf, long length_buf)
   if (stdio->magic != &stdio_file_magic)
     internal_error (__FILE__, __LINE__,
 		    _("stdio_file_write: bad magic number"));
-  fwrite (buf, length_buf, 1, stdio->file);
+  /* Calling error crashes when we are called from the exception framework.  */
+  if (fwrite (buf, length_buf, 1, stdio->file))
+    ;
 }
 
 static void
@@ -491,7 +493,9 @@ stdio_file_fputs (const char *linebuffer, struct ui_file *file)
   if (stdio->magic != &stdio_file_magic)
     internal_error (__FILE__, __LINE__,
 		    _("stdio_file_fputs: bad magic number"));
-  fputs (linebuffer, stdio->file);
+  /* Calling error crashes when we are called from the exception framework.  */
+  if (fputs (linebuffer, stdio->file))
+    ;
 }
 
 static int
diff --git a/gdb/utils.c b/gdb/utils.c
index d14009f..2cc721c 100644
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -862,10 +862,16 @@ internal_vproblem (struct internal_problem *problem,
       case 1:
 	dejavu = 2;
 	fputs_unfiltered (msg, gdb_stderr);
-	abort ();	/* NOTE: GDB has only three calls to abort().  */
+	abort ();	/* NOTE: GDB has only four calls to abort().  */
       default:
 	dejavu = 3;
-	write (STDERR_FILENO, msg, sizeof (msg));
+        /* Newer GLIBC versions put the warn_unused_result attribute
+           on write, but this is one of those rare cases where
+           ignoring the return value is correct.  Casting to (void)
+           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().  */
 	exit (1);
       }
   }
@@ -930,7 +936,7 @@ further debugging may prove unreliable.", file, line, problem->name, msg);
   if (quit_p)
     {
       if (dump_core_p)
-	abort ();		/* NOTE: GDB has only three calls to abort().  */
+	abort ();		/* NOTE: GDB has only four calls to abort().  */
       else
 	exit (1);
     }
@@ -940,7 +946,7 @@ further debugging may prove unreliable.", file, line, problem->name, msg);
 	{
 #ifdef HAVE_WORKING_FORK
 	  if (fork () == 0)
-	    abort ();		/* NOTE: GDB has only three calls to abort().  */
+	    abort ();		/* NOTE: GDB has only four calls to abort().  */
 #endif
 	}
     }

Re: RFA: Building GDB under GLIBC 2.8

[Joel Brobecker]

> Okay, I think this revision makes the changes people suggested.

One of the things that I was wondering about is the report about
numerous core files found after running the testsuite. Was this
addressed by this new version?

> commit 01ff08caa6977858da014630578f708b86f84803
> Author: Jim Blandy <jimb@red-bean.com>
> Date:   Fri Dec 12 17:47:01 2008 -0800
> 
>     gdb
>     	Check return values of functions declared with warn_unused_result
>     	attribute in GLIBC 2.8.
>     	* cli/cli-cmds.c (pwd_command): Check return value from getcwd.
>     	* inflow.c (check_syscall): New function.
>     	(new_tty): Use check_syscall to check return values from open and dup.
>     	* linux-nat.c (linux_nat_info_proc_cmd): Check return value from fgets.
>     	* main.c (captured_main): Call cwd after setting up gdb_stderr;
>     	check for errors from getcwd.
>     	* mi/mi-cmd-env.c (mi_cmd_env_pwd): Check return value from getcwd.
>     	* ui-file.c (stdio_file_write): Ignore return value from fwrite.
>     	(stdio_file_fputs): Same.
>     	* utils.c (internal_vproblem): abort if last-ditch error message
>     	write fails.

Looks OK to me. I'm a little bit pained by seeing us forced to check
return values in cases where we really don't care, but if this is
the way things are going to be in glibc from now on. Were the glibc
developers contacted and do we know what they said? I'm wondering
if these new attributes are a done-deal or whether there are
re-considering...

> --- a/gdb/linux-nat.c
> +++ b/gdb/linux-nat.c
> @@ -3666,8 +3666,10 @@ linux_nat_info_proc_cmd (char *args, int from_tty)
>        if ((procfile = fopen (fname1, "r")) != NULL)
>  	{
>  	  struct cleanup *cleanup = make_cleanup_fclose (procfile);
> -	  fgets (buffer, sizeof (buffer), procfile);
> -	  printf_filtered ("cmdline = '%s'\n", buffer);
> +          if (fgets (buffer, sizeof (buffer), procfile))
> +            printf_filtered ("cmdline = '%s'\n", buffer);
> +          else
> +            warning (_("unable to read '/proc/%lld/cmdline'"), pid);
                             
I suggest taking advantage of fname1 in the call to warning, rather than
duplicating the actual path.

-- 
Joel

Re: RFA: Building GDB under GLIBC 2.8

[Tom Tromey]

>>>>> "Joel" == Joel Brobecker <brobecker@adacore.com> writes:

Joel> Looks OK to me. I'm a little bit pained by seeing us forced to check
Joel> return values in cases where we really don't care, but if this is
Joel> the way things are going to be in glibc from now on. Were the glibc
Joel> developers contacted and do we know what they said? I'm wondering
Joel> if these new attributes are a done-deal or whether there are
Joel> re-considering...

You must ask for this checking.  It is only enabled if you use
-D_FORTIFY_SOURCE=2.  glibc is working as intended here, as this
checking is part of the point of this feature.

The problem is that some distros enable this by default.  And, other
distros build their packages this way as a security measure.  So, it
is handy if gdb is modified to conform.  The alternative is the
distros carrying their own patches for this, plus developers on Ubuntu
having to do something special.

Tom

Re: RFA: Building GDB under GLIBC 2.8

[Jim Blandy]

I've committed these two patches, with the suggestion made by Joel.

2009-01-06  Jim Blandy  <jimb@red-bean.com>

	Check return values of functions declared with warn_unused_result
	attribute in GLIBC 2.8.
	* cli/cli-cmds.c (pwd_command): Check return value from getcwd.
	* inflow.c (check_syscall): New function.
	(new_tty): Use check_syscall to check return values from open and dup.
	* linux-nat.c (linux_nat_info_proc_cmd): Check return value from fgets.
	* main.c (captured_main): Call cwd after setting up gdb_stderr;
	check for errors from getcwd.
	* mi/mi-cmd-env.c (mi_cmd_env_pwd): Check return value from getcwd.
	* ui-file.c (stdio_file_write): Ignore return value from fwrite.
	(stdio_file_fputs): Same.
	* utils.c (internal_vproblem): abort if last-ditch error message
	write fails.

	* top.c (gdb_init): Don't set the current directory here; that's
	already been done in captured_main.