Re: [RFC][PATCH 00/15] Fast tracepoint support for ARMv7

[Yao Qi <qiyaoltc at gmail dot com>]

henrik.wallin@windriver.com writes:

Hi Henrik,
First of all, thanks for your patches...

> This implements fast tracepoint support for ARMv7. It handles both arm
> and thumb mode.
> The first 8 patches are bug-fixes and not directly related to the
> ARM support and should probably be separated out from this patch set,
> but I keep them in here for now. The following 7 patches contains the
> ARM support for fast tracepoint.

As Pedro suggested, please give a test case or steps of reproducing
these problems your first 8 patches fix.

> There are some left-over issues:
> - The breakpoint based tracepoints does not work. There is another
>   thread [1] providing needed gdbserver changes to achieve that. Those
>   patches together with these should provide full tracepoint support for
>   ARMv7. (Not verified)
>
> - Patch "gdbserver: Add help functions to get arm/thumb mode" needs a
>   better solution.
>
> - How to correctly identify that the running ARM cpu is supporting
>   ARMv7-a.

Why do you need to identify the type of ARM cpu? avoid emitting ARMv7
instructions on ARMv6 CPU?

>
> - No updates done to the testsuite.
>
> Known limitations
> - Current implementation will refuse all instruction that do
>   any relative operation with the PC counter, as relocating such an
>   instruction is non-trivial. This can be improved as some of those
>   instructions can be re-written with alternative instruction(s).

Yes, relocating PC relative instructions is complex for fast tracepoint
support on any targets.  However, if you have free cycles in the future,
I'd like to see these PC relative instructions are supported in your
fast tracepoint patches.

>
> - There is no included jit tracepoint expression optimization
>   included. This should be possible to add.
>

This needs much work too.

> - Can only set a tracepoint at a 4 bytes instructions, as a 2 bytes
>   slot is too small for the needed jump instruction. This feels like a
>   natural limitation for this functionality, but means that for thumb
>   the user will hit this limitation quite often.

Sounds reasonable to me.

>
> - The inserted 4 byte jump instruction has a limitation on how far it
>   can branch. The length is validated, but if too far it will not be
>   possible to insert the tracepoint. With very large programs this
>   might be an issue, but again feels like a natural limitation.

Agreed.

>   
> Testing
> - The internal testsystem has not been run, but I will look into
>   that. I've run other tests on it though and I have no known
>   problems.

Please run GDB testsuite, they are very useful.

One issue you didn't mention here is that GDBserver can't move threads
out of jump pad without hardware single step.  See
linux-low.c:linux_resume_one_lwp_throw:

  else if (fast_tp_collecting == 2)
    {
      if (debug_threads)
	debug_printf ("lwp %ld wants to get out of fast tracepoint jump pad"
		      " single-stepping\n",
		      lwpid_of (thread));

      if (can_hardware_single_step ())
	step = 1;
      else
	{
	  internal_error (__FILE__, __LINE__,
			  "moving out of jump pad single-stepping"
			  " not implemented on this target");
	}

      /* Postpone any pending signal.  It was enqueued above.  */
      signal = 0;
    }

I don't know how much GDBserver software single step (not implemented
yet) can help here.

-- 
Yao (éå)