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

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

[henrik.wallin]

From: Henrik Wallin <henrik.wallin@windriver.com>

Hi,

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.

I post this as an RFC at this point, as I'm not actively working
with this. I will response and post updates to this series, but any major
improvements / changes will be hard to achieve for me at this point.

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.

- 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).

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

- 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.

- 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.
  
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.

Patch description:

Patches are based on master, e753e154bf8a1f507b43e03dec04b341dde3f429

patch 1-8:
Bugfixes in generic code. Not specific to ARM. Should probably be
split out from this patch set.

patch 9-13: Adds new code (but not activated) for ARM tracepoints.

patch 14-15: Activates and makes tracepoints available.

[1]: https://sourceware.org/ml/gdb-patches/2015-09/msg00221.html

Henrik Wallin (9):
  Fix mmap usage of MAP_FIXED for multiple pages.
  gdbserver: Move pointer dereference to after assert checks.
  gdb: Add relocate instruction helpers
  gdb: Add arm_fast_tracepoint_valid_at
  gdbserver: Add helper functions to create arm instructions
  gdbserver: Add help functions to get arm/thumb mode
  gdbserver: Add arm_install_fast_tracepoint_jump_pad
  gdbserver: Add linux-arm-ipa.c
  gdb/gdbserver: Enable tracepoint for ARM

Par Olsson (6):
  Fix endian problem for tracepoint enabled flag
  Fix internal error when saving fast tracepoint definitions
  Fix crash in enable/disable after detach
  Fix crash in tstatus after detach
  Fix crash when tstart after detach.
  Add possibility to restart trace after tfind.

 gdb/arm-tdep.c                | 658 ++++++++++++++++++++++++++++++++++++++++++
 gdb/breakpoint.c              |   2 +-
 gdb/common/agent.c            |   7 +
 gdb/gdbserver/Makefile.in     |   6 +
 gdb/gdbserver/configure.srv   |   1 +
 gdb/gdbserver/linux-arm-ipa.c |  97 +++++++
 gdb/gdbserver/linux-arm-low.c | 410 +++++++++++++++++++++++++-
 gdb/gdbserver/tracepoint.c    |  33 ++-
 gdb/tracepoint.c              |  14 +-
 9 files changed, 1218 insertions(+), 10 deletions(-)
 create mode 100644 gdb/gdbserver/linux-arm-ipa.c

-- 
2.1.4

[RFC][PATCH 01/15] Fix endian problem for tracepoint enabled flag

[henrik.wallin]

From: Par Olsson <par.olsson@windriver.com>

When running big endian machines there is a problem with
the enabled flag for tracepoints as it is defined as a
int8_t but written from gdbserver as an integer and then
read in the agent as 8-bit value.
This caused problem when tracepoint was disabled and
re-enabled.

gdb/gdbserver/ChangeLog:

	* tracepoint.c (struct tracepoint): Change type of enabled.

Signed-off-by: Par Olsson <par.olsson@windriver.com>
Signed-off-by: Henrik Wallin <henrik.wallin@windriver.com>
---
 gdb/gdbserver/tracepoint.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gdb/gdbserver/tracepoint.c b/gdb/gdbserver/tracepoint.c
index b6c70c9cc7a0..a2723e39e500 100644
--- a/gdb/gdbserver/tracepoint.c
+++ b/gdb/gdbserver/tracepoint.c
@@ -724,7 +724,7 @@ struct tracepoint
   enum tracepoint_type type;
 
   /* True if the tracepoint is currently enabled.  */
-  int8_t enabled;
+  uint32_t enabled;
 
   /* The number of single steps that will be performed after each
      tracepoint hit.  */
-- 
2.1.4

Re: [RFC][PATCH 01/15] Fix endian problem for tracepoint enabled flag

[Pedro Alves]

On 10/14/2015 12:14 PM, henrik.wallin@windriver.com wrote:
> From: Par Olsson <par.olsson@windriver.com>
> 
> When running big endian machines there is a problem with
> the enabled flag for tracepoints as it is defined as a
> int8_t but written from gdbserver as an integer and then
> read in the agent as 8-bit value.
> This caused problem when tracepoint was disabled and
> re-enabled.
> 
> gdb/gdbserver/ChangeLog:
> 
> 	* tracepoint.c (struct tracepoint): Change type of enabled.

Seems wasteful for no obvious reason.  Why not make gdbserver write
one byte then?

Thanks,
Pedro Alves

RE: [RFC][PATCH 01/15] Fix endian problem for tracepoint enabled flag

[Wallin, Henrik]

2015-10-15 19:16 GMT+02:00 Pedro Alves <palves@redhat.com>:
> On 10/14/2015 12:14 PM, henrik.wallin@windriver.com wrote:
>> From: Par Olsson <par.olsson@windriver.com>
>>
>> When running big endian machines there is a problem with
>> the enabled flag for tracepoints as it is defined as a
>> int8_t but written from gdbserver as an integer and then
>> read in the agent as 8-bit value.
>> This caused problem when tracepoint was disabled and
>> re-enabled.
>>
>> gdb/gdbserver/ChangeLog:
>>
>>       * tracepoint.c (struct tracepoint): Change type of enabled.
>
> Seems wasteful for no obvious reason.  Why not make gdbserver write
> one byte then?

I guess this was the easy fix... I will change the write instead.

thanks,
/ Henrik

[RFC][PATCH 02/15] Fix internal error when saving fast tracepoint definitions

[henrik.wallin]

From: Par Olsson <par.olsson@windriver.com>

When trying to save fast tracepoints to file, gdb returns internal failure:

  gdb/breakpoint.c:13446: internal-error: unhandled tracepoint type 27
  A problem internal to GDB has been detected, further debugging may prove unreliable.

And no file including the fast tracepoints definition is created.

gdb/ChangeLog:

	* breakpoint.c (tracepoint_print_recreate): Fix logic error
	if -> else if.

Signed-off-by: Par Olsson <par.olsson@windriver.com>
Signed-off-by: Henrik Wallin <henrik.wallin@windriver.com>
---
 gdb/breakpoint.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 2c901ff5f2fe..106464888500 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -13423,7 +13423,7 @@ tracepoint_print_recreate (struct breakpoint *self, struct ui_file *fp)
 
   if (self->type == bp_fast_tracepoint)
     fprintf_unfiltered (fp, "ftrace");
-  if (self->type == bp_static_tracepoint)
+  else if (self->type == bp_static_tracepoint)
     fprintf_unfiltered (fp, "strace");
   else if (self->type == bp_tracepoint)
     fprintf_unfiltered (fp, "trace");
-- 
2.1.4

Re: [RFC][PATCH 02/15] Fix internal error when saving fast tracepoint definitions

[Pedro Alves]

On 10/14/2015 12:14 PM, henrik.wallin@windriver.com wrote:
> From: Par Olsson <par.olsson@windriver.com>
> 
> When trying to save fast tracepoints to file, gdb returns internal failure:
> 
>   gdb/breakpoint.c:13446: internal-error: unhandled tracepoint type 27
>   A problem internal to GDB has been detected, further debugging may prove unreliable.
> 
> And no file including the fast tracepoints definition is created.
> 
> gdb/ChangeLog:
> 
> 	* breakpoint.c (tracepoint_print_recreate): Fix logic error
> 	if -> else if.

This looks good, but I think we have tests that save/restore breakpoints.
Can we extend them to cover this as well?

Thanks,
Pedro Alves

[RFC][PATCH 04/15] Fix crash in tstatus after detach

[henrik.wallin]

From: Par Olsson <par.olsson@windriver.com>

When calling tstatus after detaching the process,
gdbserver tries to access inferior memory which
results in a crash.
This changes the behavior of the agent_loaded_p()
to return false if no inferior is loaded.

gdb/ChangeLog:

	* agent.c (agent_loaded_p): Add check that inferior is present.

Signed-off-by: Par Olsson <par.olsson@windriver.com>
Signed-off-by: Henrik Wallin <henrik.wallin@windriver.com>
---
 gdb/common/agent.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/gdb/common/agent.c b/gdb/common/agent.c
index 5c307290589d..c9b6c41bc4ff 100644
--- a/gdb/common/agent.c
+++ b/gdb/common/agent.c
@@ -73,9 +73,16 @@ static struct ipa_sym_addresses ipa_sym_addrs;
 
 static int all_agent_symbols_looked_up = 0;
 
+#ifdef GDBSERVER
+#include <inferiors.h>
+#endif
 int
 agent_loaded_p (void)
 {
+#ifdef GDBSERVER
+  if (current_thread == NULL)
+    return 0;
+#endif
   return all_agent_symbols_looked_up;
 }
 
-- 
2.1.4

Re: [RFC][PATCH 04/15] Fix crash in tstatus after detach

[Gary Benson]

Hi Henrik,

henrik.wallin@windriver.com wrote:
> diff --git a/gdb/common/agent.c b/gdb/common/agent.c
> index 5c307290589d..c9b6c41bc4ff 100644
> --- a/gdb/common/agent.c
> +++ b/gdb/common/agent.c
> @@ -73,9 +73,16 @@ static struct ipa_sym_addresses ipa_sym_addrs;
>  
>  static int all_agent_symbols_looked_up = 0;
>  
> +#ifdef GDBSERVER
> +#include <inferiors.h>
> +#endif
>  int
>  agent_loaded_p (void)
>  {
> +#ifdef GDBSERVER
> +  if (current_thread == NULL)
> +    return 0;
> +#endif
>    return all_agent_symbols_looked_up;
>  }
>  

Please don't introduce "#ifdef GDBSERVER" conditionals into common
code, I spent some time removing them last year.  I know I didn't
get them all, but the remaining two are on my hit list :)

Thanks,
Gary

-- 
http://gbenson.net/

RE: [RFC][PATCH 04/15] Fix crash in tstatus after detach

[Wallin, Henrik]

2015-10-14 13:48 GMT+02:00 Gary Benson <gbenson@redhat.com>:
> Hi Henrik,
>
> henrik.wallin@windriver.com wrote:
>> diff --git a/gdb/common/agent.c b/gdb/common/agent.c
>> index 5c307290589d..c9b6c41bc4ff 100644
>> --- a/gdb/common/agent.c
>> +++ b/gdb/common/agent.c
>> @@ -73,9 +73,16 @@ static struct ipa_sym_addresses ipa_sym_addrs;
>>
>>  static int all_agent_symbols_looked_up = 0;
>>
>> +#ifdef GDBSERVER
>> +#include <inferiors.h>
>> +#endif
>>  int
>>  agent_loaded_p (void)
>>  {
>> +#ifdef GDBSERVER
>> +  if (current_thread == NULL)
>> +    return 0;
>> +#endif
>>    return all_agent_symbols_looked_up;
>>  }
>>
>
> Please don't introduce "#ifdef GDBSERVER" conditionals into common
> code, I spent some time removing them last year.  I know I didn't
> get them all, but the remaining two are on my hit list :)

Ok :)
I will check that.

thanks,
/ Henrik

Re: [RFC][PATCH 04/15] Fix crash in tstatus after detach

[Gary Benson]

Wallin, Henrik wrote:
> 2015-10-14 13:48 GMT+02:00 Gary Benson <gbenson@redhat.com>:
> > henrik.wallin@windriver.com wrote:
> > > diff --git a/gdb/common/agent.c b/gdb/common/agent.c
> > > index 5c307290589d..c9b6c41bc4ff 100644
> > > --- a/gdb/common/agent.c
> > > +++ b/gdb/common/agent.c
> > > @@ -73,9 +73,16 @@ static struct ipa_sym_addresses ipa_sym_addrs;
> > >
> > >  static int all_agent_symbols_looked_up = 0;
> > >
> > > +#ifdef GDBSERVER
> > > +#include <inferiors.h>
> > > +#endif
> > >  int
> > >  agent_loaded_p (void)
> > >  {
> > > +#ifdef GDBSERVER
> > > +  if (current_thread == NULL)
> > > +    return 0;
> > > +#endif
> > >    return all_agent_symbols_looked_up;
> > >  }
> > >
> >
> > Please don't introduce "#ifdef GDBSERVER" conditionals into common
> > code, I spent some time removing them last year.  I know I didn't
> > get them all, but the remaining two are on my hit list :)
> 
> Ok :)
> I will check that.

Thanks Henrik.

Cheers,
Gary

