Remote Debugging Protocol - hex case

Remote Debugging Protocol - hex case

[Tim Auton]

I just had a problem when using a remote debugging stub with GDB 6.6.
The stub is part of Avrora, a simulator for the Atmel AVR
microcontroller. The problem is that Avrora was sending replies using
upper-case hex, so when a hex reply started with "E" it was *sometimes*
interpreted as an error. Obviously, fixing the stub to use lower case
for hex is on the agenda. But in looking through the GDB documentation
and remote.c source file I found a number of inconsistencies which are
probably worth addressing.

First, I could find nowhere in the documentation that specified stubs
must use lower-case for hex. If they should (and of course they
should, because upper-case doesn't work properly), it should be in the
docs.

Even looking through the relevant source (remote.c), at first glance it
looks like it *does* support upper-case hex. The fromhex() function
supports upper case and there are a couple of functions -
packet_check_result() and packet_ok() - which implement checking which
can distinguish upper-case hex replies from true error messages. But
those functions simply aren't used, other than in remote_send_printf().
The functions remote_read_bytes() and remote_rcmd() each implement
something like packet_check_result() and will correctly handle
upper-case hex as a result. All the other functions treat any reply
which starts with an 'E' as an error, but will accept all other upper
case hex replies. The upshot of all this is that it's pretty easy to
write a stub which uses upper-case hex and for it to appear to work just
fine.

As far as I can see, making remote debugging support upper-case hex
properly won't be much trouble. It pretty much amounts to replacing the
disparate error checking in remote.c with calls to
packet_check_result(). But it's quite possible that I'm missing wider
repercussions which are obvious to someone who knows GDB and the source
much better than me (which is probably most of you). In particular, if
there are any stubs in the wild which reply with something other than
the documented error codes Exx (where x is a hex digit) or E.something
and expect it to be treated as an error, supporting upper-case hex might
break them. On the other hand, looking at the mailing list archives, not
supporting upper case hex consistently has caused some breakage too.

In summary, I see three options:

1) Patch remote.c to support upper-case hex consistently, update docs to
suggest lower-case for backward compatibility.

	Pros: Consistent with the rest of GDB, which generally supports
	upper-case hex.

	Cons: Any stubs which don't provide a two-digit errno or a E.something
	string will break. (Do any exist?)

2) Update docs to require lower-case, remove partial support for
upper-case hex from remote.c to avoid confusion.

	Pros: Should only break stubs which are intermittently broken so
	need fixing anyway; avoids misleading testing results when
	developing new stubs.

	Cons: Inconsistent with rest of GDB, which generally supports
	upper-case hex. Will cause some breakage, if only of stubs which are
	intermittently broken anyway.

3) Do nothing.

	Pros: Zero effort.

	Cons: People writing new stubs will continue to get bitten by the
	inconsistency.

I'm happy to write and submit the patches once the maintainers decide on
the best course of action, though as I'm not intimate with GDB's
internals and don't (yet) have the first clue about texinfo (but am
wiling to learn), the maintainers may wish to be more circumspect than
ususal in checking them. I've found the GNU coding standards, but
pointers to anything else relevant would be well received (off-list, if
appropriate).

Thoughts?


Tim

Re: Remote Debugging Protocol - hex case

[Daniel Jacobowitz]

On Tue, Feb 13, 2007 at 09:45:41PM +0000, Tim Auton wrote:
> As far as I can see, making remote debugging support upper-case hex
> properly won't be much trouble. It pretty much amounts to replacing the
> disparate error checking in remote.c with calls to
> packet_check_result(). But it's quite possible that I'm missing wider
> repercussions which are obvious to someone who knows GDB and the source
> much better than me (which is probably most of you).

As far as I know, you are correct - it's just a matter of cleanup.
The "more correct" functions you mention are much newer, and it's hard
to test some of the older bits of the file - that's why not much has
happened.

> 1) Patch remote.c to support upper-case hex consistently, update docs to
> suggest lower-case for backward compatibility.
> 
> 	Pros: Consistent with the rest of GDB, which generally supports
> 	upper-case hex.
> 
> 	Cons: Any stubs which don't provide a two-digit errno or a 
> 	E.something
> 	string will break. (Do any exist?)

