Stop using errno values around target_xfer interfaces and memory errors.
authorPedro Alves <palves@redhat.com>
Wed, 9 Oct 2013 17:00:00 +0000 (17:00 +0000)
committerPedro Alves <palves@redhat.com>
Wed, 9 Oct 2013 17:00:00 +0000 (17:00 +0000)
target_read_memory & friends build on top of target_read (thus on top
of the target_xfer machinery), but turn all errors to EIO, an errno
value.  I think we'd better convert all these to return a
target_xfer_error too, like target_xfer_partial in a previous patch.
The patch starts by doing that.

(The patch does not add a enum target_xfer_error value for '0'/no
error, and likewise does not change the return type of several of
these functions to enum target_xfer_error, because different functions
return '0' with different semantics.)

I audited the tree for memory_error calls, EIO checks, places where
GDB hardcodes 'errno = EIO', and also for strerror calls.  What I
found is that nowadays there's really no need to handle random errno
values, other than the EIOs gdb itself hardcodes.  No doubt errno
values would appear in common code back in the day when
target_xfer_memory was the main interface to access memory, but
nowadays, any errno value that deprecated interface could return is
just absorved by default_xfer_partial:

      else if (xfered == 0 && errno == 0)
/* "deprecated_xfer_memory" uses 0, cross checked against
           ERRNO as one indication of an error.  */
return 0;
      else
return -1;

There are two places in the code that check for EIO and print "out of
bounds", and defer to strerror for other errors.  That's
c-lang.c:c_get_string, and valprint.c.:val_print_string.  AFAICT, the
strerror branch can never be reached nowadays, as the only error
possible to get at those points is EIO, given that it's GDB itself
that set that errno value (in target_read_memory, etc.).

breakpoint.c:insert_bp_location always prints the error val as if an
errno, returned by target_insert_breakpoint, with strerr.  Now the
error here is either always EIO for mem-break.c targets (again
hardcoded by the target_read_memory/target_write_memory functions), so
this always prints "Input/output error" or similar (depending on
host), or, for remote targets (and probably others), this gem:

  Error accessing memory address 0x80200400: Unknown error -1.

This patch makes these 3 places print the exact same error
memory_error prints.  This changes output, but I think this is better,
for making memory error output consistent with other commands, and, it
means we have a central place to tweak for memory errors.

E.g., this changes:

 Cannot insert breakpoint 1.
 Error accessing memory address 0x5fc660: Input/output error.

to:

 Cannot insert breakpoint 1.
 Cannot access memory at address 0x5fc660

Which I find pretty much acceptable.

Surprisingly, only py-prettyprint.exp had a regression, for needing an
adjustment.  I also grepped the testsuite for the old errors, and
found no other hits.

Now that errno values aren't used anywhere in any of these memory
access related routines, I made memory_error itself take a
target_xfer_error instead of an errno.  The new
target_xfer_memory_error function added recently is no longer
necessary, and is thus removed.

Tested on x86_64 Fedora 17, native and gdbserver.

gdb/
2013-10-09  Pedro Alves  <palves@redhat.com>

* breakpoint.c (insert_bp_location): Use memory_error_message to
build the memory error string.
* c-lang.c: Include "gdbcore.h".
(c_get_string): Use memory_error to throw error.
(target_xfer_memory_error): Delete.
(memory_error_message): New, factored out from
target_xfer_memory_error.
(memory_error): Change parameter type to target_xfer_error.
Rewrite.
(read_memory): Use memory_error instead of
target_xfer_memory_error.
* gdbcore.h: Include "target.h".
(memory_error): Change parameter type to target_xfer_error.
(memory_error_message): Declare function.
* target.c (target_read_memory, target_read_stack)
(target_write_memory, target_write_raw_memory): Return
TARGET_XFER_E_IO on error.  Adjust comments.
(get_target_memory): Pass TARGET_XFER_E_IO to memory_error,
instead of EIO.
* target.h (target_read, target_insert_breakpoint)
(target_remove_breakpoint): Adjust comments.
* valprint.c (partial_memory_read): Rename parameter, and adjust
comment.
(val_print_string): Use memory_error_message to build the memory
error string.

gdb/testsuite/
2013-10-09  Pedro Alves  <palves@redhat.com>

* gdb.python/py-prettyprint.exp (run_lang_tests): Adjust expected
output.

gdb/ChangeLog
gdb/breakpoint.c
gdb/c-lang.c
gdb/corefile.c
gdb/gdbcore.h
gdb/target.c
gdb/target.h
gdb/testsuite/ChangeLog
gdb/testsuite/gdb.python/py-prettyprint.exp
gdb/valprint.c