-- 
http://gbenson.net/

Re: [RFC][PATCH 04/15] Fix crash in tstatus after detach

[Pedro Alves]

On 10/14/2015 12:14 PM, henrik.wallin@windriver.com wrote:
> From: Par Olsson <par.olsson@windriver.com>
> 
> When calling tstatus after detaching the process,
> gdbserver tries to access inferior memory which
> results in a crash.
> This changes the behavior of the agent_loaded_p()
> to return false if no inferior is loaded.

Sounds like it should be easy to cook up a testcase.
Could you do that?

> 
> gdb/ChangeLog:
> 
> 	* agent.c (agent_loaded_p): Add check that inferior is present.
> 
> Signed-off-by: Par Olsson <par.olsson@windriver.com>
> Signed-off-by: Henrik Wallin <henrik.wallin@windriver.com>
> ---
>  gdb/common/agent.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/gdb/common/agent.c b/gdb/common/agent.c
> index 5c307290589d..c9b6c41bc4ff 100644
> --- a/gdb/common/agent.c
> +++ b/gdb/common/agent.c
> @@ -73,9 +73,16 @@ static struct ipa_sym_addresses ipa_sym_addrs;
>  
>  static int all_agent_symbols_looked_up = 0;
>  
> +#ifdef GDBSERVER
> +#include <inferiors.h>
> +#endif
>  int
>  agent_loaded_p (void)
>  {
> +#ifdef GDBSERVER
> +  if (current_thread == NULL)
> +    return 0;
> +#endif
>    return all_agent_symbols_looked_up;
>  }
>  

Should probably be moved up to the caller.

Thanks,
Pedro Alves

RE: [RFC][PATCH 04/15] Fix crash in tstatus after detach

[Wallin, Henrik]

2015-10-15 19:22 GMT+02:00 Pedro Alves <palves@redhat.com>:
> On 10/14/2015 12:14 PM, henrik.wallin@windriver.com wrote:
>> From: Par Olsson <par.olsson@windriver.com>
>>
>> When calling tstatus after detaching the process,
>> gdbserver tries to access inferior memory which
>> results in a crash.
>> This changes the behavior of the agent_loaded_p()
>> to return false if no inferior is loaded.
>
> Sounds like it should be easy to cook up a testcase.
> Could you do that?

I will do, after I manage to actually reproduce this one...
The patch was done on 7.6 and then rebased. I will check some more but it looks like this might've been fixed by some other changes between 7.6 and master.

>
>>
>> gdb/ChangeLog:
>>
>>       * agent.c (agent_loaded_p): Add check that inferior is present.
>>
>> Signed-off-by: Par Olsson <par.olsson@windriver.com>
>> Signed-off-by: Henrik Wallin <henrik.wallin@windriver.com>
>> ---
>>  gdb/common/agent.c | 7 +++++++
>>  1 file changed, 7 insertions(+)
>>
>> diff --git a/gdb/common/agent.c b/gdb/common/agent.c
>> index 5c307290589d..c9b6c41bc4ff 100644
>> --- a/gdb/common/agent.c
>> +++ b/gdb/common/agent.c
>> @@ -73,9 +73,16 @@ static struct ipa_sym_addresses ipa_sym_addrs;
>>
>>  static int all_agent_symbols_looked_up = 0;
>>
>> +#ifdef GDBSERVER
>> +#include <inferiors.h>
>> +#endif
>>  int
>>  agent_loaded_p (void)
>>  {
>> +#ifdef GDBSERVER
>> +  if (current_thread == NULL)
>> +    return 0;
>> +#endif
>>    return all_agent_symbols_looked_up;
>>  }
>>
>
> Should probably be moved up to the caller.

I will check that.

thanks,
/ Henrik

[RFC][PATCH 03/15] Fix crash in enable/disable after detach

[henrik.wallin]

From: Par Olsson <par.olsson@windriver.com>

When calling enable/disable on a fast tracepoint
after detaching the process, gdbserver tries to
access inferior memory which results in a crash.

gdb/gdbserver/ChangeLog:

	* tracepoint.c (cmd_qtenable_disable): Fix problem with
	enabling tracepoint after inferior have disconnected.

Signed-off-by: Par Olsson <par.olsson@windriver.com>
Signed-off-by: Henrik Wallin <henrik.wallin@windriver.com>
---
 gdb/gdbserver/tracepoint.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/gdb/gdbserver/tracepoint.c b/gdb/gdbserver/tracepoint.c
index a2723e39e500..78a5eb028b72 100644
--- a/gdb/gdbserver/tracepoint.c
+++ b/gdb/gdbserver/tracepoint.c
@@ -2775,14 +2775,20 @@ cmd_qtenable_disable (char *own_buf, int enable)
 		   enable ? "Enabling" : "Disabling",
 		   (int) num, paddress (addr));
 
-      tp->enabled = enable;
-
       if (tp->type == fast_tracepoint || tp->type == static_tracepoint)
 	{
 	  int ret;
 	  int offset = offsetof (struct tracepoint, enabled);
 	  CORE_ADDR obj_addr = tp->obj_addr_on_target + offset;
 
+	  if (current_thread == NULL)
+	    {
+	      trace_debug ("Trying to enable/disable the tracepoint "
+			   "without inferior");
+	      strcpy (own_buf, "E.No process attached.");
+	      return;
+	    }
+
 	  ret = prepare_to_access_memory ();
 	  if (ret)
 	    {
@@ -2803,6 +2809,8 @@ cmd_qtenable_disable (char *own_buf, int enable)
 	    }
 	}
 
+      tp->enabled = enable;
+
       write_ok (own_buf);
     }
   else
-- 
2.1.4

Re: [RFC][PATCH 03/15] Fix crash in enable/disable after detach

[Pedro Alves]

On 10/14/2015 12:14 PM, henrik.wallin@windriver.com wrote:
> From: Par Olsson <par.olsson@windriver.com>
> 
> When calling enable/disable on a fast tracepoint
> after detaching the process, gdbserver tries to
> access inferior memory which results in a crash.
> 
> gdb/gdbserver/ChangeLog:
> 
> 	* tracepoint.c (cmd_qtenable_disable): Fix problem with
> 	enabling tracepoint after inferior have disconnected.

Can you write/show a test case?

Thanks,
Pedro Alves

RE: [RFC][PATCH 03/15] Fix crash in enable/disable after detach

[Wallin, Henrik]

2015-10-15 18:30 GMT+02:00 Pedro Alves <palves@redhat.com>:
> On 10/14/2015 12:14 PM, henrik.wallin@windriver.com wrote:
>> From: Par Olsson <par.olsson@windriver.com>
>>
>> When calling enable/disable on a fast tracepoint
>> after detaching the process, gdbserver tries to
>> access inferior memory which results in a crash.
>>
>> gdb/gdbserver/ChangeLog:
>>
>>       * tracepoint.c (cmd_qtenable_disable): Fix problem with
>>       enabling tracepoint after inferior have disconnected.
>
> Can you write/show a test case?

Yes, I will loook into adding that. Same for the other ones :)

thanks,
/ Henrik

>
> Thanks,
> Pedro Alves
>

[RFC][PATCH 08/15] gdbserver: Move pointer dereference to after assert checks.

[henrik.wallin]

From: Henrik Wallin <henrik.wallin@windriver.com>

gdb/gdbserver/ChangeLog:

	* linux-arm-low.c (arm_new_thread): Move pointer dereference
	to after assert checks.

Signed-off-by: Henrik Wallin <henrik.wallin@windriver.com>
---
 gdb/gdbserver/linux-arm-low.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/gdb/gdbserver/linux-arm-low.c b/gdb/gdbserver/linux-arm-low.c
