gdb: fix wrong buffer size passed to jit_unwind_reg_get_impl

Message ID 20250112210051.1545556-1-jan.vrany@labware.com
State New
Headers
Series gdb: fix wrong buffer size passed to jit_unwind_reg_get_impl |

Checks

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

Commit Message

Jan Vrany Jan. 12, 2025, 9 p.m. UTC
  While testing other patches I realized that commit 7fcdec025 ("GDB: Use
gdb::array_view for buffers used in register reading and unwinding")
broke gdb.base/jit-reader.exp.

What has happened is that the aforementioned commit changed parameter
type from plain gdb_byte * to gdb::array_view<gdb_byte> in number of
functions, including deprecated_frame_register_read(). It did not
however update jit_unwind_reg_get_impl() which reads:

    static struct gdb_reg_value *
    jit_unwind_reg_get_impl (struct gdb_unwind_callbacks *cb, int regnum)
    {
      ...
      value = ((struct gdb_reg_value *)
	       xmalloc (sizeof (struct gdb_reg_value) + size - 1));
      value->defined = deprecated_frame_register_read (priv->this_frame, gdb_reg,
						       value->value);
      ...
    }

The "value" member of struct gdb_reg_value is declared as
unsigned char value[1] but in fact is used as flexible array (see
the xmalloc allocation above).

After the change gdb_byte * to gdb::array_view<gdb_byte> the code still
compiles just fine but passes down an array_view of length 1 (using
conversion constructor).

This then fails later down the road in frame_register_unwind():

    gdb_assert (buffer.size () >= value->type ()->length ());

This commit fixes this problem by explicitly passing down an array_view
with correct size.

Tested on x86_64-linux-gnu.
---
 gdb/jit.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
  

Comments

Simon Marchi Jan. 13, 2025, 4:46 a.m. UTC | #1
On 2025-01-12 16:00, Jan Vrany wrote:
> While testing other patches I realized that commit 7fcdec025 ("GDB: Use
> gdb::array_view for buffers used in register reading and unwinding")
> broke gdb.base/jit-reader.exp.
> 
> What has happened is that the aforementioned commit changed parameter
> type from plain gdb_byte * to gdb::array_view<gdb_byte> in number of
> functions, including deprecated_frame_register_read(). It did not
> however update jit_unwind_reg_get_impl() which reads:
> 
>     static struct gdb_reg_value *
>     jit_unwind_reg_get_impl (struct gdb_unwind_callbacks *cb, int regnum)
>     {
>       ...
>       value = ((struct gdb_reg_value *)
> 	       xmalloc (sizeof (struct gdb_reg_value) + size - 1));
>       value->defined = deprecated_frame_register_read (priv->this_frame, gdb_reg,
> 						       value->value);
>       ...
>     }
> 
> The "value" member of struct gdb_reg_value is declared as
> unsigned char value[1] but in fact is used as flexible array (see
> the xmalloc allocation above).
> 
> After the change gdb_byte * to gdb::array_view<gdb_byte> the code still
> compiles just fine but passes down an array_view of length 1 (using
> conversion constructor).
> 
> This then fails later down the road in frame_register_unwind():
> 
>     gdb_assert (buffer.size () >= value->type ()->length ());
> 
> This commit fixes this problem by explicitly passing down an array_view
> with correct size.
> 
> Tested on x86_64-linux-gnu.
> ---
>  gdb/jit.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/gdb/jit.c b/gdb/jit.c
> index 4b9400ab2f8..d8cc94c1843 100644
> --- a/gdb/jit.c
> +++ b/gdb/jit.c
> @@ -985,8 +985,9 @@ jit_unwind_reg_get_impl (struct gdb_unwind_callbacks *cb, int regnum)
>    size = register_size (frame_arch, gdb_reg);
>    value = ((struct gdb_reg_value *)
>  	   xmalloc (sizeof (struct gdb_reg_value) + size - 1));
> -  value->defined = deprecated_frame_register_read (priv->this_frame, gdb_reg,
> -						   value->value);
> +  value->defined = deprecated_frame_register_read
> +			(priv->this_frame, gdb_reg,
> +			 gdb::array_view<gdb_byte> (value->value, size));
>    value->size = size;
>    value->free = reg_value_free_impl;
>    return value;

I already had sent a patch here, which I have now pushed:

  https://inbox.sourceware.org/gdb-patches/95536d3b-33c3-43d2-a570-845bd6ad634e@polymtl.ca/T/#m9646f23ad72c721da8da114c83ea04df4e72b254

The end result is very similar to yours.

Thanks,

Simon
  

Patch

diff --git a/gdb/jit.c b/gdb/jit.c
index 4b9400ab2f8..d8cc94c1843 100644
--- a/gdb/jit.c
+++ b/gdb/jit.c
@@ -985,8 +985,9 @@  jit_unwind_reg_get_impl (struct gdb_unwind_callbacks *cb, int regnum)
   size = register_size (frame_arch, gdb_reg);
   value = ((struct gdb_reg_value *)
 	   xmalloc (sizeof (struct gdb_reg_value) + size - 1));
-  value->defined = deprecated_frame_register_read (priv->this_frame, gdb_reg,
-						   value->value);
+  value->defined = deprecated_frame_register_read
+			(priv->this_frame, gdb_reg,
+			 gdb::array_view<gdb_byte> (value->value, size));
   value->size = size;
   value->free = reg_value_free_impl;
   return value;