This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Stop using errno values around target_xfer interfaces and memory errors.


On 08/23/2013 05:36 PM, Pedro Alves wrote:
> 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-08-23  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-08-23  Pedro Alves  <palves@redhat.com>
> 
> 	* gdb.python/py-prettyprint.exp (run_lang_tests): Adjust expected
> 	output.

I'm checking this in.

-- 
Pedro Alves


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]