index a277bb6876fb..81d23d953434 100644
--- a/gdb/gdbserver/linux-arm-low.c
+++ b/gdb/gdbserver/linux-arm-low.c
@@ -686,8 +686,8 @@ arm_new_thread (struct lwp_info *lwp)
 static void
 arm_new_fork (struct process_info *parent, struct process_info *child)
 {
-  struct arch_process_info *parent_proc_info = parent->priv->arch_private;
-  struct arch_process_info *child_proc_info = child->priv->arch_private;
+  struct arch_process_info *parent_proc_info;
+  struct arch_process_info *child_proc_info;
   struct lwp_info *child_lwp;
   struct arch_lwp_info *child_lwp_info;
   int i;
@@ -698,6 +698,9 @@ arm_new_fork (struct process_info *parent, struct process_info *child)
   gdb_assert (child->priv != NULL
 	      && child->priv->arch_private != NULL);
 
+  parent_proc_info = parent->priv->arch_private;
+  child_proc_info = child->priv->arch_private;
+
   /* Linux kernel before 2.6.33 commit
      72f674d203cd230426437cdcf7dd6f681dad8b0d
      will inherit hardware debug registers from parent
-- 
2.1.4

Re: [RFC][PATCH 08/15] gdbserver: Move pointer dereference to after assert checks.

[Pedro Alves]

On 10/14/2015 12:14 PM, henrik.wallin@windriver.com wrote:
> From: Henrik Wallin <henrik.wallin@windriver.com>
> 
> gdb/gdbserver/ChangeLog:
> 
> 	* linux-arm-low.c (arm_new_thread): Move pointer dereference
> 	to after assert checks.

OK.

Thanks,
Pedro Alves

RE: [RFC][PATCH 08/15] gdbserver: Move pointer dereference to after assert checks.

[Wallin, Henrik]

2015-10-15 19:29 GMT+02:00 Pedro Alves <palves@redhat.com>:
> On 10/14/2015 12:14 PM, henrik.wallin@windriver.com wrote:
>> From: Henrik Wallin <henrik.wallin@windriver.com>
>>
>> gdb/gdbserver/ChangeLog:
>>
>>       * linux-arm-low.c (arm_new_thread): Move pointer dereference
>>       to after assert checks.
>
> OK.

Next step is?
I guess someone with push permissions need to pick this one up?

thanks,
/ Henrik

Re: [RFC][PATCH 08/15] gdbserver: Move pointer dereference to after assert checks.

[Antoine Tremblay]

On 10/29/2015 01:50 PM, Wallin, Henrik wrote:
>
>
> 2015-10-15 19:29 GMT+02:00 Pedro Alves <palves@redhat.com>:
>> On 10/14/2015 12:14 PM, henrik.wallin@windriver.com wrote:
>>> From: Henrik Wallin <henrik.wallin@windriver.com>
>>>
>>> gdb/gdbserver/ChangeLog:
>>>
>>>        * linux-arm-low.c (arm_new_thread): Move pointer dereference
>>>        to after assert checks.
>>
>> OK.
>
> Next step is?
> I guess someone with push permissions need to pick this one up?
>

I can push it for you. (Most likely next week)

Regards,
Antoine

Re: [RFC][PATCH 08/15] gdbserver: Move pointer dereference to after assert checks.

[Antoine Tremblay]

On 10/29/2015 01:53 PM, Antoine Tremblay wrote:
>
>
> On 10/29/2015 01:50 PM, Wallin, Henrik wrote:
>>
>>
>> 2015-10-15 19:29 GMT+02:00 Pedro Alves <palves@redhat.com>:
>>> On 10/14/2015 12:14 PM, henrik.wallin@windriver.com wrote:
>>>> From: Henrik Wallin <henrik.wallin@windriver.com>
>>>>
>>>> gdb/gdbserver/ChangeLog:
>>>>
>>>>        * linux-arm-low.c (arm_new_thread): Move pointer dereference
>>>>        to after assert checks.
>>>
>>> OK.
>>
>> Next step is?
>> I guess someone with push permissions need to pick this one up?
>>
>
> I can push it for you. (Most likely next week)
>

Pushed. I had the time now after all.

[RFC][PATCH 10/15] gdb: Add arm_fast_tracepoint_valid_at

[henrik.wallin]

From: Henrik Wallin <henrik.wallin@windriver.com>

This adds the function. No users yet.

A 4 byte jump instruction will be used and the code need to
handle multiple cases as the instruction can be 2, or 4 bytes long and
the mode can be either arm or thumb.

gdb/ChangeLog:

	* arm-tdep.c (arm_fast_tracepoint_valid_at): New function.

Signed-off-by: Henrik Wallin <henrik.wallin@windriver.com>
---
 gdb/arm-tdep.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 47 insertions(+)

diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
index b277c3ef405c..601d589b8a89 100644
--- a/gdb/arm-tdep.c
+++ b/gdb/arm-tdep.c
@@ -22,6 +22,7 @@
 #include <ctype.h>		/* XXX for isupper ().  */
 
 #include "frame.h"
+#include "disasm.h"
 #include "inferior.h"
 #include "infrun.h"
 #include "gdbcmd.h"
@@ -10506,6 +10507,52 @@ arm_relocate_instruction (struct gdbarch *gdbarch,
   arm_relocate_instruction_func (&rel);
 }
 
+static int
+arm_fast_tracepoint_valid_at (struct gdbarch *gdbarch,
+			      CORE_ADDR addr, char **msg)
+{
+  static struct ui_file *gdb_null = NULL;
+  int len;
+
+  /* Check if the instruction is relocatable.   */
+  if (arm_check_relocate_instruction (gdbarch, addr, msg) == -1)
+    return 0;
+
+  /* Dummy file descriptor for the disassembler.  */
+  if (!gdb_null)
+    gdb_null = ui_file_new ();
+
+  /* A branch instruction used for fast tracepoint takes 4 bytes.
+     (A 2 bytes branch instruction only gets us 4k away,
+     so will not be enough.)
+
+     target gdbserver will validate that the relative branch
+     distance will fit in the instructions.
+     (16M for Thumb, 32M for ARM)
+
+     We only allow to replace one instuction. (4 bytes)
+     Replacing 2 instructions is not safe. Consider
+     the case where code wants to jump to the 2nd instruction - it
+     will jump into the middle of a branch instruction.   */
+
+  if (arm_pc_is_thumb (gdbarch, addr))
+    {
+      len = gdb_print_insn (gdbarch, addr, gdb_null, NULL);
+      if (len == 2)
+	{
+	  if (msg)
+	    *msg = xstrprintf (_("; instruction is only 2 bytes long, "
+				 "need 4 bytes for the jump"));
+	  return 0;
+	}
+    }
+
+  if (msg)
+    *msg = NULL;
+
+  return 1;
+}
+
 \f
 /* Initialize the current architecture based on INFO.  If possible,
    re-use an architecture from ARCHES, which is a list of
-- 
2.1.4

Re: [RFC][PATCH 10/15] gdb: Add arm_fast_tracepoint_valid_at

[Yao Qi]

henrik.wallin@windriver.com writes:

> +  if (arm_pc_is_thumb (gdbarch, addr))
> +    {
> +      len = gdb_print_insn (gdbarch, addr, gdb_null, NULL);

We don't need to call gdb_print_insn to know the instruction size.
Instead, we can do something simpler,

  if (arm_pc_is_thumb (gdbarch, pc))
    {
       len = thumb_insn_size (inst1);
    }
  else
    len = 4;

See how it is done in arm-tdep.c:arm_breakpoint_from_pc

-- 
Yao (齐尧)

RE: [RFC][PATCH 10/15] gdb: Add arm_fast_tracepoint_valid_at

[Wallin, Henrik]

2015-10-27 12:25 GMT+01:00 Yao Qi <qiyaoltc@gmail.com>:
> henrik.wallin@windriver.com writes:
>
>> +  if (arm_pc_is_thumb (gdbarch, addr))
>> +    {
>> +      len = gdb_print_insn (gdbarch, addr, gdb_null, NULL);
>
> We don't need to call gdb_print_insn to know the instruction size.
> Instead, we can do something simpler,
>
>   if (arm_pc_is_thumb (gdbarch, pc))
>     {
>        len = thumb_insn_size (inst1);
>     }
>   else
>     len = 4;

yes, that looks better.

thanks,
/ Henrik

[RFC][PATCH 07/15] Fix mmap usage of MAP_FIXED for multiple pages.

[henrik.wallin]

From: Henrik Wallin <henrik.wallin@windriver.com>

mmap using MAP_FIXED will overwrite mapped pages if multiple pages
are requested.
E.g. On ARM it will result in overwriting the main code pages.

Fix by not using MAP_FIXED and use mmunmap in case we
don't get the start address we asked for.

gdb/gdbserver/ChangeLog:

	* tracepoint.c : Include stdint.h and sys/mman.h.
	(initialize_tracepoint): Fix problem with
	MAP_FIXED usage in mmap.

Signed-off-by: Henrik Wallin <henrik.wallin@windriver.com>
---
 gdb/gdbserver/tracepoint.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/gdb/gdbserver/tracepoint.c b/gdb/gdbserver/tracepoint.c
index d2ad197e58ab..35a125c951d5 100644
--- a/gdb/gdbserver/tracepoint.c
+++ b/gdb/gdbserver/tracepoint.c
@@ -27,6 +27,9 @@
 #include <unistd.h>
 #include "gdb_sys_time.h"
 #include <inttypes.h>
+#include <stdint.h>
+#include <sys/mman.h>
+
 #include "ax.h"
 #include "tdesc.h"
 
@@ -7392,10 +7395,12 @@ initialize_tracepoint (void)
 	  = (char *) mmap ((void *) addr,
 			   pagesize * SCRATCH_BUFFER_NPAGES,
 			   PROT_READ | PROT_WRITE | PROT_EXEC,
-			   MAP_PRIVATE | MAP_ANONYMOUS | MAP_FIXED,
+			   MAP_PRIVATE | MAP_ANONYMOUS,
 			   -1, 0);
-	if (gdb_jump_pad_buffer != MAP_FAILED)
+	if (gdb_jump_pad_buffer == (void *)addr)
 	  break;
+	if (gdb_jump_pad_buffer != MAP_FAILED)
+	  munmap(gdb_jump_pad_buffer, pagesize * SCRATCH_BUFFER_NPAGES);
       }
 
     if (addr == 0)
-- 
2.1.4

Re: [RFC][PATCH 07/15] Fix mmap usage of MAP_FIXED for multiple pages.

[Andreas Schwab]

henrik.wallin@windriver.com writes:

> mmap using MAP_FIXED will overwrite mapped pages if multiple pages
> are requested.

It's also true if you request a single page.

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

RE: [RFC][PATCH 07/15] Fix mmap usage of MAP_FIXED for multiple pages.

[Wallin, Henrik]

2015-10-14 17:26 GMT+02:00 Andreas Schwab <schwab@suse.de>:
> henrik.wallin@windriver.com writes:
>
>> mmap using MAP_FIXED will overwrite mapped pages if multiple pages
>> are requested.
>
> It's also true if you request a single page.

True, I will adjust the commit message.

thanks,
/ Henrik

[RFC][PATCH 12/15] gdbserver: Add help functions to get arm/thumb mode

[henrik.wallin]

From: Henrik Wallin <henrik.wallin@windriver.com>

This adds new function. No users yet.

This is a quick dirty way to ask gdb what the mode is.
The relocate_instruction function is used with a bogus address.
gdb side advances the to address if arm mode, otherwise not.

gdb/ChangeLog:

        * arm-tdep.c (arm_relocate_instruction_func) : Add code
	to handle special case address value.

gdb/gdbserver/ChangeLog:

	* linux-arm-low.c (is_target_arm) : New function.

Signed-off-by: Henrik Wallin <henrik.wallin@windriver.com>
---
 gdb/arm-tdep.c                | 9 ++++++++-
 gdb/gdbserver/linux-arm-low.c | 9 +++++++++
 2 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
index 601d589b8a89..74c58eb91c4d 100644
--- a/gdb/arm-tdep.c
+++ b/gdb/arm-tdep.c
@@ -10447,7 +10447,14 @@ arm_relocate_instruction_func (struct relocate_insn *rel)
   rel->byte_order_for_code = gdbarch_byte_order_for_code (rel->gdbarch);
   rel->result = 0;
 
-  if (arm_pc_is_thumb (rel->gdbarch, rel->oldloc))
+  if (rel->oldloc == 0xFFFFFFFF)
+    {
+      uint32_t tmp = read_memory_unsigned_integer (*(rel->to), 4,
+						   rel->byte_order_for_code);
+      if (! arm_pc_is_thumb (rel->gdbarch, tmp))
+	*rel->to += 1;
+    }
+  else if (arm_pc_is_thumb (rel->gdbarch, rel->oldloc))
     {
       uint16_t insn1;
       uint16_t insn2;
diff --git a/gdb/gdbserver/linux-arm-low.c b/gdb/gdbserver/linux-arm-low.c
index 044e7527a3b0..1853e79bc140 100644
--- a/gdb/gdbserver/linux-arm-low.c
+++ b/gdb/gdbserver/linux-arm-low.c
@@ -917,6 +917,15 @@ arm_regs_info (void)
     return &regs_info_arm;
 }
 
+static int
+is_target_arm (CORE_ADDR to, CORE_ADDR from)
+{
+  CORE_ADDR ptr = to;
+  write_inferior_memory (to, (unsigned char *) &from, 4);
+  relocate_instruction (&ptr, 0xFFFFFFFF);
+  return (to == ptr) ? 0 : 1;
+}
+
 static uint32_t
 mk_t_b_rel (CORE_ADDR from, CORE_ADDR to)
 {
-- 
2.1.4

[RFC][PATCH 06/15] Add possibility to restart trace after tfind.

[henrik.wallin]

From: Par Olsson <par.olsson@windriver.com>

After starting examine the collected trace data
with tfind it was not possible to re-start a trace session.
This patch will add a question if the user wants to
drop all collected data and start a new session.

gdb/ChangeLog:

	* tracepoint.c : Initiate traceframe_number.
	(trace_start_command): Add interactive question if
	user wants to clear and restart a trace session when issuing a
	tstart while in progress examining trace data.

Signed-off-by: Par Olsson <par.olsson@windriver.com>
Signed-off-by: Henrik Wallin <henrik.wallin@windriver.com>
---
 gdb/tracepoint.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/gdb/tracepoint.c b/gdb/tracepoint.c
index 6ca66e7c8d18..4aad1d6a4197 100644
--- a/gdb/tracepoint.c
+++ b/gdb/tracepoint.c
@@ -123,7 +123,7 @@ static VEC(tsv_s) *tvariables;
 static int next_tsv_number = 1;
 
 /* Number of last traceframe collected.  */
-static int traceframe_number;
+static int traceframe_number = -1;
 
 /* Tracepoint for last traceframe collected.  */
 static int tracepoint_number;
@@ -1914,6 +1914,18 @@ trace_start_command (char *args, int from_tty)
 	  && !query (_("A trace is running already.  Start a new run? ")))
 	error (_("New trace run not started."));
     }
+  if (traceframe_number >= 0)
+    {
+      if (from_tty
+	  && !query (_("Currently examining trace data. "
+		       "Start a new run? ")))
+	error (_("New trace run not started."));
+      /* Reset our local state.  */
+      set_traceframe_num (-1);
+      set_tracepoint_num (-1);
+      set_traceframe_context (NULL);
+      clear_traceframe_info ();
+    }
 
   start_tracing (args);
 }
-- 
2.1.4

Re: [RFC][PATCH 06/15] Add possibility to restart trace after tfind.

[Pedro Alves]

On 10/14/2015 12:14 PM, henrik.wallin@windriver.com wrote:
> From: Par Olsson <par.olsson@windriver.com>
> 
> After starting examine the collected trace data
> with tfind it was not possible to re-start a trace session.

What would happen?

> This patch will add a question if the user wants to
> drop all collected data and start a new session.

Testcase.

Thanks,
Pedro Alves

[RFC][PATCH 05/15] Fix crash when tstart after detach.

[henrik.wallin]

From: Par Olsson <par.olsson@windriver.com>

When calling tstart after detaching the process,
gdbserver tries to access inferior memory which
results in a crash.
Also add error message when calling tstart if
there is no inferior.

gdb/gdbserver/ChangeLog:

	* tracepoint.c (cmd_qtinit): Add check that inferior is
	present before calling clear_inferior_trace_buffer.
	(cmd_qtstart): Add check that inferior is present.

Signed-off-by: Par Olsson <par.olsson@windriver.com>
Signed-off-by: Henrik Wallin <henrik.wallin@windriver.com>
---
 gdb/gdbserver/tracepoint.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/gdb/gdbserver/tracepoint.c b/gdb/gdbserver/tracepoint.c