index 3dd66c8c7f0704bf65736324df97e19092c5e35a..21bc3fe4ba08725fa29adbc9f2ca3f5f8dfa9512 100644 (file)
@@ -1,3 +1,31 @@
+2013-10-09  Pedro Alves  <palves@redhat.com>
+
+       * breakpoint.c (insert_bp_location): Use memory_error_message to
+       build the memory error string.
+       * c-lang.c: Include "gdbcore.h".
+       (c_get_string): Use memory_error to throw error.
+       (target_xfer_memory_error): Delete.
+       (memory_error_message): New, factored out from
+       target_xfer_memory_error.
+       (memory_error): Change parameter type to target_xfer_error.
+       Rewrite.
+       (read_memory): Use memory_error instead of
+       target_xfer_memory_error.
+       * gdbcore.h: Include "target.h".
+       (memory_error): Change parameter type to target_xfer_error.
+       (memory_error_message): Declare function.
+       * target.c (target_read_memory, target_read_stack)
+       (target_write_memory, target_write_raw_memory): Return
+       TARGET_XFER_E_IO on error.  Adjust comments.
+       (get_target_memory): Pass TARGET_XFER_E_IO to memory_error,
+       instead of EIO.
+       * target.h (target_read, target_insert_breakpoint)
+       (target_remove_breakpoint): Adjust comments.
+       * valprint.c (partial_memory_read): Rename parameter, and adjust
+       comment.
+       (val_print_string): Use memory_error_message to build the memory
+       error string.
+
 2013-10-09  Jan Kratochvil  <jan.kratochvil@redhat.com>
 
        * common/filestuff.c (gdb_fopen_cloexec): Remove initialization of
index d3e9e496f09cee69dd68b1f6b70052a27b81efdf..f0da90a5ce76364e7355180d69db09c50073d201 100644 (file)
@@ -2570,15 +2570,16 @@ insert_bp_location (struct bp_location *bl,
                }
              else
                {
+                 char *message = memory_error_message (TARGET_XFER_E_IO,
+                                                       bl->gdbarch, bl->address);
+                 struct cleanup *old_chain = make_cleanup (xfree, message);
+
                  fprintf_unfiltered (tmp_error_stream, 
-                                     "Cannot insert breakpoint %d.\n", 
-                                     bl->owner->number);
-                 fprintf_filtered (tmp_error_stream, 
-                                   "Error accessing memory address ");
-                 fputs_filtered (paddress (bl->gdbarch, bl->address),
-                                 tmp_error_stream);
-                 fprintf_filtered (tmp_error_stream, ": %s.\n",
-                                   safe_strerror (val));
+                                     "Cannot insert breakpoint %d.\n"
+                                     "%s\n",
+                                     bl->owner->number, message);
+
+                 do_cleanups (old_chain);
                }
 
            }
index 48a1fb02be990a7d171ec1adcec5d88d4f2aaf14..33bb4443f088215244d743014832bf3ddf202c5f 100644 (file)
@@ -35,6 +35,7 @@
 #include "gdb_obstack.h"
 #include <ctype.h>
 #include "exceptions.h"
+#include "gdbcore.h"
 
 extern void _initialize_c_language (void);
 
@@ -312,12 +313,7 @@ c_get_string (struct value *value, gdb_byte **buffer,
       if (err)
        {
          xfree (*buffer);
-         if (err == EIO)
-           throw_error (MEMORY_ERROR, "Address %s out of bounds",
-                        paddress (get_type_arch (type), addr));
-         else
-           error (_("Error reading string from inferior: %s"),
-                  safe_strerror (err));
+         memory_error (err, addr);
        }
     }
 
index 1b733e2db056f1c771043b3e46355db8d7d57128..345df90799efe426477a8ea2dddae2126f95538b 100644 (file)
@@ -193,24 +193,20 @@ Use the \"file\" or \"exec-file\" command."));
 }
 \f
 
