Re: [PATCH 3/3] [GDBserver] Make Zx/zx packet handling idempotent.

[Pedro Alves <palves@redhat.com>]

On 05/21/2014 03:28 AM, Yao Qi wrote:
> The patch is good!
> 
> On 04/24/2014 07:41 AM, Pedro Alves wrote:
>> So the fix is to generalize mem-break.c to track all kinds of Zx
>> breakpoints, and filter out duplicates.  As mentioned, this ends up
>> adding support for target-side conditions on hardware breakpoints and
>> watchpoints too (though GDB itself doesn't support the latter yet).
>>
> 
> Can GDBserver returns feature "ConditionalWatchpoints" for qSupported
> query now?

I think so, yes, though I haven't tried it.  (and I'm not planning to,
feel free to pick that.)

>> As software/memory breakpoints work by poking at memory, when setting
>> a GDB Z0 breakpoint (but not internal breakpoints, as those can assume
>> the conditions are already right), we need to tell the target to
>> prepare to access memory (which on Linux means stop threads).  If that
>> operation fails, we need to return error to GDB.  Seeing an error, if
>> this is the first breakpoint of that type that GDB tries to insert,
>> GDB would then assume the breakpoint type is supported, but it may
>> actually not be.  So we need to check whether the type is supported at
> 
> I am confused by this, isn't it "GDB would then assume the breakpoint
> type is NOT supported, but it may actually be"?

Nope.  It's like this:

Say Z0 is NOT supported by the target.

- GDB sends Z0
- GDBserver asks the backend to prepare to access memory
- GDBserver's target backend tries to stop threads or whatever
- that fails, and GDBserver returns E01
- GDB interprets that as:
      "target supports Z0, but something went wrong with
       inserting the breakpoint, say, e.g., that address is
       unmapped?"
- GDB now "knows" the target supports Z0, so sends another
  the next time a breakpoint needs to be inserted
- This time preparing to access memory actually suceeds,
  and then GDBserver finds out the target backend actually
  doesn't support Z0.
- GDBserver returns the empty reply, indicating no support
  for Z0.
- GDB hits this error in packet_ok:

      case PACKET_UNKNOWN:
      /* The stub does not support the packet.  */
      if (config->detect == AUTO_BOOLEAN_AUTO
	  && config->support == PACKET_ENABLE)
	{
	  /* If the stub previously indicated that the packet was
	     supported then there is a protocol error.  */
	  error (_("Protocol error: %s (%s) conflicting enabled responses."),
		 config->name, config->title);
	}


That's why the sequence must be:

- GDB sends Z0
- GDBserver checks whether the target backend supports Z0 before
  actually trying to do anything to the inferior.
- The backend says it doesn't support Z0.
- GDBserver returns empty reply.

-- 
Pedro Alves