index 78a5eb028b72..d2ad197e58ab 100644
--- a/gdb/gdbserver/tracepoint.c
+++ b/gdb/gdbserver/tracepoint.c
@@ -2402,7 +2402,8 @@ cmd_qtinit (char *packet)
     }
 
   clear_trace_buffer ();
-  clear_inferior_trace_buffer ();
+  if (agent_loaded_p ())
+    clear_inferior_trace_buffer ();
 
   write_ok (packet);
 }
@@ -3203,6 +3204,13 @@ cmd_qtstart (char *packet)
   struct tracepoint *tpoint, *prev_ftpoint, *prev_stpoint;
   CORE_ADDR tpptr = 0, prev_tpptr = 0;
 
+  if (current_thread == NULL)
+    {
+      trace_debug ("Trying to start the trace without inferior");
+      strcpy (packet, "E.No process attached.");
+      return;
+    }
+
   trace_debug ("Starting the trace");
 
   /* Pause all threads temporarily while we patch tracepoints.  */
-- 
2.1.4

Re: [RFC][PATCH 05/15] Fix crash when tstart after detach.

[Pedro Alves]

On 10/14/2015 12:14 PM, henrik.wallin@windriver.com wrote:
> From: Par Olsson <par.olsson@windriver.com>
> 
> When calling tstart after detaching the process,
> gdbserver tries to access inferior memory which
> results in a crash.
> Also add error message when calling tstart if
> there is no inferior.
> 

You know what I'll ask for by now.  :-)

Thanks,
Pedro Alves

[RFC][PATCH 15/15] gdb/gdbserver: Enable tracepoint for ARM

[henrik.wallin]

From: Henrik Wallin <henrik.wallin@windriver.com>

This enables tracepoint for ARM.

- Fast tracepoint.
  A fast tracepoint can be set in thumb and arm mode code.
  Validation is done for the instruction to relocate. Currently
  all PC-relative instructions are not possible to relocate.
  The inserted branch instruction needs to be 4 bytes, which also
  limit the possible instructions to relocate in thumb mode.
  It should be possible to rewrite some instructions, so more
  instructions are relocatable.

- Breakpoint based tracepoint does not work
  This needs new infrastructure in gdbserver for setting breakpoints
  and to step over an instruction. The infrastructure to be able to
  do this is currently only available in gdb.
  There is no added error check for this.

- jit compilation of tracepoint expressions is not implemented.

gdb/ChangeLog:

	* arm-tdep.c (arm_gdbarch_init) : Install fast tracepoint
	callbacks.

gdb/gdbserver/ChangeLog:

	* linux-arm-low.c (arm_get_thread_area) : New function.
	(arm_get_min_fast_tracepoint_insn_len) : New function.
	(arm_supports_tracepoints) : New function.
	(struct linux_target_ops) : Add fast tracepoint callbacks.

Signed-off-by: Henrik Wallin <henrik.wallin@windriver.com>
---
 gdb/arm-tdep.c                |  5 +++++
 gdb/gdbserver/linux-arm-low.c | 30 ++++++++++++++++++++++++++++++
 2 files changed, 35 insertions(+)

diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
index 74c58eb91c4d..1c34d1a4715b 100644
--- a/gdb/arm-tdep.c
+++ b/gdb/arm-tdep.c
@@ -11156,6 +11156,11 @@ arm_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
     user_reg_add (gdbarch, arm_register_aliases[i].name,
 		  value_of_arm_user_reg, &arm_register_aliases[i].regnum);
 
+  set_gdbarch_fast_tracepoint_valid_at (gdbarch,
+					arm_fast_tracepoint_valid_at);
+
+  set_gdbarch_relocate_instruction (gdbarch, arm_relocate_instruction);
+
   return gdbarch;
 }
 
diff --git a/gdb/gdbserver/linux-arm-low.c b/gdb/gdbserver/linux-arm-low.c
index 6bb069fe71bb..c3f85960bb28 100644
--- a/gdb/gdbserver/linux-arm-low.c
+++ b/gdb/gdbserver/linux-arm-low.c
@@ -918,6 +918,30 @@ arm_regs_info (void)
 }
 
 static int
+arm_get_thread_area (int lwpid, CORE_ADDR *addr)
+{
+  uint32_t val;
+
+  if (ptrace (PTRACE_GET_THREAD_AREA, lwpid, NULL, &val) != 0)
+    return -1;
+
+  *addr = val;
+  return 0;
+}
+
+static int
+arm_get_min_fast_tracepoint_insn_len(void)
+{
+  return 4;
+}
+
+static int
+arm_supports_tracepoints(void)
+{
+  return 1;
+}
+
+static int
 is_target_arm (CORE_ADDR to, CORE_ADDR from)
 {
   CORE_ADDR ptr = to;
@@ -1324,6 +1348,12 @@ struct linux_target_ops the_low_target = {
   arm_new_thread,
   arm_new_fork,
   arm_prepare_to_resume,
+  NULL,                         /* process_qsupported */
+  arm_supports_tracepoints,     /* supports_tracepoints */
+  arm_get_thread_area,          /* get_thread_area */
+  arm_install_fast_tracepoint_jump_pad, /* install_fast_tracepoint_jump_pad */
+  NULL,                         /* emit_ops */
+  arm_get_min_fast_tracepoint_insn_len, /* get_min_fast_tracepoint_insn_len */
 };
 
 void
-- 
2.1.4

[RFC][PATCH 11/15] gdbserver: Add helper functions to create arm instructions

[henrik.wallin]

From: Henrik Wallin <henrik.wallin@windriver.com>

This adds the function. No users yet.

gdb/gdbserver/ChangeLog:

	* linux-arm-low.c : Add code generation helpers

Signed-off-by: Henrik Wallin <henrik.wallin@windriver.com>
---
 gdb/gdbserver/linux-arm-low.c | 135 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 135 insertions(+)

diff --git a/gdb/gdbserver/linux-arm-low.c b/gdb/gdbserver/linux-arm-low.c
index 81d23d953434..044e7527a3b0 100644
--- a/gdb/gdbserver/linux-arm-low.c
+++ b/gdb/gdbserver/linux-arm-low.c
@@ -20,6 +20,7 @@
 #include "linux-low.h"
 #include "arch/arm.h"
 #include "linux-aarch32-low.h"
+#include <inttypes.h>
 
 #include <sys/uio.h>
 /* Don't include elf.h if linux/elf.h got included by gdb_proc_service.h.
@@ -916,6 +917,140 @@ arm_regs_info (void)
     return &regs_info_arm;
 }
 
+static uint32_t
+mk_t_b_rel (CORE_ADDR from, CORE_ADDR to)
+{
+  uint32_t from_ = ((uint32_t) from) & ~1;
+  uint32_t to_   = ((uint32_t) to) & ~1;
+  return to_ - from_ - 4;
+}
+
+static int
+mk_t_b_isreachable (CORE_ADDR from, CORE_ADDR to)
+{
+  int32_t rel = mk_t_b_rel (from, to);
+  rel >>= 24;
+  return !rel || !(rel + 1);
+}
+
+static uint16_t *
+mk_t_b_instr (uint16_t *mem, CORE_ADDR from, CORE_ADDR to)
+{
+  uint32_t imm10, imm11;
+  uint32_t s, j1, j2;
+  uint32_t rel;
+
+  rel = mk_t_b_rel (from, to);
+  rel >>= 1;
+
+  imm11 = rel & 0x7ff;
+  rel >>= 11;
+  imm10 = rel & 0x3ff;
+  rel >>= 10;
+  s  = (rel > 3);
+  j1 = s ^ !(rel & 2);
+  j2 = s ^ !(rel & 1);
+
+  *mem++ = 0xF000 | (s << 10) | imm10;
+  *mem++ = 0x9000 | (j1 << 13) | (j2 << 11) | imm11;
+
+  return mem;
+}
+
+static uint16_t *
+mk_t_blx_instr (uint16_t *mem, int reg)
+{
+  reg &= 0xF;
+  *mem++ = 0x4780 | (reg << 3);
+  return mem;
+}
+
+static uint16_t *
+mk_t_load_instr (uint16_t *mem, int reg, uint32_t val)
+{
+  uint32_t imm4, imm3, imm8;
+  uint32_t i;
+
+  imm8 = val & 0x00FF;
+  val >>= 8;
+  imm3 = val & 0x0007;
+  val >>= 3;
+  i = val & 0x0001;
+  val >>= 1;
+  imm4 = val & 0x000F;
+  val >>= 4;
+
+  *mem++ = 0xF240 | (i << 10) | imm4;
+  *mem++ = 0x0000 | (imm3 << 12) | (reg << 8) | imm8;
+
+  imm8 = val & 0x00FF;
+  val >>= 8;
+  imm3 = val & 0x0007;
+  val >>= 3;
+  i = val & 0x0001;
+  val >>= 1;
+  imm4 = val & 0x000F;
+
+  *mem++ = 0xF2C0 | (i << 10) | imm4;
+  *mem++ = 0x0000 | (imm3 << 12) | (reg << 8) | imm8;
+
+  return mem;
+}
+
+static uint32_t
+mk_a_b_rel (CORE_ADDR from, CORE_ADDR to)
+{
+  return (uint32_t) to - (uint32_t) from - 8;
+}
+
+static int
+mk_a_b_isreachable (CORE_ADDR from, CORE_ADDR to)
+{
+  int32_t rel = mk_a_b_rel (from, to);
+  rel >>= 25;
+  return !rel || !(rel + 1);
+}
+
+static uint32_t *
+mk_a_b_instr (uint32_t *mem, CORE_ADDR from, CORE_ADDR to)
+{
+  uint32_t imm24 = mk_a_b_rel (from, to);
+
+  imm24 >>= 2;
+  imm24 &= 0x00FFFFFF;
+  *mem++ = 0xEA000000 | imm24;
+
+  return mem;
+}
+
+static uint32_t *
+mk_a_blx_instr (uint32_t *mem, int reg)
+{
+  *mem++ = 0xE12FFF30 | (reg & 0xF);
+  return mem;
+}
+
+static uint32_t *
+mk_a_load_instr (uint32_t *mem, int reg, uint32_t val)
+{
+  uint32_t imm4, imm12;
+
+  imm12 = val & 0x0FFF;
+  val >>= 12;
+  imm4 = val & 0xF;
+  val >>= 4;
+
+  *mem++ = 0xE3000000 | ((reg & 0xF) << 12) | (imm4 << 16) | imm12;
+
+  imm12 = val & 0x0FFF;
+  val >>= 12;
+  imm4 = val & 0xF;
+
+  *mem++ = 0xE3400000 | ((reg & 0xF) << 12) | (imm4 << 16) | imm12;
+
+  return mem;
+}
+
 struct linux_target_ops the_low_target = {
   arm_arch_setup,
   arm_regs_info,
-- 
2.1.4

[RFC][PATCH 14/15] gdbserver: Add linux-arm-ipa.c

[henrik.wallin]

From: Henrik Wallin <henrik.wallin@windriver.com>

This adds the ipa for arm.

Currently hard-code to "arm_with_neon".
This probably needs to be improved to validate the arm architecture
that actually is running.

The ipa does not support jit compilation of the fast tracepoint expressions.

gdb/gdbserver/ChangeLog:

	* configure.srv : Add IPA for arm
	* Makefile.in : Add IPA for arm
	* linux-arm-ipa.c : New file

Signed-off-by: Henrik Wallin <henrik.wallin@windriver.com>
---
 gdb/gdbserver/Makefile.in     |  6 +++
 gdb/gdbserver/configure.srv   |  1 +
 gdb/gdbserver/linux-arm-ipa.c | 97 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 104 insertions(+)
 create mode 100644 gdb/gdbserver/linux-arm-ipa.c

diff --git a/gdb/gdbserver/Makefile.in b/gdb/gdbserver/Makefile.in
index cd146f4abd20..8b57072b59a1 100644
--- a/gdb/gdbserver/Makefile.in
+++ b/gdb/gdbserver/Makefile.in
@@ -517,6 +517,12 @@ rsp-low-ipa.o: ../common/rsp-low.c
 errors-ipa.o: ../common/errors.c
 	$(IPAGENT_COMPILE) $<
 	$(POSTCOMPILE)
+arm-linux-ipa.o: arm-with-neon.c
+	$(IPAGENT_COMPILE) $<
+	$(POSTCOMPILE)
+linux-arm-ipa.o: linux-arm-ipa.c
+	$(IPAGENT_COMPILE) $<
+	$(POSTCOMPILE)
 
 ax.o: ax.c
 	$(COMPILE) $(WARN_CFLAGS_NO_FORMAT) $<
diff --git a/gdb/gdbserver/configure.srv b/gdb/gdbserver/configure.srv
index f187c9de0e65..93fe855b93d9 100644
--- a/gdb/gdbserver/configure.srv
+++ b/gdb/gdbserver/configure.srv
@@ -81,6 +81,7 @@ case "${target}" in
 			srv_linux_usrregs=yes
 			srv_linux_regsets=yes
 			srv_linux_thread_db=yes
+			ipa_obj="arm-linux-ipa.o linux-arm-ipa.o"
 			;;
   arm*-*-mingw32ce*)	srv_regobj=reg-arm.o
 			srv_tgtobj="win32-low.o win32-arm-low.o"
diff --git a/gdb/gdbserver/linux-arm-ipa.c b/gdb/gdbserver/linux-arm-ipa.c
new file mode 100644
index 000000000000..e14465b94423
--- /dev/null
+++ b/gdb/gdbserver/linux-arm-ipa.c
@@ -0,0 +1,97 @@
+/* GNU/Linux/arm specific low level interface, for the in-process
+   agent library for GDB.
+
+   Copyright (C) 2015 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include "server.h"
+#include <stdint.h>
+#include <sys/mman.h>
+#include "tracepoint.h"
+
+/* Defined in auto-generated file regs-arm.c.  */
+void init_registers_arm_with_neon (void);
+extern const struct target_desc *tdesc_arm_with_neon;
+
+/* reg map
+ */
+int index_map[] = {
+  2,				/*  0 - r0 */
+  3,				/*  1 - r1 */
+  4,				/*  2 - r2 */
+  5,				/*  3 - r3 */
+  6,				/*  4 - r4 */
+  7,				/*  5 - r5 */
+  8,				/*  6 - r6 */
+  9,				/*  7 - r7 */
+  10,				/*  8 - r8 */
+  11,				/*  9 - r9 */
+  12,				/* 10 - r10 */
+  13,				/* 11 - r11 */
+  14,				/* 12 - r12 */
+  -1,				/* 13 - r13 - sp */
+  15,				/* 14 - r14 - lr */
+  0,				/* 15 - r15 - pc */
+  -1,				/* 16 - */
+  -1,				/* 17 - */
+  -1,				/* 18 - */
+  -1,				/* 19 - */
+  -1,				/* 20 - */
+  -1,				/* 21 - */
+  -1,				/* 22 - */
+  -1,				/* 23 - */
+  -1,				/* 24 - */
+  1,				/* 25 - cpsr */
+};
+
+void
+supply_fast_tracepoint_registers (struct regcache *regcache,
+				  const unsigned char *buf)
+{
+  const uint32_t *regs = (const uint32_t *) buf;
+  uint32_t val;
+  int i;
+
+  for (i = 0; i < sizeof(index_map) / sizeof(index_map[0]); i++)
+    {
+      int index = index_map[i];
+      if (index != -1)
+	{
+	  val = regs[index];
+	  supply_register (regcache, i, &val);
+	}
+    }
+  /* special for sp   */
+  val = (uint32_t) regs + 16 * 4;
+  supply_register (regcache, 13, &val);
+}
+
+IP_AGENT_EXPORT_FUNC ULONGEST
+gdb_agent_get_raw_reg (const unsigned char *raw_regs, int regnum)
+{
+  /* only for jit  */
+  return 0;
+}
+
+const char *gdbserver_xmltarget;
+
+void
+initialize_low_tracepoint (void)
+{
+  init_registers_arm_with_neon ();
+  ipa_tdesc = tdesc_arm_with_neon;
+}
-- 
2.1.4

Re: [RFC][PATCH 14/15] gdbserver: Add linux-arm-ipa.c

[Yao Qi]

henrik.wallin@windriver.com writes:

> Currently hard-code to "arm_with_neon".
> This probably needs to be improved to validate the arm architecture
> that actually is running.

You can tell the arm architecture by checking HWCAP.  See
ppc64_host_hwcap in nat/ppc-linux.c about how to get HWCAP, and see
linux-arm-low.c:arm_read_description about how to select target
description according to the HWCAP.

-- 
Yao (齐尧)

[RFC][PATCH 13/15] gdbserver: Add arm_install_fast_tracepoint_jump_pad

[henrik.wallin]

From: Henrik Wallin <henrik.wallin@windriver.com>

This adds the function. No users yet.

Installs the jump pad. It handles arm and thumb mode.
It will fail if it turns out that the distance to jump is too far away,
or if the relocation done by the gdb side returns "not possible".

gdb/gdbserver/ChangeLog:

	* linux-arm-low.c (append_insns) : New function.
	(copy_instruction) : New function.
	(arm_install_fast_tracepoint_jump_pad): New function.

Signed-off-by: Henrik Wallin <henrik.wallin@windriver.com>
---
 gdb/gdbserver/linux-arm-low.c | 229 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 229 insertions(+)

diff --git a/gdb/gdbserver/linux-arm-low.c b/gdb/gdbserver/linux-arm-low.c
index 1853e79bc140..6bb069fe71bb 100644
--- a/gdb/gdbserver/linux-arm-low.c
+++ b/gdb/gdbserver/linux-arm-low.c
@@ -926,6 +926,21 @@ is_target_arm (CORE_ADDR to, CORE_ADDR from)
   return (to == ptr) ? 0 : 1;
 }
 