-/* Report a target xfer memory error by throwing a suitable
-   exception.  */
-
-static void
-target_xfer_memory_error (enum target_xfer_error err, CORE_ADDR memaddr)
+char *
+memory_error_message (enum target_xfer_error err,
+                     struct gdbarch *gdbarch, CORE_ADDR memaddr)
 {
   switch (err)
     {
     case TARGET_XFER_E_IO:
       /* Actually, address between memaddr and memaddr + len was out of
         bounds.  */
-      throw_error (MEMORY_ERROR,
-                  _("Cannot access memory at address %s"),
-                  paddress (target_gdbarch (), memaddr));
+      return xstrprintf (_("Cannot access memory at address %s"),
+                        paddress (gdbarch, memaddr));
     case TARGET_XFER_E_UNAVAILABLE:
-      throw_error (NOT_AVAILABLE_ERROR,
-                  _("Memory at address %s unavailable."),
-                  paddress (target_gdbarch (), memaddr));
+      return xstrprintf (_("Memory at address %s unavailable."),
+                        paddress (gdbarch, memaddr));
     default:
       internal_error (__FILE__, __LINE__,
                      "unhandled target_xfer_error: %s (%s)",
@@ -219,18 +215,30 @@ target_xfer_memory_error (enum target_xfer_error err, CORE_ADDR memaddr)
     }
 }
 
-/* Report a memory error by throwing a MEMORY_ERROR error.  */
+/* Report a memory error by throwing a suitable exception.  */
 
 void
-memory_error (int status, CORE_ADDR memaddr)
+memory_error (enum target_xfer_error err, CORE_ADDR memaddr)
 {
-  if (status == EIO)
-    target_xfer_memory_error (TARGET_XFER_E_IO, memaddr);
-  else
-    throw_error (MEMORY_ERROR,
-                _("Error accessing memory address %s: %s."),
-                paddress (target_gdbarch (), memaddr),
-                safe_strerror (status));
+  char *str;
+
+  /* Build error string.  */
+  str = memory_error_message (err, target_gdbarch (), memaddr);
+  make_cleanup (xfree, str);
+
+  /* Choose the right error to throw.  */
+  switch (err)
+    {
+    case TARGET_XFER_E_IO:
+      err = MEMORY_ERROR;
+      break;
+    case TARGET_XFER_E_UNAVAILABLE:
+      err = NOT_AVAILABLE_ERROR;
+      break;
+    }
+
+  /* Throw it.  */
+  throw_error (err, ("%s"), str);
 }
 
 /* Same as target_read_memory, but report an error if can't read.  */
@@ -248,9 +256,9 @@ read_memory (CORE_ADDR memaddr, gdb_byte *myaddr, ssize_t len)
                                          memaddr + xfered, len - xfered);
 
       if (xfer == 0)
-       target_xfer_memory_error (TARGET_XFER_E_IO, memaddr + xfered);
+       memory_error (TARGET_XFER_E_IO, memaddr + xfered);
       if (xfer < 0)
-       target_xfer_memory_error (xfer, memaddr + xfered);
+       memory_error (xfer, memaddr + xfered);
       xfered += xfer;
       QUIT;
     }
index 01ed4df5c4e0414b19f364cd97f43b16d3460ae2..41bcf1f44e22adba0d8a91ec0826da4b6e49b476 100644 (file)
@@ -27,6 +27,7 @@ struct regcache;
 
 #include "bfd.h"
 #include "exec.h"
+#include "target.h"
 
 /* Return the name of the executable file as a string.
    ERR nonzero means get error if there is none specified;
@@ -40,7 +41,13 @@ extern int have_core_file_p (void);
 
 /* Report a memory error with error().  */
 
-extern void memory_error (int status, CORE_ADDR memaddr);
+extern void memory_error (enum target_xfer_error status, CORE_ADDR memaddr);
+
+/* The string 'memory_error' would use as exception message.  Space
+   for the result is malloc'd, caller must free.  */
+
+extern char *memory_error_message (enum target_xfer_error err,
+                                  struct gdbarch *gdbarch, CORE_ADDR memaddr);
 
 /* Like target_read_memory, but report an error if can't read.  */
 
index 559ae0c82bc10515755c21c300b96ec2c965c038..72793597571749110ed7fefe189fe216fa53d654 100644 (file)
@@ -1787,9 +1787,9 @@ target_xfer_partial (struct target_ops *ops,
   return retval;
 }
 
