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] fix misparsing in remote_parse_stop_reply


On 08/17/2015 05:09 AM, Sandra Loosemore wrote:
> While trying to do some testing on one of our ARM boards via a debug 
> probe and stub that speaks RSP, I was mystified by getting this error:
> 
> (gdb) target remote ...
> (gdb) break main
> (gdb) load
> (gdb) c
> Cannot detach breakpoints of inferior_ptid
> 
> Eventually I realized that this wasn't just a badly-worded message; GDB 
> was somehow mis-interpreting the stop reply 'T05f:f0021000;' as 
> TARGET_WAITKIND_FORKED, and wandering off into code that it shouldn't 
> have been executing in the first place.  (On ARM, register 0xf is the 
> PC, BTW, and this particular stub works fine with a GDB from last year.)
> 
> Tracking it down further to remote_parse_stop_reply, the trouble is that 
> the test
> 
> else if (strncmp (p, "fork", p1 - p) == 0)
> 
> is matching here; p1 points to the colon so it is matching exactly one 
> character, the initial 'f'.  While "fork" is new-ish, all the stop 
> reason keywords use similar matching logic.
> 
> I don't see anything in the RSP documentation to indicate that the 
> special stop reason keywords in a 'T' reply can be abbreviated, so I 
> suspect this is just a think-o in the code, and it should be testing for 
> an exact match of the keyword instead.  

Indeed.

> Is there something else going on here that I'm missing?

Nope.

> 
> The attached patch is sufficient to unblock testing.  Does it seem 
> plausible?  This particular debug probe is quite slow so I haven't had 
> time to run the full GDB testsuite yet, or test on any other target.
> 
> It would also be good if somebody could translate that puzzling error 
> message into something more user-friendly and less 
> implementor-speaky....  but I still don't know what it means, so I can't 
> suggest anything.  :-(

This is really an internal error.  Users shouldn't be seeing it.
After a fork, because the child is a copy of the parent, it has
all the parent's breakpoints inserted in memory as well.
detach_breakpoints's job is to replace them with the original
instructions in the child, while leaving the parent's breakpoints marked
inserted in gdb's structures.  The parent is inferior_ptid, the child is
the passed in ptid.  So if ptid is inferior_ptid, we'd be removing
breakpoints from the parent, which is not what is wanted here.

> 2015-08-16  Sandra Loosemore  <sandra@codesourcery.com>
> 
> 	gdb/
> 	* remote.c (strprefix): New.
> 	(remote_parse_stop_reply): Use strprefix instead of strncmp
> 	to ensure exact match of keyword.
> 

This is OK (master and release branches that need it).

Thanks,
Pedro Alves


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