+static void
+append_insns (CORE_ADDR *to, size_t len, const unsigned char *buf)
+{
+  write_inferior_memory (*to, buf, len);
+  *to += len;
+}
+
+static int
+copy_instruction (CORE_ADDR *to, CORE_ADDR from, size_t len)
+{
+  CORE_ADDR before = *to;
+  relocate_instruction (to, from);
+  return (before == *to) ? -1 : 1;
+}
+
 static uint32_t
 mk_t_b_rel (CORE_ADDR from, CORE_ADDR to)
 {
@@ -1060,6 +1075,220 @@ mk_a_load_instr (uint32_t *mem, int reg, uint32_t val)
   return mem;
 }
 
+static int
+arm_install_fast_tracepoint_jump_pad (CORE_ADDR tpoint,
+				      CORE_ADDR tpaddr,
+				      CORE_ADDR collector,
+				      CORE_ADDR lockaddr,
+				      ULONGEST orig_size,
+				      CORE_ADDR *jump_entry,
+				      CORE_ADDR *trampoline,
+				      ULONGEST *trampoline_size,
+				      unsigned char *jjump_pad_insn,
+				      ULONGEST *jjump_pad_insn_size,
+				      CORE_ADDR *adjusted_insn_addr,
+				      CORE_ADDR *adjusted_insn_addr_end,
+				      char *err)
+{
+  unsigned char buf[0x100];
+  CORE_ADDR buildaddr = *jump_entry;
+
+  /* Register usage:
+     r0 - arg1:tpoint   /   tmp
+     r1 - arg2:sp
+     r2 - collector
+     r3 - tmp
+     r4 - lockaddr
+     r5 - saved sp      /   collector_t   */
+
+  if (is_target_arm(buildaddr, tpaddr))	/* arm mode  */
+    {
+      uint32_t *ptr = (uint32_t *) buf;
+
+      *ptr++ = 0xe92d5fff;            /* push { r0-r12,lr }  */
+      *ptr++ = 0xe10f0000;            /* mrs r0,cpsr  */
+      *ptr++ = 0xe52d0004;            /* push r0  */
+
+      ptr = mk_a_load_instr (ptr, 0, (uint32_t) tpaddr);
+      *ptr++ = 0xe52d0004;            /* push r0 (orig pc)  */
+      *ptr++ = 0xe1a0100d;            /* mov r1, sp (regs:arg2)  */
+
+      ptr = mk_a_load_instr (ptr, 0, 0xffff0fe0);
+      ptr = mk_a_blx_instr (ptr, 0);
+      *ptr++ = 0xe52d0004;            /* push r0 (tls)  */
+
+      ptr = mk_a_load_instr (ptr, 0, (uint32_t) tpoint);
+      *ptr++ = 0xe52d0004;            /* push r0 (tpoint:arg1)  */
+      ptr = mk_a_load_instr (ptr, 2, (uint32_t) collector);
+      ptr = mk_a_load_instr (ptr, 4, (uint32_t) lockaddr);
+
+      *ptr++ = 0xe1a0500d;            /*    mov r5, sp  */
+      *ptr++ = 0xf57ff05f;            /* 1: dmb sy  */
+      *ptr++ = 0xe1943f9f;            /* 2: ldrex r3, [r4]  */
+      *ptr++ = 0xe3530000;            /*    cmp r3, #0  */
+      *ptr++ = 0x1a000002;            /*    bne 3  */
+      *ptr++ = 0xe184ef95;            /*    strex r14, r5, [r4]  */
+      *ptr++ = 0xe35e0000;            /*    cmp r14, #0  */
+      *ptr++ = 0x1afffff9;            /*    bne 2  */
+      *ptr++ = 0xf57ff05f;            /* 3: dmb sy  */
+      *ptr++ = 0x1afffff6;            /*    bne 1  */
+
+      *ptr++ = 0xe3c53007;            /* bic r3, r5, 7  */
+      *ptr++ = 0xe1a0d003;            /* mov sp, r3  */
+      ptr = mk_a_blx_instr (ptr, 2);
+      *ptr++ = 0xe1a0d005;            /* mov sp, r5  */
+
+      *ptr++ = 0xe3a03000;            /* mov r3, #0  */
+      *ptr++ = 0xe5843000;            /* str r3, [r4]  */
+
+      *ptr++ = 0xe28dd00c;            /* add sp, sp, #12  */
+      *ptr++ = 0xe49d0004;            /* pop r0  */
+      *ptr++ = 0xe12cf000;            /* msr cpsr,r0  */
+      *ptr++ = 0xe8bd5fff;            /* pop { r0-r12,lr }  */
+
+      append_insns (&buildaddr, (uint32_t) ptr - (uint32_t) buf, buf);
+
+      *adjusted_insn_addr = buildaddr;
+      if (copy_instruction (&buildaddr, tpaddr, 4) < 0)
+	{
+	  strcpy (err, "E.Cannot move instruction to jump_pad. "
+		  "Not possible to relocate.");
+	  return 1;
+	}
+      *adjusted_insn_addr_end = buildaddr;
+
+      /* Possible improvements:
+	 This branch can be made non-relative:
+	 B <mem location>:
+	 push    {r0,r1}
+	 movw    r0, #<mem location>
+	 movt    r0, #<mem location>
+	 str     r0, [sp, #4]
+	 pop     {r0,pc}  */
+      if (!mk_a_b_isreachable (buildaddr, tpaddr + 4))
+	{
+	  strcpy (err, "E.Cannot construct valid branch "
+		  "instruction for fast tracepoint."
+		  " Too long relative branch.");
+	  return 1;
+	}
+      /* b <tp_addr + 4>  */
+      (void) mk_a_b_instr ((uint32_t *) buf, buildaddr, tpaddr + 4);
+      append_insns (&buildaddr, 4, buf);
+
+      /* write tp instr.  */
+      if (!mk_a_b_isreachable (tpaddr, *jump_entry))
+	{
+	  strcpy (err, "E.Cannot construct valid branch "
+		  "instruction for fast tracepoint."
+		  " Too long relative branch.");
+	  return 1;
+	}
+      (void) mk_a_b_instr ((uint32_t *) jjump_pad_insn, tpaddr, *jump_entry);
+      *jjump_pad_insn_size = 4;
+      *jump_entry = buildaddr;
+    }
+  else                        /* thumb mode  */
+    {
+      uint16_t *ptr = (uint16_t *) buf;
+
+      *ptr++ = 0xe92d;            /* push { r0-r12,lr }  */
+      *ptr++ = 0x5fff;
+      *ptr++ = 0xf3ef;            /* mrs r0,cpsr  */
+      *ptr++ = 0x8000;
+      *ptr++ = 0xb401;            /* push r0  */
+
+      ptr = mk_t_load_instr (ptr, 0, (uint32_t) tpaddr);
+      *ptr++ = 0xb401;            /* push r0 (orig pc)  */
+      *ptr++ = 0x4669;            /* mov r1, sp (regs:arg2)  */
+
+      ptr = mk_t_load_instr (ptr, 0, 0xffff0fe0);
+      ptr = mk_t_blx_instr (ptr, 0);
+      *ptr++ = 0xb401;		/* push r0 (tls)  */
+
+      ptr = mk_t_load_instr (ptr, 0, (uint32_t) tpoint);
+      *ptr++ = 0xb401;		/* push r0 (tpoint:arg1)  */
+      ptr = mk_t_load_instr (ptr, 2, (uint32_t) collector);
+      ptr = mk_t_load_instr (ptr, 4, (uint32_t) lockaddr);
+
+      *ptr++ = 0x466d;		/*    mov r5, sp  */
+      *ptr++ = 0xf3bf;		/* 1: dmb sy  */
+      *ptr++ = 0x8f5f;
+      *ptr++ = 0xe854;		/* 2: ldrex   r3, [r4]	*/
+      *ptr++ = 0x3f00;
+      *ptr++ = 0x2b00;		/*    cmp     r3, #0  */
+      *ptr++ = 0xd104;		/*    bne.n   3	 */
+      *ptr++ = 0xe844;		/*    strex   r14, r5, [r4]  */
+      *ptr++ = 0x5e00;
+      *ptr++ = 0xf1be;		/*    cmp.w   r14, #0  */
+      *ptr++ = 0x0f00;
+      *ptr++ = 0xd1f6;		/*    bne.n   2	 */
+      *ptr++ = 0xf3bf;		/* 3. dmb  sy  */
+      *ptr++ = 0x8f5f;
+      *ptr++ = 0xd1f1;		/*    bne.n   1	 */
+
+      *ptr++ = 0xf025;		/* bic r3, r5, 7  */
+      *ptr++ = 0x0307;
+      *ptr++ = 0x469d;		/* mov sp, r3  */
+      ptr = mk_t_blx_instr (ptr, 2);
+      *ptr++ = 0x46ad;		/* mov sp, r5  */
+
+      *ptr++ = 0xf04f;		/* mov r3, #0  */
+      *ptr++ = 0x0300;
+      *ptr++ = 0x6023;		/* str r3, [r4]	 */
+
+      *ptr++ = 0xb003;		/* add sp, #12	*/
+      *ptr++ = 0xbc01;		/* pop r0  */
+      *ptr++ = 0xf380;		/* msr cpsr,r0	*/
+      *ptr++ = 0x8c00;
+      *ptr++ = 0xe8bd;		/* pop { r0-r12,lr }  */
+      *ptr++ = 0x5fff;
+
+      append_insns (&buildaddr, (uint32_t) ptr - (uint32_t) buf, buf);
+
+      *adjusted_insn_addr = buildaddr;
+      if (copy_instruction (&buildaddr, tpaddr, 4) < 0)
+	{
+	  strcpy (err, "E.Cannot move instruction to jump_pad."
+		  " Not possible to relocate.");
+	  return 1;
+	}
+      *adjusted_insn_addr_end = buildaddr;
+
+      /* Possible improvements:
+	 This branch can be made non-relative:
+	 B <mem location>:
+	 push	   {r0,r1}
+	 movw	   r0, #<mem location>
+	 movt	   r0, #<mem location>
+	 str	   r0, [sp, #4]
+	 pop	   {r0,pc}  */
+      if (!mk_t_b_isreachable (buildaddr, tpaddr + 4))
+	{
+	  strcpy (err, "E.Cannot construct valid branch "
+		  "instruction for fast tracepoint."
+		  "Too long relative branch.");
+	  return 1;
+	}
+      (void) mk_t_b_instr ((uint16_t *) buf, buildaddr, tpaddr + 4);
+      append_insns (&buildaddr, 4, buf);
+
+      /* write tp instr.	*/
+      if (!mk_t_b_isreachable (tpaddr, *jump_entry))
+	{
+	  strcpy (err, "E.Cannot construct valid branch "
+		  "instruction for fast tracepoint."
+		  "Too long relative branch.");
+	  return 1;
+	}
+      (void) mk_t_b_instr ((uint16_t *) jjump_pad_insn, tpaddr, *jump_entry);
+      *jjump_pad_insn_size = 4;
+      *jump_entry = buildaddr;
+    }
+
+  return 0;
+}
+
 struct linux_target_ops the_low_target = {
   arm_arch_setup,
   arm_regs_info,
-- 
2.1.4

[RFC][PATCH 09/15] gdb: Add relocate instruction helpers

[henrik.wallin]

From: Henrik Wallin <henrik.wallin@windriver.com>

This adds the functions. No users yet.

The functions are used both when validating an instruction
when the users sets a fast tracepoint and when relocating
an instruction when gdbserver/ipa installs the jump pad.

Currently all PC relative instructions are considered
not relocatable.

Futher improvements can be made by rewriting some of those
instructions with alternative instructions.

gdb/ChangeLog:

	* arm-tdep.c : Add relocate functionality to be used by fast
	tracepoint support.

Signed-off-by: Henrik Wallin <henrik.wallin@windriver.com>
---
 gdb/arm-tdep.c | 599 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 599 insertions(+)

diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
index 4c99ddfe89cb..b277c3ef405c 100644
--- a/gdb/arm-tdep.c
+++ b/gdb/arm-tdep.c
@@ -9907,6 +9907,605 @@ arm_register_g_packet_guesses (struct gdbarch *gdbarch)
   /* Otherwise we don't have a useful guess.  */
 }
 