-/* Read LEN bytes of target memory at address MEMADDR, placing the results in
-   GDB's memory at MYADDR.  Returns either 0 for success or an errno value
-   if any error occurs.
+/* Read LEN bytes of target memory at address MEMADDR, placing the
+   results in GDB's memory at MYADDR.  Returns either 0 for success or
+   a target_xfer_error value if any error occurs.
 
    If an error occurs, no guarantee is made about the contents of the data at
    MYADDR.  In particular, the caller should not depend upon partial reads
@@ -1808,7 +1808,7 @@ target_read_memory (CORE_ADDR memaddr, gdb_byte *myaddr, ssize_t len)
                   myaddr, memaddr, len) == len)
     return 0;
   else
-    return EIO;
+    return TARGET_XFER_E_IO;
 }
 
 /* Like target_read_memory, but specify explicitly that this is a read from
@@ -1825,13 +1825,14 @@ target_read_stack (CORE_ADDR memaddr, gdb_byte *myaddr, ssize_t len)
                   myaddr, memaddr, len) == len)
     return 0;
   else
-    return EIO;
+    return TARGET_XFER_E_IO;
 }
 
 /* Write LEN bytes from MYADDR to target memory at address MEMADDR.
-   Returns either 0 for success or an errno value if any error occurs.
-   If an error occurs, no guarantee is made about how much data got written.
-   Callers that can deal with partial writes should call target_write.  */
+   Returns either 0 for success or a target_xfer_error value if any
+   error occurs.  If an error occurs, no guarantee is made about how
+   much data got written.  Callers that can deal with partial writes
+   should call target_write.  */
 
 int
 target_write_memory (CORE_ADDR memaddr, const gdb_byte *myaddr, ssize_t len)
@@ -1843,14 +1844,14 @@ target_write_memory (CORE_ADDR memaddr, const gdb_byte *myaddr, ssize_t len)
                    myaddr, memaddr, len) == len)
     return 0;
   else
-    return EIO;
+    return TARGET_XFER_E_IO;
 }
 
 /* Write LEN bytes from MYADDR to target raw memory at address
-   MEMADDR.  Returns either 0 for success or an errno value if any
-   error occurs.  If an error occurs, no guarantee is made about how
-   much data got written.  Callers that can deal with partial writes
-   should call target_write.  */
+   MEMADDR.  Returns either 0 for success or a target_xfer_error value
+   if any error occurs.  If an error occurs, no guarantee is made
+   about how much data got written.  Callers that can deal with
+   partial writes should call target_write.  */
 
 int
 target_write_raw_memory (CORE_ADDR memaddr, const gdb_byte *myaddr, ssize_t len)
@@ -1862,7 +1863,7 @@ target_write_raw_memory (CORE_ADDR memaddr, const gdb_byte *myaddr, ssize_t len)
                    myaddr, memaddr, len) == len)
     return 0;
   else
-    return EIO;
+    return TARGET_XFER_E_IO;
 }
 
 /* Fetch the target's memory map.  */
@@ -2440,7 +2441,7 @@ get_target_memory (struct target_ops *ops, CORE_ADDR addr, gdb_byte *buf,
      for this target).  */
   if (target_read (ops, TARGET_OBJECT_RAW_MEMORY, NULL, buf, addr, len)
       != len)
-    memory_error (EIO, addr);
+    memory_error (TARGET_XFER_E_IO, addr);
 }
 
 ULONGEST
index 7622465554638dc945799b2e4eebc12474773e50..56ca40c8b1ae4ffa78afc69a08ff6319fdeb7108 100644 (file)
@@ -239,11 +239,12 @@ DEF_VEC_P(static_tracepoint_marker_p);
    starting point.  The ANNEX can be used to provide additional
    data-specific information to the target.
 