I think they are sufficiently broken that we should not bend over
backwards to support them (though recognizing "ENN" as an error might
be useful - a recent PR was at least the second time I've seen someone
do that.)

> I'm happy to write and submit the patches once the maintainers decide on
> the best course of action, though as I'm not intimate with GDB's
> internals and don't (yet) have the first clue about texinfo (but am
> wiling to learn), the maintainers may wish to be more circumspect than
> ususal in checking them. I've found the GNU coding standards, but
> pointers to anything else relevant would be well received (off-list, if
> appropriate).

That, and the list archives, should be fine as references.  I'd be
glad to review patches in this area.  Please note that this would be
non-trivial work, so it would require an FSF copyright assignment for
GDB - let me know if you're able and willing to do that (or have one
already).

-- 
Daniel Jacobowitz
CodeSourcery

Re: Remote Debugging Protocol - hex case

[Tim Auton]

On 13 Feb 2007, at 22:49, Daniel Jacobowitz wrote:
> On Tue, Feb 13, 2007 at 09:45:41PM +0000, Tim Auton wrote:
>> As far as I can see, making remote debugging support upper-case hex
>> properly won't be much trouble. It pretty much amounts to  
>> replacing the
>> disparate error checking in remote.c with calls to
>> packet_check_result(). But it's quite possible that I'm missing wider
>> repercussions which are obvious to someone who knows GDB and the  
>> source
>> much better than me (which is probably most of you).
>
> As far as I know, you are correct - it's just a matter of cleanup.
> The "more correct" functions you mention are much newer, and it's hard
> to test some of the older bits of the file - that's why not much has
> happened.

OK, I'll have a good look at the testing framework and see what's  
there at
present.

>> 1) Patch remote.c to support upper-case hex consistently, update  
>> docs to
>> suggest lower-case for backward compatibility.
>>
>> 	Pros: Consistent with the rest of GDB, which generally supports
>> 	upper-case hex.
>>
>> 	Cons: Any stubs which don't provide a two-digit errno or a
>> 	E.something
>> 	string will break. (Do any exist?)
>
> I think they are sufficiently broken that we should not bend over
> backwards to support them (though recognizing "ENN" as an error might
> be useful - a recent PR was at least the second time I've seen someone
> do that.)

Can I take it that means option 1 looks most desirable?

>> I'm happy to write and submit the patches once the maintainers  
>> decide on
>> the best course of action, though as I'm not intimate with GDB's
>> internals and don't (yet) have the first clue about texinfo (but am
>> wiling to learn), the maintainers may wish to be more circumspect  
>> than
>> ususal in checking them. I've found the GNU coding standards, but
>> pointers to anything else relevant would be well received (off- 
>> list, if
>> appropriate).
>
> That, and the list archives, should be fine as references.  I'd be
> glad to review patches in this area.  Please note that this would be
> non-trivial work, so it would require an FSF copyright assignment for
> GDB - let me know if you're able and willing to do that (or have one
> already).

I don't have one, but FSF copyright assignment is no problem to me and
there's no legal impediment. I presume you'll let me know exactly what
I need to do to sort the papers out.


Tim

Re: Remote Debugging Protocol - hex case

[Daniel Jacobowitz]

On Wed, Feb 14, 2007 at 04:25:51PM +0000, Tim Auton wrote:
> OK, I'll have a good look at the testing framework and see what's  
> there at
> present.

It won't tell you much.  The way I usually test remote protocol work
is by running the whole testsuite using gdbserver - but gdbserver
doesn't implement e.g. the old thread protocol.

> Can I take it that means option 1 looks most desirable?

To me anyway.

> I don't have one, but FSF copyright assignment is no problem to me and
> there's no legal impediment. I presume you'll let me know exactly what
> I need to do to sort the papers out.

Great!  I'll send you the form off-list - and thank you very much for
expressing interest in this.

-- 
Daniel Jacobowitz
CodeSourcery