+/* Local structure to pass information in and out of the relocate
+   helper functions  */
+struct relocate_insn
+{
+  struct gdbarch *gdbarch;
+  CORE_ADDR *to;
+  CORE_ADDR oldloc;
+  enum bfd_endian byte_order_for_code;
+  int result; /* 0: copy unmodif, >0: handled, <0: not possible  */
+};
+
+static void
+append_insns (CORE_ADDR *to, ULONGEST len, uint32_t insn,
+	      enum bfd_endian byte_order)
+{
+  write_memory_unsigned_integer (*to, len, byte_order, insn);
+  *to += len;
+}
+
+static void
+arm_relocate_insn_arm (struct relocate_insn *rel, uint32_t insn)
+{
+  unsigned int cond = bits (insn, 28, 31);
+  unsigned int op   = (bits (insn, 25, 27) << 1) | bit (insn, 4);
+
+  /* 1111 ---- ---- ---- ---- ---- ---- ---- :
+     unconditional instructions  */
+  if (cond == 15)
+    {
+      /* 1111 0--- ---- ---- ---- ---- ---- ---- :
+	 Memory hints, Advanced SIMD instructions, and
+	 miscellaneous instructions  */
+      if (bit (insn, 27) == 0)
+	{
+	  unsigned int op1 = bits (insn, 20, 26);
+	  unsigned int op2 = bits (insn, 4, 7);
+	  unsigned int rn = bits (insn, 16, 19);
+
+	  /* All variants we are interested in have rn == 15 - check first  */
+	  if (rn == 15)
+	    {
+	      /* 1111 0100 x101 1111 ---- ---- ---- ---- :
+		 PLI (literal)  */
+	      if ((op1 & 0x77) == 0x45)
+		rel->result = -1;
+	      /* 1111 0101 x101 1111 ---- ---- ---- ---- :
+		 PLD (literal)  */
+	      else if ((op1 & 0x77) == 0x55)
+		rel->result = -1;
+	      /* 1111 0110 x101 1111 ---- ---- ---0 ---- :
+		 PLI (register)  */
+	      else if ((op1 & 0x77) == 0x65 && (op2 & 0x1) == 0x0)
+		rel->result = -1;
+	      /* 1111 0111 xx01 1111 ---- ---- ---0 ---- :
+		 PLD, PLDW (register)  */
+	      else if ((op1 & 0x73) == 0x71 && (op2 & 0x1) == 0x0)
+		rel->result = -1;
+	    }
+	}
+      /* 1111 101- ---- ---- ---- ---- ---- ---- :
+	 BL, BLX (immediate)  */
+      else if (bits (insn, 25, 27) == 5)
+	rel->result = -1;
+      /* 1111 110x xxxx ---- ---- ---- ---- ---- :
+	 STC/STC2, LDC/LDC2 ( x != 00x0x)  */
+      else if (bits (insn, 25, 27) == 6 &&
+	       (bits (insn, 20, 24) & 0x1A) != 0x0)
+	{
+	  unsigned int rn = bits (insn, 16, 19);
+	  if (rn == 15)
+	    rel->result = -1;
+	}
+    }
+
+  /* ---- 00x- ---- ---- ---- ---- ---x ---- :
+     Data-processing and miscellaneous instructions  */
+  else if (op <= 3)
+    {
+      unsigned int op1 = bits (insn, 20, 24);
+      unsigned int op2 = bits (insn, 4, 7);
+
+      if (bit (insn, 25) == 0)
+	{
+	  /* ---- 000x xxxx ---- ---- ---- xxx0 ---- :
+	     Data-processing (register) ( x != 10xx0 )  */
+	  if ((op1 & 0x19) != 0x10 && (op2 & 0x1) == 0x0) {
+	    unsigned int rm = bits (insn, 0, 3);
+	    unsigned int rn = bits (insn, 16, 19);
+
+	    /* ---- 000x xxxx nnnn ---- ---- xxx0 mmmm :
+	       AND,EOR,SUB,RSB,ADD,ADC,SBC,RSC,TST,TEQ,
+	       CMP,CMN,ORR,MOV,LSL,LSR,ASR,RRX,ROR,BIC,MVN  */
+	    if (rn == 15 || rm == 15)
+	      rel->result = -1;
+	  }
+	  /* ---- 000x xxxx ---- ---- ---- xxx1 ---- :
+	     Data-processing (register-shifted register)  */
+	  else if ((op1 & 0x19) != 0x10 && (op2 & 0x9) == 0x1)
+	    {
+	      ;
+	    }
+
+	  /* ---- 0001 0xx0 ---- ---- ---- 0xxx ---- :
+	     Miscellaneous instructions  */
+	  else if ((op1 & 0x19) == 0x10 && (op2 & 0x8) == 0x0)
+	    {
+	      /* ---- 0001 0010 ---- ---- ---- 0011 ---- :
+		 BLX (register)  */
+	      if (bits (insn, 4, 6) == 3 && bits (insn, 21, 22) == 1) /* BLX  */
+		rel->result = -1;
+	    }
+	  /* ---- 0001 0xx0 ---- ---- ---- 1xx0 ---- :
+	     Halfword multiply and multiply accumulate  */
+	  else if ((op1 & 0x19) == 0x10 && (op2 & 0x9) == 0x8)
+	    {
+	      ;
+	    }
+	  /* ---- 0000 xxxx ---- ---- ---- 1001 ---- :
+	     Multiply and multiply accumulate  */
+	  else if ((op1 & 0x10) == 0x00 && op2 == 0x9)
+	    {
+	      ;
+	    }
+	  /* ---- 0001 xxxx ---- ---- ---- 1001 ---- :
+	     Synchronization primitives  */
+	  else if ((op1 & 0x10) == 0x10 && op2 == 0x9)
+	    {
+	      ;
+	    }
+	  /* ---- 000x xxxx ---- ---- ---- 1xx1 ---- :
+	     Extra load/store instructions  */
+	  else if (op2 == 0xB || (op2 & 0xd) == 0xd)
+	    {
+	      unsigned int rn = bits (insn, 16, 19);
+	      /* ---- 000x xxxx 1111 ---- ---- 1xx1 ---- :
+		 STRH, LDRH, LDRD, LDRSB  */
+	      if (rn == 15)
+		rel->result = -1;
+	    }
+	}
+      else
+	{
+	  /* ---- 001x xxxx ---- ---- ---- ---- ---- :
+	     Data-processing (immediate)  */
+	  if ((op1 & 0x19) != 0x10)
+	    {
+	      unsigned int op  = bits (insn, 21, 24);
+	      unsigned int op2 = bits (insn, 20, 24);
+	      unsigned int rn = bits (insn, 16, 19);
+
+	      if (rn == 15) /* Only check those with n == 15  */
+		{
+		  /* ---- 0010 xxxx 1111 ---- ---- ---- ---- :
+		     AND, EOR, ADR, RSB, ADR, ADC, SBC, RSC  */
+		  if (op <= 7)
+		    rel->result = -1;
+		  /* ---- 0011 0xx1 1111 ---- ---- ---- ---- :
+		     TST, TEQ, CMP, CMN  */
+		  else if ((op2 & 0x19) == 0x11)
+		    rel->result = -1;
+		  /* ---- 0011 1x0x 1111 ---- ---- ---- ---- :
+		     ORR, BIC  */
+		  else if (op == 0xC || op == 0xE)
+		    rel->result = -1;
+		  /* ---- 0011 1x1x 1111 ---- ---- ---- ---- :
+		     MOV, MVN  */
+		  else if (op == 0xD || op == 0xF)
+		    {
+		      ;
+		    }
+		}
+	    }
+	}
+    }
+
+  /* ---- 01A- ---- ---- ---- ---- ---B ---- :
+     Load/store word and unsigned byte  */
+  else if (op <= 6)
+    {
+      unsigned int a = bit (insn, 25);
+      unsigned int rt = bits (insn, 12, 15);
+      unsigned int rn = bits (insn, 16, 19);
+      unsigned int op1 = bits (insn, 20, 24);
+      unsigned int op1_m1 = (op1 & 0x17);
+      unsigned int op1_m2 = (op1 & 0x5);
+
+      /* a and b can not both be 1 - no need to test for b  */
+
+      /* ---- 01Ax xxxx nnnn ---- ---- ---B ---- :
+	 STR (immediate), STR (register)  */
+      if (op1_m2 == 0x00 && op1_m1 != 0x02)
+	{
+	  if (rt == 15 || rn == 15)
+	    rel->result = -1;
+	}
+      /* ---- 01Ax xxxx nnnn ---- ---- ---B ---- :
+	 STRT  */
+      else if (op1_m1 == 0x02)
+	{
+	  if (rt == 15)
+	    rel->result = -1;
+	}
+      /* ---- 01Ax xxxx nnnn ---- ---- ---B ---- :
+	 LDR (literal)  */
+      else if (op1_m2 == 0x01 && op1_m1 != 0x03)
+	{
+	  if (rn == 15)
+	    rel->result = -1;
+	}
+      /* ---- 01Ax xxxx nnnn ---- ---- ---B ---- :
+	 LDRT  */
+      else if (op1_m1 == 0x03)
+	{
+	  ;
+	}
+      /* ---- 01Ax xxxx nnnn ---- ---- ---B ---- :
+	 STRB (immediate), STRB (register)  */
+      else if (op1_m2 == 0x04 && op1_m1 != 0x06)
+	{
+	  if (rn == 15)
+	    rel->result = -1;
+	}
+      /* ---- 01Ax xxxx nnnn ---- ---- ---B ---- :
+	 STRBT  */
+      else if (op1_m1 == 0x06)
+	{
+	  ;
+	}
+      /* ---- 01Ax xxxx nnnn ---- ---- ---B ---- :
+	 LDRB (immediate), LDRB (register)  */
+      else if (op1_m2 == 0x05 && op1_m1 != 0x07)
+	{
+	  if (rn == 15)
+	    rel->result = -1;
+	}
+      /* ---- 01Ax xxxx nnnn ---- ---- ---B ---- :
+	 LDRBT  */
+      else if (op1_m1 == 0x07)
+	{
+	  ;
+	}
+    }
+
+  /* ---- 011- ---- ---- ---- ---- ---1 ---- :
+     Media instructions  */
+  else if (op <= 7)
+    {
+      ;
+    }
+
+  /* ---- 10x- ---- ---- ---- ---- ---x ---- :
+     Branch, branch with link, and block data transfer  */
+  else if (op <= 11)
+    {
+      unsigned int op1 = bits (insn, 20, 24);
+      unsigned int r = bit (insn, 15);
+      unsigned int rn = bits (insn, 16, 19);
+
+      /* ---- 101x xxxx nnnn r--- ---- ---- ---- :
+	 B, BL, BLX  */
+      if (bit (insn, 25))
+	rel->result = -1;
+      /* ---- 100x xxx0 nnnn r--- ---- ---- ---- :
+	 STMDA, STM, STMDB, STMIB  */
+      else if (bit (insn, 20) == 0)
+	{
+	  if (rn == 15 || r == 1)
+	    rel->result = -1;
+	}
+      /* ---- 100x xxx1 nnnn r--- ---- ---- ---- :
+	 LDMDA, LDM/LDMIALDMFD, LDMDB/LDMEA, LSMIB/LDMED  */
+      else
+	{
+	  ;
+	}
+    }
+
+  /* ---- 11x- ---- ---- ---- ---- ---x ---- :
+     Coprocessor instructions, and Supervisor Call  */
+  else
+    {
+      unsigned int op1 = bits (insn, 20, 25);
+      unsigned int rn = bits (insn, 16, 19);
+      unsigned int coproc = bits (insn, 8, 11);
+      unsigned int op = bit (insn, 4);
+
+      /* ---- 1100 000x nnnn ---- xxxx ---x ---- :
+	 undefined  */
+      /* ---- 1111 xxxx nnnn ---- xxxx ---x ---- :
+	 SVC  */
+      if ((op1 & 0x3E) == 0 || (op1 & 0x30) == 0x30)
+	{
+	  ;
+	}
+      else if ((coproc & 0xE) != 0xA)
+	{
+	  /* ---- 1100 010x nnnn ---- xxxx ---x ---- :
+	     MCRR/MCRR2, MRRC/MRRC2  */
+	  if (op1 == 4 || op1 == 5)
+	    {
+	      ;
+	    }
+	  /* ---- 110x xxxx nnnn ---- xxxx ---x ---- :
+	     STC/STC2, LDC/LDC2  */
+	  else if ((op1 & 0x20) == 0)
+	    {
+	      if (rn == 15)
+		rel->result = -1;
+	    }
+	  /* ---- 1110 xxxx nnnn ---- xxxx ---x ---- :
+	     CDP/CDP2, MCR/MCR2, MRC/MRC2  */
+	  else if ((op1 & 0x30) == 0x20)
+	    {
+	      ;
+	    }
+	}
+      else
+	{
+	  /* ---- 110x xxxx nnnn ---- 101x ---x ---- :
+	     Extension register load/store instructions  */
+	  if ((op1 & 0x20) == 0 && (op1 & 0x3A) != 0)
+	    {
+	      /* ---- 110x xxxx 1111 ---- 101- ---- ---- :
+		 VSTM, VSTR, VLDM, VLDR  */
+	      if (rn == 15)
+		rel->result = -1;
+	    }
+	}
+    }
+
+  if (rel->to && rel->result == 0)
+    {
+      append_insns (rel->to, 4, insn, rel->byte_order_for_code);
+    }
+}
+
+static void
+arm_relocate_insn_thumb16 (struct relocate_insn *rel, uint16_t insn)
+{
+  unsigned short op = bits (insn, 10, 15);
+
+  /* 0100 01-- ---- ---- :
+     Special data instructions and branch and exchange.  */
+  if (op == 0x11)
+    {
+      /* 0100 0111 1x-- ---- :
+	 BLX (register)  */
+      if (bits (insn, 7, 9) == 7)
+	rel->result = -1;
+      /* 0100 01xx xx-- ---- :
+	 ADD/MOV/CMP high registers.  */
+      else if (bits (insn, 6, 7) != 0)
+	{
+	  if (bits (insn, 3, 6) == 15 ||
+	      ((bit (insn, 7) << 3) | bits (insn, 0, 2)) == 15)
+	    rel->result = -1;
+	}
+    }
+  /* 0100 1x-- ---- ---- :
+     LDR (literal)  */
+  else if ((op & 0x3E) == 0x12)
+    rel->result = -1;
+  /* 1010 0x-- ---- ---- :
+     Generate PC-relative address (ADR)  */
+  else if ((op & 0x3E) == 0x28)
+    rel->result = -1;
+  /* 1011 xx-- ---- ---- :
+     Misc 16-bit instructions  */
+  else if ((op & 0x3C) == 0x2C)
+    {
+      unsigned short op2 = bits (insn, 8, 11);
+      /* 1011 x0x1 ---- ---- :
+	 CBNZ, CBZ  */
+      if ((op2 & 0x5) == 0x1)
+	rel->result = -1;
+      /* 1011 1111 ---- xxxx :
+	 IT  */
+      else if (op2 == 0xF && bits (insn, 0, 3) != 0)
+	rel->result = -1;
+    }
+  /* 1101 xx-- ---- ---- :
+     Conditional branch and supervisor calls  */
+  else if ((op & 0x3C) == 0x34) /*   */
+    {
+      /* 1101 xxxx ---- ---- :
+	 B (conditional)  */
+      if (bits (insn, 9, 11) != 7)
+	rel->result = -1;
+    }
+  /* 1110 0x-- ---- ---- :
+     B (unconditional)  */
+  else if ((op & 0x3E) == 0x38)
+    rel->result = -1;
+
+  if (rel->to && rel->result == 0)
+    append_insns (rel->to, 2, insn, rel->byte_order_for_code);
+}
+
+static void
+arm_relocate_insn_thumb32 (struct relocate_insn *rel,
+			   uint16_t insn1, uint16_t insn2)
+{
+  unsigned int op1 = bits (insn1, 11, 12);
+  unsigned int op2 = bits (insn1, 4, 10);
+  unsigned short op = bit (insn2, 15);
+
+  if (op1 == 1)
+    {
+      /* 1110 100x x1xx ---- x--- ---- ---- ---- :
+	 Load/store dual, load/store excl, table branch  */
+      if ((op2 & 0x64) == 0x04) /*   */
+	{
+	  unsigned int op1 = bits (insn1, 7, 8);
+	  unsigned int op2 = bits (insn1, 4, 5);
+	  /* 1110 1000 1101 ---- ---- ---- 000- ---- :
+	     TBB, TBH  */
+	  if (op1 == 1 && op2 == 1 && bits (insn2, 5, 7) == 0)
+	    rel->result = -1;
+	  /* 1110 1000 x111 1111 ---- ---- ---- ---- :
+	     LDRD (literal)  */
+	  /* 1110 1001 x1x1 1111 ---- ---- ---- ---- :
+	     LDRD (literal)  */
+	  else if (bits (insn1, 0, 3) == 15)
+	    {
+	      if (((op1 & 0x2) == 0 && op2 == 3) ||
+		  ((op1 & 0x2) == 2 && (op2 & 1) == 1))
+		rel->result = -1;
+	    }
+	}
+      /* 1110 11xx xxxx ---- x--- ---- ---- ---- :
+	 Coprocessor, Advanced SIMD, and Floating-point instructions  */
+      else if ((op2 & 0x40) == 0x40)
+	{
+	  unsigned int op3 = bits (insn1, 4, 9);
+	  unsigned int coproc = bits (insn2, 9, 11);
+	  /* 1110 1101 xx01 nnnn ---- 101x ---x ---- :
+	     Extension register load/store instructions / VLDR  */
+	  if (coproc == 5 && (op3 & 0x33) == 0x11)
+	    rel->result = -1;
+	  /* 1110 110x xxx1 nnnn ---- xxxx ---x ---- :
+	     LDC/LDC2 (literal)  */
+	  else if (coproc != 5 && (op3 & 0x21) == 1 &&
+		   (op3 & 0x3A) != 0 && bits (insn1, 0, 3) == 0xF)
+	    rel->result = -1;
+	}
+    }
+
+  else if (op1 == 2)
+    {
+      /* 1111 0xxx xxxx ---- 1--- ---- ---- ---- :
+	 Branches and miscellaneous control  */
+      if (op)
+	{
+	  /* 1111 0xxx xxxx ---- 11xx xxxx ---- ---- :
+	     BL/BLX  */
+	  if (bit (insn2, 14))
+	    rel->result = -1;
+	  /* 1111 0xxx xxxx ---- 10x1 xxxx ---- ---- :
+	     B (unconditional)  */
+	  else if (bit (insn2, 12))
+	    rel->result = -1;
+	  /* 1111 0xxx xxxx ---- 10x0 xxxx ---- ---- :
+	     B (conditional)  */
+	  else if (bits (insn1, 7, 9) != 0x7)
+	    rel->result = -1;
+	}
+      /* 1111 0x1x xxxx ---- 0--- ---- ---- ---- :
+	 Data processing (plain binary immediate)  */
+      else if (bit (insn1, 9))
+	{
+	  int op = bits (insn1, 4, 8);
+	  int rn = bits (insn1, 0, 3);
+	  /* 1111 0x1x x0x0 1111 0--- ---- ---- ---- :
+	     ADR  */
+	  if ((op == 0 || op == 0xa) && rn == 0xf)
+	    rel->result = -1;
+	}
+    }
+
+  else
+    {
+      /* 1111 100x x001 ---- x--- ---- ---- ---- :
+	 Load byte, memory hints  */
+      if ((op2 & 0x67) == 0x1)
+	{
+	  /* 1111 100x x001 nnnn tttt xxxx xx-- ---- :
+	     LDRB (literal), PLD (literal),
+	     LDRSB (literal), PLI (immediate, literal)  */
+	  if (bits (insn1, 0, 3) == 15)
+	    rel->result = -1;
+	}
+      /* 1111 100x x011 ---- x--- ---- ---- ---- :
+	 Load halfword, memory hints  */
+      else if ((op2 & 0x67) == 0x3)
+	{
+	  /* 1111 100x x011 nnnn tttt xxxx xx-- ---- :
+	     LDRH (literal), LDRSH (literal)  */
+	  if (bits (insn1, 0, 3) == 15 && bits (insn2, 12, 15) != 15)
+	    rel->result = -1;
+	}
+      /* 1111 100x x101 ---- x--- ---- ---- ---- :
+	 Load word  */
+      else if ((op2 & 0x67) == 0x5)
+	{
+	  int rt = bits (insn2, 12, 15);
+	  int rn = bits (insn1, 0, 3);
+	  /* 1111 100x x101 1111 ---- xxxx xx-- ---- :
+	     LDR (literal)  */
+	  if (rn == 15)
+	    rel->result = -1;
+	}
+      /* 1111 11xx xxxx ---- x--- ---- ---- ---- :
+	 Coprocessor, Advanced SIMD, and Floating-point instructions  */
+      else if ((op2 & 0x40) == 0x40)
+	{
+	  unsigned int op1_ = bits (insn1, 4, 9);
+
+	  /* 1111 110x xxx1 1111 ---- xxxx ---x ---- :
+	     LDC, LDC2 (literal)  */
+	  if ((bits (insn2, 8, 11) & 0xE) != 0xA &&
+	      (op1_ & 0x21) == 0x01 && (op1_ & 0x3A) != 0 &&
+	      bits (insn1, 0, 3) == 15)
+	    rel->result = -1;
+	}
+    }
+
+  if (rel->to && rel->result == 0)
+    {
+      append_insns (rel->to, 2, insn1, rel->byte_order_for_code);
+      append_insns (rel->to, 2, insn2, rel->byte_order_for_code);
+    }
+}
+
+static void
+arm_relocate_instruction_func (struct relocate_insn *rel)
+{
+  rel->byte_order_for_code = gdbarch_byte_order_for_code (rel->gdbarch);
+  rel->result = 0;
+
+  if (arm_pc_is_thumb (rel->gdbarch, rel->oldloc))
+    {
+      uint16_t insn1;
+      uint16_t insn2;
+
+      insn1 = read_memory_unsigned_integer (rel->oldloc, 2,
+					    rel->byte_order_for_code);
+      if (thumb_insn_size (insn1) == 2)
+	arm_relocate_insn_thumb16 (rel, insn1);
+      else
+	{
+	  insn2 = read_memory_unsigned_integer (rel->oldloc + 2, 2,
+						rel->byte_order_for_code);
+	  arm_relocate_insn_thumb32 (rel, insn1, insn2);
+	}
+    }
+  else
+    {
+      uint32_t insn;
+
+      insn = read_memory_unsigned_integer (rel->oldloc, 4,
+					   rel->byte_order_for_code);
+      arm_relocate_insn_arm (rel, insn);
+    }
+}
+
+static int
+arm_check_relocate_instruction (struct gdbarch *gdbarch,
+				CORE_ADDR oldloc, char **msg)
+{
+  struct relocate_insn rel;
+
+  rel.gdbarch = gdbarch;
+  rel.to = NULL;
+  rel.oldloc = oldloc;
+
+  arm_relocate_instruction_func (&rel);
+
+  if (rel.result == -1)
+    {
+      if (msg)
+	*msg = xstrprintf (_("; instruction cannot be relocated"));
+    }
+
+  return rel.result;
+}
+
+static void
+arm_relocate_instruction (struct gdbarch *gdbarch,
+			  CORE_ADDR *to, CORE_ADDR oldloc)
+{
+  struct relocate_insn rel;
+
+  rel.gdbarch = gdbarch;
+  rel.to = to;
+  rel.oldloc = oldloc;
+
+  arm_relocate_instruction_func (&rel);
+}
+
 \f
 /* Initialize the current architecture based on INFO.  If possible,
    re-use an architecture from ARCHES, which is a list of
-- 
2.1.4

Re: [RFC][PATCH 09/15] gdb: Add relocate instruction helpers

[Yao Qi]

henrik.wallin@windriver.com writes:

> The functions are used both when validating an instruction
> when the users sets a fast tracepoint and when relocating
> an instruction when gdbserver/ipa installs the jump pad.
>
> Currently all PC relative instructions are considered
> not relocatable.
>
> Futher improvements can be made by rewriting some of those
> instructions with alternative instructions.
>
> gdb/ChangeLog:
>
> 	* arm-tdep.c : Add relocate functionality to be used by fast
> 	tracepoint support.

You are relocating instructions in GDB side, which uses qRelocInsn
packet.  Why don't you relocate them in GDBserver side?  Search 
aarch64_relocate_instruction in gdbserver/linux-aarch64-low.c, and you
may have some clues from it.

-- 
Yao (齐尧)

RE: [RFC][PATCH 09/15] gdb: Add relocate instruction helpers

[Wallin, Henrik]

2015-10-27 12:20 GMT+01:00 Yao Qi <qiyaoltc@gmail.com>:
> henrik.wallin@windriver.com writes:
>
>> The functions are used both when validating an instruction
>> when the users sets a fast tracepoint and when relocating
>> an instruction when gdbserver/ipa installs the jump pad.
>>
>> Currently all PC relative instructions are considered
>> not relocatable.
>>
>> Futher improvements can be made by rewriting some of those
>> instructions with alternative instructions.
>>
>> gdb/ChangeLog:
>>
>>       * arm-tdep.c : Add relocate functionality to be used by fast
>>       tracepoint support.
>
> You are relocating instructions in GDB side, which uses qRelocInsn
> packet.  Why don't you relocate them in GDBserver side?  Search
> aarch64_relocate_instruction in gdbserver/linux-aarch64-low.c, and you
> may have some clues from it.

Thanks for looking at the patches. I will check arm64 code and see if it can be moved to gdbserver side.

thanks,
/ Henrik

Re: [RFC][PATCH 09/15] gdb: Add relocate instruction helpers

[Simon Marchi]

On 15-10-27 07:20 AM, Yao Qi wrote:
> henrik.wallin@windriver.com writes:
> 
>> The functions are used both when validating an instruction
>> when the users sets a fast tracepoint and when relocating
>> an instruction when gdbserver/ipa installs the jump pad.
>>
>> Currently all PC relative instructions are considered
>> not relocatable.
>>
>> Futher improvements can be made by rewriting some of those
>> instructions with alternative instructions.
>>
>> gdb/ChangeLog:
>>
>> 	* arm-tdep.c : Add relocate functionality to be used by fast
>> 	tracepoint support.
> 
> You are relocating instructions in GDB side, which uses qRelocInsn
> packet.  Why don't you relocate them in GDBserver side?  Search 
> aarch64_relocate_instruction in gdbserver/linux-aarch64-low.c, and you
> may have some clues from it.
> 

Hi Yao,

It seems like the code that Henrik adds in this patch (for relocating an
instruction) is very similar to the code currently in arm-tdep.c for
displaced stepping.  I think we can extract it and place it in arch/,
then use it both for displaced stepping and fast tracepoints.  That's
what aarch64 does currently, is that right?  Do you think it's the
right way to go for arm as well?

Simon

Re: [RFC][PATCH 09/15] gdb: Add relocate instruction helpers

[Yao Qi]

Simon Marchi <simon.marchi@ericsson.com> writes:

> It seems like the code that Henrik adds in this patch (for relocating an
> instruction) is very similar to the code currently in arm-tdep.c for
> displaced stepping.  I think we can extract it and place it in arch/,
> then use it both for displaced stepping and fast tracepoints.  That's
> what aarch64 does currently, is that right?  Do you think it's the
> right way to go for arm as well?

Yes, I think the instruction decoding part in arm for instruction
relocation (for both fast tracepoint and displaced stepping) can be
shared.  The possibility of code sharing was considered at the beginning
of aarch64 work.  However, this possibility wasn't considered, AFICS,
when the arm displaced stepping was added, so I am afraid we need to
refactor, if not rewrite, the existing arm displaced stepping code a lot
for the purpose of sharing code.  It must take much effort.

On the other hand, GDBserver can't move threads out of jump pad without
hardware single step, which is a showstopper to ARM linux fast
tracepoint support.

In short, it is right to share code, but we need to figure out how to
move threads out of jump pad on ARM linux first.  Secondly, rewrite arm
displaced stepping code, and share decoding part with fast tracepoint
instruction relocation.  Unfortunately, the first one is still an open
question to me.

-- 
Yao (齐尧)

Re: [RFC][PATCH 09/15] gdb: Add relocate instruction helpers

[Simon Marchi]

On 15-11-23 06:00 AM, Yao Qi wrote:
> Simon Marchi <simon.marchi@ericsson.com> writes:
> 
>> It seems like the code that Henrik adds in this patch (for relocating an
>> instruction) is very similar to the code currently in arm-tdep.c for
>> displaced stepping.  I think we can extract it and place it in arch/,
>> then use it both for displaced stepping and fast tracepoints.  That's
>> what aarch64 does currently, is that right?  Do you think it's the
>> right way to go for arm as well?
> 
> Yes, I think the instruction decoding part in arm for instruction
> relocation (for both fast tracepoint and displaced stepping) can be
> shared.  The possibility of code sharing was considered at the beginning
> of aarch64 work.  However, this possibility wasn't considered, AFICS,
> when the arm displaced stepping was added, so I am afraid we need to
> refactor, if not rewrite, the existing arm displaced stepping code a lot
> for the purpose of sharing code.  It must take much effort.

Ok, but if that's what it takes to get fast tracepoint support on ARM, we will
consider it.  It's certainly better than having a parallel implementation of
instruction decoding/relocation gdbserver-side.

> On the other hand, GDBserver can't move threads out of jump pad without
> hardware single step, which is a showstopper to ARM linux fast
> tracepoint support.

Ok, I see why that is important.  However, why is hardware single step
necessary?  With software single-step support added into gdbserver (thanks to
Antoine), isn't it possible to single step a thread out of the jump pad using
that?

> In short, it is right to share code, but we need to figure out how to
> move threads out of jump pad on ARM linux first.  Secondly, rewrite arm
> displaced stepping code, and share decoding part with fast tracepoint
> instruction relocation.  Unfortunately, the first one is still an open
> question to me.

If this problem can't be fixed directly, Antoine and I thought that we could
still have fast tracepoints support, but with known issues and some limitations,
such as not being able to instrument signal handlers, only allowing one tstart
for the life of the process (to avoid problems mentioned in stabilize_threads).

Could that be a good plan B?

Simon

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

[Yao Qi]

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 (齐尧)

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

[Wallin, Henrik]

2015-10-27 12:13 GMT+01:00 Yao Qi <qiyaoltc@gmail.com>:
> henrik.wallin@windriver.com writes:
>
> Hi Henrik,
> First of all, thanks for your patches...

Thanks for taking a look at this.

>> 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.

I will work on the first patches some more regarding testcases.

>
>> 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?

I guess I'm not sure how gdb identifies different ARM architecture.
I want to be sure that this only are run on those ARMs that it works on.
(ARMv7). 

>
>>
>> - 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;
>     }

So, isn't there a check somewhere to protect setting breakpoints inside the jump pad? Or do I remember wrong.
Or does this limitation comes into play at other times?

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

No idea. I guess this is related to what Antoine works on. 

thanks,
/ Henrik

>
> --
> Yao (齐尧)