Further fix "value is not available" with debug frame

Message ID 20240302072611.30526-1-tdevries@suse.de
State Committed
Headers
Series Further fix "value is not available" with debug frame |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_gdb_build--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gdb_check--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 success Testing passed

Commit Message

Tom de Vries March 2, 2024, 7:26 a.m. UTC
  In commit 2aaba744467 ("[gdb] Fix "value is not available" with debug frame")
I fixed a case in frame_unwind_register_value where using "set debug frame on"
caused an "info frame" command to abort, reporting a "value is not available"
error, due to the tpidruro register being unavailable.

Subsequently, commit bbb12eb9c84 ("gdb/arm: Remove tpidruro register from
non-FreeBSD target descriptions") removed the unavailable register, which
caused a progression on test-case gdb.base/inline-frame-cycle-unwind.exp.

While investigating the progression (see PR python/31437), I found that the
"debug frame" output of the test-case (when reverting commit bbb12eb9c84)
showed a smilar problem:
...
Python Exception <class 'gdb.error'>: value is not available^M
...
that was absent without "debug frame".

Fix this likewise in fetch_lazy_register, and update the test-case to check
for the exception.

Furthermore, I realized that there's both value::entirely_available and
value::entirely_unavailable, and that commit 2aaba744467 handled the case
of !entirely_available by printing unavailable.

Instead, print:
- "unavailable" for entirely_unavailable, and
- "partly unavailable" for !entirely_unavailable && !entirely_available.

Tested on x86_64-linux and arm-linux.
---
 gdb/frame.c                                   |  8 ++++---
 .../gdb.base/inline-frame-cycle-unwind.exp    |  7 +++++-
 gdb/value.c                                   | 23 ++++++++++++-------
 3 files changed, 26 insertions(+), 12 deletions(-)


base-commit: 5c97cb1c80994462632ba7c62a54a3fbecaff2ca
  

Comments

Tom de Vries March 19, 2024, 9:31 a.m. UTC | #1
On 3/2/24 08:26, Tom de Vries wrote:
> In commit 2aaba744467 ("[gdb] Fix "value is not available" with debug frame")
> I fixed a case in frame_unwind_register_value where using "set debug frame on"
> caused an "info frame" command to abort, reporting a "value is not available"
> error, due to the tpidruro register being unavailable.
> 
> Subsequently, commit bbb12eb9c84 ("gdb/arm: Remove tpidruro register from
> non-FreeBSD target descriptions") removed the unavailable register, which
> caused a progression on test-case gdb.base/inline-frame-cycle-unwind.exp.
> 
> While investigating the progression (see PR python/31437), I found that the
> "debug frame" output of the test-case (when reverting commit bbb12eb9c84)
> showed a smilar problem:
> ...
> Python Exception <class 'gdb.error'>: value is not available^M
> ...
> that was absent without "debug frame".
> 
> Fix this likewise in fetch_lazy_register, and update the test-case to check
> for the exception.
> 
> Furthermore, I realized that there's both value::entirely_available and
> value::entirely_unavailable, and that commit 2aaba744467 handled the case
> of !entirely_available by printing unavailable.
> 
> Instead, print:
> - "unavailable" for entirely_unavailable, and
> - "partly unavailable" for !entirely_unavailable && !entirely_available.
> 

Pushed.

Thanks,
- Tom

> Tested on x86_64-linux and arm-linux.
> ---
>   gdb/frame.c                                   |  8 ++++---
>   .../gdb.base/inline-frame-cycle-unwind.exp    |  7 +++++-
>   gdb/value.c                                   | 23 ++++++++++++-------
>   3 files changed, 26 insertions(+), 12 deletions(-)
> 
> diff --git a/gdb/frame.c b/gdb/frame.c
> index 5c7aae9edf4..c56655c55c1 100644
> --- a/gdb/frame.c
> +++ b/gdb/frame.c
> @@ -1315,9 +1315,7 @@ frame_unwind_register_value (const frame_info_ptr &next_frame, int regnum)
>   
>   	  if (value->lazy ())
>   	    gdb_printf (&debug_file, " lazy");
> -	  else if (!value->entirely_available ())
> -	    gdb_printf (&debug_file, " unavailable");
> -	  else
> +	  else if (value->entirely_available ())
>   	    {
>   	      int i;
>   	      gdb::array_view<const gdb_byte> buf = value->contents ();
> @@ -1328,6 +1326,10 @@ frame_unwind_register_value (const frame_info_ptr &next_frame, int regnum)
>   		gdb_printf (&debug_file, "%02x", buf[i]);
>   	      gdb_printf (&debug_file, "]");
>   	    }
> +	  else if (value->entirely_unavailable ())
> +	    gdb_printf (&debug_file, " unavailable");
> +	  else
> +	    gdb_printf (&debug_file, " partly unavailable");
>   	}
>   
>         frame_debug_printf ("%s", debug_file.c_str ());
> diff --git a/gdb/testsuite/gdb.base/inline-frame-cycle-unwind.exp b/gdb/testsuite/gdb.base/inline-frame-cycle-unwind.exp
> index 82632757f13..85e3cc32f0e 100644
> --- a/gdb/testsuite/gdb.base/inline-frame-cycle-unwind.exp
> +++ b/gdb/testsuite/gdb.base/inline-frame-cycle-unwind.exp
> @@ -131,9 +131,14 @@ with_test_prefix "cycle at level 1" {
>   gdb_test "maint flush register-cache" \
>       "Register cache flushed\\." ""
>   gdb_test_no_output "set debug frame 1"
> +set ok 1
>   gdb_test_multiple "bt" "backtrace with debugging on" {
>       -re "^$gdb_prompt $" {
> -	pass $gdb_test_name
> +	gdb_assert { $ok } $gdb_test_name
> +    }
> +    -re "Python Exception <class 'gdb.error'>: \[^\r\n\]*\r\n" {
> +	set ok 0
> +	exp_continue
>       }
>       -re "\[^\r\n\]+\r\n" {
>   	exp_continue
> diff --git a/gdb/value.c b/gdb/value.c
> index a2b2721d183..7ddfeb7c728 100644
> --- a/gdb/value.c
> +++ b/gdb/value.c
> @@ -4014,9 +4014,6 @@ value::fetch_lazy_register ()
>   	}
>         else
>   	{
> -	  int i;
> -	  gdb::array_view<const gdb_byte> buf = new_val->contents ();
> -
>   	  if (new_val->lval () == lval_register)
>   	    gdb_printf (&debug_file, " register=%d", new_val->regnum ());
>   	  else if (new_val->lval () == lval_memory)
> @@ -4026,11 +4023,21 @@ value::fetch_lazy_register ()
>   	  else
>   	    gdb_printf (&debug_file, " computed");
>   
> -	  gdb_printf (&debug_file, " bytes=");
> -	  gdb_printf (&debug_file, "[");
> -	  for (i = 0; i < register_size (gdbarch, regnum); i++)
> -	    gdb_printf (&debug_file, "%02x", buf[i]);
> -	  gdb_printf (&debug_file, "]");
> +	  if (new_val->entirely_available ())
> +	    {
> +	      int i;
> +	      gdb::array_view<const gdb_byte> buf = new_val->contents ();
> +
> +	      gdb_printf (&debug_file, " bytes=");
> +	      gdb_printf (&debug_file, "[");
> +	      for (i = 0; i < register_size (gdbarch, regnum); i++)
> +		gdb_printf (&debug_file, "%02x", buf[i]);
> +	      gdb_printf (&debug_file, "]");
> +	    }
> +	  else if (new_val->entirely_unavailable ())
> +	    gdb_printf (&debug_file, " unavailable");
> +	  else
> +	    gdb_printf (&debug_file, " partly unavailable");
>   	}
>   
>         frame_debug_printf ("%s", debug_file.c_str ());
> 
> base-commit: 5c97cb1c80994462632ba7c62a54a3fbecaff2ca
  

Patch

diff --git a/gdb/frame.c b/gdb/frame.c
index 5c7aae9edf4..c56655c55c1 100644
--- a/gdb/frame.c
+++ b/gdb/frame.c
@@ -1315,9 +1315,7 @@  frame_unwind_register_value (const frame_info_ptr &next_frame, int regnum)
 
 	  if (value->lazy ())
 	    gdb_printf (&debug_file, " lazy");
-	  else if (!value->entirely_available ())
-	    gdb_printf (&debug_file, " unavailable");
-	  else
+	  else if (value->entirely_available ())
 	    {
 	      int i;
 	      gdb::array_view<const gdb_byte> buf = value->contents ();
@@ -1328,6 +1326,10 @@  frame_unwind_register_value (const frame_info_ptr &next_frame, int regnum)
 		gdb_printf (&debug_file, "%02x", buf[i]);
 	      gdb_printf (&debug_file, "]");
 	    }
+	  else if (value->entirely_unavailable ())
+	    gdb_printf (&debug_file, " unavailable");
+	  else
+	    gdb_printf (&debug_file, " partly unavailable");
 	}
 
       frame_debug_printf ("%s", debug_file.c_str ());
diff --git a/gdb/testsuite/gdb.base/inline-frame-cycle-unwind.exp b/gdb/testsuite/gdb.base/inline-frame-cycle-unwind.exp
index 82632757f13..85e3cc32f0e 100644
--- a/gdb/testsuite/gdb.base/inline-frame-cycle-unwind.exp
+++ b/gdb/testsuite/gdb.base/inline-frame-cycle-unwind.exp
@@ -131,9 +131,14 @@  with_test_prefix "cycle at level 1" {
 gdb_test "maint flush register-cache" \
     "Register cache flushed\\." ""
 gdb_test_no_output "set debug frame 1"
+set ok 1
 gdb_test_multiple "bt" "backtrace with debugging on" {
     -re "^$gdb_prompt $" {
-	pass $gdb_test_name
+	gdb_assert { $ok } $gdb_test_name
+    }
+    -re "Python Exception <class 'gdb.error'>: \[^\r\n\]*\r\n" {
+	set ok 0
+	exp_continue
     }
     -re "\[^\r\n\]+\r\n" {
 	exp_continue
diff --git a/gdb/value.c b/gdb/value.c
index a2b2721d183..7ddfeb7c728 100644
--- a/gdb/value.c
+++ b/gdb/value.c
@@ -4014,9 +4014,6 @@  value::fetch_lazy_register ()
 	}
       else
 	{
-	  int i;
-	  gdb::array_view<const gdb_byte> buf = new_val->contents ();
-
 	  if (new_val->lval () == lval_register)
 	    gdb_printf (&debug_file, " register=%d", new_val->regnum ());
 	  else if (new_val->lval () == lval_memory)
@@ -4026,11 +4023,21 @@  value::fetch_lazy_register ()
 	  else
 	    gdb_printf (&debug_file, " computed");
 
-	  gdb_printf (&debug_file, " bytes=");
-	  gdb_printf (&debug_file, "[");
-	  for (i = 0; i < register_size (gdbarch, regnum); i++)
-	    gdb_printf (&debug_file, "%02x", buf[i]);
-	  gdb_printf (&debug_file, "]");
+	  if (new_val->entirely_available ())
+	    {
+	      int i;
+	      gdb::array_view<const gdb_byte> buf = new_val->contents ();
+
+	      gdb_printf (&debug_file, " bytes=");
+	      gdb_printf (&debug_file, "[");
+	      for (i = 0; i < register_size (gdbarch, regnum); i++)
+		gdb_printf (&debug_file, "%02x", buf[i]);
+	      gdb_printf (&debug_file, "]");
+	    }
+	  else if (new_val->entirely_unavailable ())
+	    gdb_printf (&debug_file, " unavailable");
+	  else
+	    gdb_printf (&debug_file, " partly unavailable");
 	}
 
       frame_debug_printf ("%s", debug_file.c_str ());