-   Return the number of bytes actually transfered, or -1 if the
-   transfer is not supported or otherwise fails.  Return of a positive
-   value less than LEN indicates that no further transfer is possible.
-   Unlike the raw to_xfer_partial interface, callers of these
-   functions do not need to retry partial transfers.  */
+   Return the number of bytes actually transfered, or a negative error
+   code (an 'enum target_xfer_error' value) if the transfer is not
+   supported or otherwise fails.  Return of a positive value less than
+   LEN indicates that no further transfer is possible.  Unlike the raw
+   to_xfer_partial interface, callers of these functions do not need
+   to retry partial transfers.  */
 
 extern LONGEST target_read (struct target_ops *ops,
                            enum target_object object,
@@ -1117,13 +1118,13 @@ int target_write_memory_blocks (VEC(memory_write_request_s) *requests,
      (*current_target.to_files_info) (&current_target)
 
 /* Insert a breakpoint at address BP_TGT->placed_address in the target
-   machine.  Result is 0 for success, or an errno value.  */
+   machine.  Result is 0 for success, non-zero for error.  */
 
 extern int target_insert_breakpoint (struct gdbarch *gdbarch,
                                     struct bp_target_info *bp_tgt);
 
 /* Remove a breakpoint at address BP_TGT->placed_address in the target
-   machine.  Result is 0 for success, or an errno value.  */
+   machine.  Result is 0 for success, non-zero for error.  */
 
 extern int target_remove_breakpoint (struct gdbarch *gdbarch,
                                     struct bp_target_info *bp_tgt);
index 7a5f913f0d76ee4aab14a86f266f831bd175fb8f..7fc1148eee7c417a4258bc32d7721bf14a6ac67c 100644 (file)
@@ -1,3 +1,8 @@
+2013-10-09  Pedro Alves  <palves@redhat.com>
+
+       * gdb.python/py-prettyprint.exp (run_lang_tests): Adjust expected
+       output.
+
 2013-10-09  Pedro Alves  <palves@redhat.com>
 
        * gdb.base/catch-syscall.exp (test_catch_syscall_without_args)
index d124a7261e2096af0d89864f041ffe6d15948c91..adbe88d0b8a7feba3e821b81183c728fbcab3c2d 100644 (file)
@@ -83,7 +83,7 @@ proc run_lang_tests {exefile lang} {
        gdb_py_test_silent_cmd "set print elements 200" "" 1
     }
 
-    gdb_test "print ns2" ".error reading variable: Address 0x0 out of bounds."
+    gdb_test "print ns2" ".error reading variable: Cannot access memory at address 0x0."
 
     gdb_test "print x" " = \"this is x\""
     gdb_test "print cstring" " = \"const string\""
index 61492ccba9788267820a16e3963fd1156b9d4f92..0ecea0c55fbf728486a160d3fb4be97ca206812b 100644 (file)
@@ -83,7 +83,7 @@ struct cmd_list_element *showprintrawlist;
 /* Prototypes for local functions */
 
 static int partial_memory_read (CORE_ADDR memaddr, gdb_byte *myaddr,
-                               int len, int *errnoptr);
+                               int len, int *errptr);
 
 static void show_print (char *, int);
 
@@ -1711,15 +1711,15 @@ val_print_array_elements (struct type *type,
 
 /* Read LEN bytes of target memory at address MEMADDR, placing the
    results in GDB's memory at MYADDR.  Returns a count of the bytes
-   actually read, and optionally an errno value in the location
-   pointed to by ERRNOPTR if ERRNOPTR is non-null.  */
+   actually read, and optionally a target_xfer_error value in the
+   location pointed to by ERRPTR if ERRPTR is non-null.  */
 
 /* FIXME: cagney/1999-10-14: Only used by val_print_string.  Can this
    function be eliminated.  */
 
 static int
 partial_memory_read (CORE_ADDR memaddr, gdb_byte *myaddr,
-                    int len, int *errnoptr)
+                    int len, int *errptr)
 {
   int nread;                   /* Number of bytes actually read.  */
   int errcode;                 /* Error from last read.  */
@@ -1744,9 +1744,9 @@ partial_memory_read (CORE_ADDR memaddr, gdb_byte *myaddr,
          nread--;
        }
     }
-  if (errnoptr != NULL)
+  if (errptr != NULL)
     {
-      *errnoptr = errcode;
+      *errptr = errcode;
     }
   return (nread);
 }
@@ -1755,7 +1755,7 @@ partial_memory_read (CORE_ADDR memaddr, gdb_byte *myaddr,
    each.  Fetch at most FETCHLIMIT characters.  BUFFER will be set to a newly
    allocated buffer containing the string, which the caller is responsible to
    free, and BYTES_READ will be set to the number of bytes read.  Returns 0 on
-   success, or errno on failure.
+   success, or a target_xfer_error on failure.
 
    If LEN > 0, reads exactly LEN characters (including eventual NULs in
    the middle or end of the string).  If LEN is -1, stops at the first
@@ -2524,18 +2524,14 @@ val_print_string (struct type *elttype, const char *encoding,
 
   if (errcode != 0)
     {
-      if (errcode == EIO)
-       {
-         fprintf_filtered (stream, "<Address ");
-         fputs_filtered (paddress (gdbarch, addr), stream);
-         fprintf_filtered (stream, " out of bounds>");
-       }
-      else
-       {
-         fprintf_filtered (stream, "<Error reading address ");
-         fputs_filtered (paddress (gdbarch, addr), stream);
-         fprintf_filtered (stream, ": %s>", safe_strerror (errcode));
-       }
+      char *str;
+
+      str = memory_error_message (errcode, gdbarch, addr);
+      make_cleanup (xfree, str);
+
+      fprintf_filtered (stream, "<error: ");
+      fputs_filtered (str, stream);
+      fprintf_filtered (stream, ">");
     }
 
   gdb_flush (stream);