[2/2,AArch64] Add more debugging output to aarch64_displaced_step_fixup

Message ID 20191230162535.21211-2-luis.machado@linaro.org
State New, archived
Headers

Commit Message

Luis Machado Dec. 30, 2019, 4:25 p.m. UTC
  While debugging the step-over-syscall problem, i wanted to see a bit more
debugging output to try to determine the root cause.

This patch does this.

gdb/ChangeLog:

2019-12-30  Luis Machado  <luis.machado@linaro.org>

	* aarch64-tdep.c (aarch64_displaced_step_fixup): Add more debugging
	output.

Signed-off-by: Luis Machado <luis.machado@linaro.org>
---
 gdb/aarch64-tdep.c | 24 +++++++++++++++++++++++-
 1 file changed, 23 insertions(+), 1 deletion(-)
  

Comments

Luis Machado Dec. 30, 2019, 4:50 p.m. UTC | #1
cc-ing Alan.

On 12/30/19 1:25 PM, Luis Machado wrote:
> While debugging the step-over-syscall problem, i wanted to see a bit more
> debugging output to try to determine the root cause.
> 
> This patch does this.
> 
> gdb/ChangeLog:
> 
> 2019-12-30  Luis Machado  <luis.machado@linaro.org>
> 
> 	* aarch64-tdep.c (aarch64_displaced_step_fixup): Add more debugging
> 	output.
> 
> Signed-off-by: Luis Machado <luis.machado@linaro.org>
> ---
>   gdb/aarch64-tdep.c | 24 +++++++++++++++++++++++-
>   1 file changed, 23 insertions(+), 1 deletion(-)
> 
> diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
> index a639b753cd..2d6c5a4bcb 100644
> --- a/gdb/aarch64-tdep.c
> +++ b/gdb/aarch64-tdep.c
> @@ -3058,8 +3058,16 @@ aarch64_displaced_step_fixup (struct gdbarch *gdbarch,
>     /* Fetch the current PC, after the displaced execution took place.  */
>     regcache_cooked_read_unsigned (regs, AARCH64_PC_REGNUM, &pc);
>   
> +  if (debug_displaced)
> +    debug_printf ("displaced: PC after stepping: %s (was %s).\n",
> +		  paddress (gdbarch, pc), paddress (gdbarch, to));
> +
>     if (dsc->cond)
>       {
> +      if (debug_displaced)
> +	debug_printf ("displaced: [Conditional] pc_adjust before: %d\n",
> +		      dsc->pc_adjust);
> +
>         if (pc - to == 8)
>   	{
>   	  /* Condition is true.  */
> @@ -3072,8 +3080,17 @@ aarch64_displaced_step_fixup (struct gdbarch *gdbarch,
>   	}
>         else
>   	gdb_assert_not_reached ("Unexpected PC value after displaced stepping");
> +
> +      if (debug_displaced)
> +	debug_printf ("displaced: [Conditional] pc_adjust after: %d\n",
> +		      dsc->pc_adjust);
>       }
>   
> +  if (debug_displaced)
> +    debug_printf ("displaced: %s PC by %d\n",
> +		  dsc->should_adjust_pc? "adjusting" : "not adjusting",
> +		  dsc->pc_adjust);
> +
>     if (dsc->should_adjust_pc)
>       {
>   
> @@ -3082,7 +3099,12 @@ aarch64_displaced_step_fixup (struct gdbarch *gdbarch,
>   	 and the instruction after the syscall instruction.  Make sure we don't
>   	 adjust the PC when we did not really move.  */
>         if ((pc - to) == 0)
> -	dsc->pc_adjust = 0;
> +	{
> +	  if (debug_displaced)
> +	    debug_printf ("displaced: PC did not move. Discarding PC \
> +adjustment.\n");
> +	  dsc->pc_adjust = 0;
> +	}
>   
>         if (debug_displaced)
>   	{
>
  
Alan Hayward Jan. 9, 2020, 4:14 p.m. UTC | #2
> On 30 Dec 2019, at 16:25, Luis Machado <luis.machado@linaro.org> wrote:

> 

> While debugging the step-over-syscall problem, i wanted to see a bit more

> debugging output to try to determine the root cause.

> 

> This patch does this.

> 

> gdb/ChangeLog:

> 

> 2019-12-30  Luis Machado  <luis.machado@linaro.org>

> 

> 	* aarch64-tdep.c (aarch64_displaced_step_fixup): Add more debugging

> 	output.

> 

> Signed-off-by: Luis Machado <luis.machado@linaro.org>

> ---

> gdb/aarch64-tdep.c | 24 +++++++++++++++++++++++-

> 1 file changed, 23 insertions(+), 1 deletion(-)

> 

> diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c

> index a639b753cd..2d6c5a4bcb 100644

> --- a/gdb/aarch64-tdep.c

> +++ b/gdb/aarch64-tdep.c

> @@ -3058,8 +3058,16 @@ aarch64_displaced_step_fixup (struct gdbarch *gdbarch,

>   /* Fetch the current PC, after the displaced execution took place.  */

>   regcache_cooked_read_unsigned (regs, AARCH64_PC_REGNUM, &pc);

> 

> +  if (debug_displaced)

> +    debug_printf ("displaced: PC after stepping: %s (was %s).\n",

> +		  paddress (gdbarch, pc), paddress (gdbarch, to));


Minor nit: Capital letter for the first word “Displaced”.
Ditto for the rest of the patch.
Otherwise ok.
Once 1/2 is okayed, happy for updated version of this to be pushed without another review.

> +

>   if (dsc->cond)

>     {

> +      if (debug_displaced)

> +	debug_printf ("displaced: [Conditional] pc_adjust before: %d\n",

> +		      dsc->pc_adjust);

> +

>       if (pc - to == 8)

> 	{

> 	  /* Condition is true.  */

> @@ -3072,8 +3080,17 @@ aarch64_displaced_step_fixup (struct gdbarch *gdbarch,

> 	}

>       else

> 	gdb_assert_not_reached ("Unexpected PC value after displaced stepping");

> +

> +      if (debug_displaced)

> +	debug_printf ("displaced: [Conditional] pc_adjust after: %d\n",

> +		      dsc->pc_adjust);

>     }

> 

> +  if (debug_displaced)

> +    debug_printf ("displaced: %s PC by %d\n",

> +		  dsc->should_adjust_pc? "adjusting" : "not adjusting",

> +		  dsc->pc_adjust);

> +

>   if (dsc->should_adjust_pc)

>     {

> 

> @@ -3082,7 +3099,12 @@ aarch64_displaced_step_fixup (struct gdbarch *gdbarch,

> 	 and the instruction after the syscall instruction.  Make sure we don't

> 	 adjust the PC when we did not really move.  */

>       if ((pc - to) == 0)

> -	dsc->pc_adjust = 0;

> +	{

> +	  if (debug_displaced)

> +	    debug_printf ("displaced: PC did not move. Discarding PC \

> +adjustment.\n");

> +	  dsc->pc_adjust = 0;

> +	}

> 

>       if (debug_displaced)

> 	{

> -- 

> 2.17.1

>
  
Luis Machado Jan. 9, 2020, 4:25 p.m. UTC | #3
On 1/9/20 1:14 PM, Alan Hayward wrote:
> 
> 
>> On 30 Dec 2019, at 16:25, Luis Machado <luis.machado@linaro.org> wrote:
>>
>> While debugging the step-over-syscall problem, i wanted to see a bit more
>> debugging output to try to determine the root cause.
>>
>> This patch does this.
>>
>> gdb/ChangeLog:
>>
>> 2019-12-30  Luis Machado  <luis.machado@linaro.org>
>>
>> 	* aarch64-tdep.c (aarch64_displaced_step_fixup): Add more debugging
>> 	output.
>>
>> Signed-off-by: Luis Machado <luis.machado@linaro.org>
>> ---
>> gdb/aarch64-tdep.c | 24 +++++++++++++++++++++++-
>> 1 file changed, 23 insertions(+), 1 deletion(-)
>>
>> diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
>> index a639b753cd..2d6c5a4bcb 100644
>> --- a/gdb/aarch64-tdep.c
>> +++ b/gdb/aarch64-tdep.c
>> @@ -3058,8 +3058,16 @@ aarch64_displaced_step_fixup (struct gdbarch *gdbarch,
>>    /* Fetch the current PC, after the displaced execution took place.  */
>>    regcache_cooked_read_unsigned (regs, AARCH64_PC_REGNUM, &pc);
>>
>> +  if (debug_displaced)
>> +    debug_printf ("displaced: PC after stepping: %s (was %s).\n",
>> +		  paddress (gdbarch, pc), paddress (gdbarch, to));
> 
> Minor nit: Capital letter for the first word “Displaced”.
> Ditto for the rest of the patch.
> Otherwise ok.
> Once 1/2 is okayed, happy for updated version of this to be pushed without another review.
> 

I'll get this fixed. Thanks!
  

Patch

diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
index a639b753cd..2d6c5a4bcb 100644
--- a/gdb/aarch64-tdep.c
+++ b/gdb/aarch64-tdep.c
@@ -3058,8 +3058,16 @@  aarch64_displaced_step_fixup (struct gdbarch *gdbarch,
   /* Fetch the current PC, after the displaced execution took place.  */
   regcache_cooked_read_unsigned (regs, AARCH64_PC_REGNUM, &pc);
 
+  if (debug_displaced)
+    debug_printf ("displaced: PC after stepping: %s (was %s).\n",
+		  paddress (gdbarch, pc), paddress (gdbarch, to));
+
   if (dsc->cond)
     {
+      if (debug_displaced)
+	debug_printf ("displaced: [Conditional] pc_adjust before: %d\n",
+		      dsc->pc_adjust);
+
       if (pc - to == 8)
 	{
 	  /* Condition is true.  */
@@ -3072,8 +3080,17 @@  aarch64_displaced_step_fixup (struct gdbarch *gdbarch,
 	}
       else
 	gdb_assert_not_reached ("Unexpected PC value after displaced stepping");
+
+      if (debug_displaced)
+	debug_printf ("displaced: [Conditional] pc_adjust after: %d\n",
+		      dsc->pc_adjust);
     }
 
+  if (debug_displaced)
+    debug_printf ("displaced: %s PC by %d\n",
+		  dsc->should_adjust_pc? "adjusting" : "not adjusting",
+		  dsc->pc_adjust);
+
   if (dsc->should_adjust_pc)
     {
 
@@ -3082,7 +3099,12 @@  aarch64_displaced_step_fixup (struct gdbarch *gdbarch,
 	 and the instruction after the syscall instruction.  Make sure we don't
 	 adjust the PC when we did not really move.  */
       if ((pc - to) == 0)
-	dsc->pc_adjust = 0;
+	{
+	  if (debug_displaced)
+	    debug_printf ("displaced: PC did not move. Discarding PC \
+adjustment.\n");
+	  dsc->pc_adjust = 0;
+	}
 
       if (debug_displaced)
 	{