diff mbox series

gdb/jit: use correctly-sized array view in deprecated_frame_register_read call

Message ID 20250111044535.292946-1-simon.marchi@polymtl.ca
State New
Headers
Series gdb/jit: use correctly-sized array view in deprecated_frame_register_read call |

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-aarch64 success Test passed
linaro-tcwg-bot/tcwg_gdb_check--master-arm success Test passed

Commit Message

Simon Marchi Jan. 11, 2025, 4:45 a.m. UTC 
  From: GDB Administrator <gdbadmin@sourceware.org>

Commit 7fcdec025c05 ("GDB: Use gdb::array_view for buffers used in
register reading and unwinding") introduces a regression in
gdb.base/jit-reader.exp:

    $ ./gdb -q -nx --data-directory=data-directory testsuite/outputs/gdb.base/jit-reader/jit-reader -ex 'jit-reader-load /home/simark/build/binutils-gdb/gdb/testsuite/outputs/gdb.base/jit-reader/jit-reader.so' -ex r -batch

    This GDB supports auto-downloading debuginfo from the following URLs:
      <https://debuginfod.archlinux.org>
    Enable debuginfod for this session? (y or [n]) [answered N; input not from terminal]
    Debuginfod has been disabled.
    To make this setting permanent, add 'set debuginfod enabled off' to .gdbinit.
    [Thread debugging using libthread_db enabled]
    Using host libthread_db library "/usr/lib/../lib/libthread_db.so.1".

    Program received signal SIGTRAP, Trace/breakpoint trap.
    Recursive internal problem.

The "Recusive internal problem" part is not good, but it's not the point
of this patch.  It still means we hit an internal error.

The stack trace is:

    #0  internal_error_loc (file=0x55555ebefb20 "/home/simark/src/binutils-gdb/gdb/frame.c", line=1207, fmt=0x55555ebef500 "%s: Assertion `%s' failed.") at /home/simark/src/binutils-gdb/gdbsupport/errors.cc:53
    #1  0x0000555561604d83 in frame_register_unwind (next_frame=..., regnum=16, optimizedp=0x7ffff12e5a20, unavailablep=0x7ffff12e5a30, lvalp=0x7ffff12e5a40, addrp=0x7ffff12e5a60, realnump=0x7ffff12e5a50, buffer=...)
        at /home/simark/src/binutils-gdb/gdb/frame.c:1207
    #2  0x0000555561608334 in deprecated_frame_register_read (frame=..., regnum=16, myaddr=...) at /home/simark/src/binutils-gdb/gdb/frame.c:1496
    #3  0x0000555561a74259 in jit_unwind_reg_get_impl (cb=0x7ffff1049ca0, regnum=16) at /home/simark/src/binutils-gdb/gdb/jit.c:988
    #4  0x00007fffd26e634e in read_register (callbacks=0x7ffff1049ca0, dw_reg=16, value=0x7fffffffb4c8) at /home/simark/src/binutils-gdb/gdb/testsuite/gdb.base/jit-reader.c:100
    #5  0x00007fffd26e645f in unwind_frame (self=0x50400000ac10, cbs=0x7ffff1049ca0) at /home/simark/src/binutils-gdb/gdb/testsuite/gdb.base/jit-reader.c:143
    #6  0x0000555561a74a12 in jit_frame_sniffer (self=0x55556374d040 <jit_frame_unwind>, this_frame=..., cache=0x5210002905f8) at /home/simark/src/binutils-gdb/gdb/jit.c:1042
    #7  0x00005555615f499e in frame_unwind_try_unwinder (this_frame=..., this_cache=0x5210002905f8, unwinder=0x55556374d040 <jit_frame_unwind>) at /home/simark/src/binutils-gdb/gdb/frame-unwind.c:138
    #8  0x00005555615f512c in frame_unwind_find_by_frame (this_frame=..., this_cache=0x5210002905f8) at /home/simark/src/binutils-gdb/gdb/frame-unwind.c:209
    #9  0x00005555616178d0 in get_frame_type (frame=...) at /home/simark/src/binutils-gdb/gdb/frame.c:2996
    #10 0x000055556282db03 in do_print_frame_info (uiout=0x511000027500, fp_opts=..., frame=..., print_level=0, print_what=SRC_AND_LOC, print_args=1, set_current_sal=1) at /home/simark/src/binutils-gdb/gdb/stack.c:1033

The problem is that function `jit_unwind_reg_get_impl` passes field
`gdb_reg_value::value`, a gdb_byte array of 1 element (used as a
flexible array member), as the array view parameter of
`deprecated_frame_register_read`.  This results in an array view of size
1.  The assertion in `frame_register_unwind` that verifies the passed in
buffer is larger enough to hold the unwound register value then fails.

Fix this by explicitly creating an array view of the right size.

At the same time, remove the `1` in the array definition, which would
have avoided this bug (it wouldn't have compiled).

Change-Id: Ie170da438ec9085863e7be8b455a067b531635dc
---
 gdb/jit.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)


base-commit: e564115c8a0e8f9b993cf0add01fa34548005d20

Comments

Hannes Domani Jan. 11, 2025, 11:04 a.m. UTC | #1 
Am Samstag, 11. Januar 2025 um 05:46:27 MEZ hat simon.marchi@polymtl.ca <simon.marchi@polymtl.ca> Folgendes geschrieben:

> From: GDB Administrator <gdbadmin@sourceware.org>
>
> Commit 7fcdec025c05 ("GDB: Use gdb::array_view for buffers used in
> register reading and unwinding") introduces a regression in
> gdb.base/jit-reader.exp:
>
>     $ ./gdb -q -nx --data-directory=data-directory testsuite/outputs/gdb.base/jit-reader/jit-reader -ex 'jit-reader-load /home/simark/build/binutils-gdb/gdb/testsuite/outputs/gdb.base/jit-reader/jit-reader.so' -ex r -batch
>
>     This GDB supports auto-downloading debuginfo from the following URLs:
>       <https://debuginfod.archlinux.org>
>     Enable debuginfod for this session? (y or [n]) [answered N; input not from terminal]
>     Debuginfod has been disabled.
>     To make this setting permanent, add 'set debuginfod enabled off' to .gdbinit.
>     [Thread debugging using libthread_db enabled]
>     Using host libthread_db library "/usr/lib/../lib/libthread_db.so.1".
>
>     Program received signal SIGTRAP, Trace/breakpoint trap.
>     Recursive internal problem.
>
> The "Recusive internal problem" part is not good, but it's not the point
> of this patch.  It still means we hit an internal error.
>
> The stack trace is:
>
>     #0  internal_error_loc (file=0x55555ebefb20 "/home/simark/src/binutils-gdb/gdb/frame.c", line=1207, fmt=0x55555ebef500 "%s: Assertion `%s' failed.") at /home/simark/src/binutils-gdb/gdbsupport/errors.cc:53
>     #1  0x0000555561604d83 in frame_register_unwind (next_frame=..., regnum=16, optimizedp=0x7ffff12e5a20, unavailablep=0x7ffff12e5a30, lvalp=0x7ffff12e5a40, addrp=0x7ffff12e5a60, realnump=0x7ffff12e5a50, buffer=...)
>         at /home/simark/src/binutils-gdb/gdb/frame.c:1207
>     #2  0x0000555561608334 in deprecated_frame_register_read (frame=..., regnum=16, myaddr=...) at /home/simark/src/binutils-gdb/gdb/frame.c:1496
>     #3  0x0000555561a74259 in jit_unwind_reg_get_impl (cb=0x7ffff1049ca0, regnum=16) at /home/simark/src/binutils-gdb/gdb/jit.c:988
>     #4  0x00007fffd26e634e in read_register (callbacks=0x7ffff1049ca0, dw_reg=16, value=0x7fffffffb4c8) at /home/simark/src/binutils-gdb/gdb/testsuite/gdb.base/jit-reader.c:100
>     #5  0x00007fffd26e645f in unwind_frame (self=0x50400000ac10, cbs=0x7ffff1049ca0) at /home/simark/src/binutils-gdb/gdb/testsuite/gdb.base/jit-reader.c:143
>     #6  0x0000555561a74a12 in jit_frame_sniffer (self=0x55556374d040 <jit_frame_unwind>, this_frame=..., cache=0x5210002905f8) at /home/simark/src/binutils-gdb/gdb/jit.c:1042
>     #7  0x00005555615f499e in frame_unwind_try_unwinder (this_frame=..., this_cache=0x5210002905f8, unwinder=0x55556374d040 <jit_frame_unwind>) at /home/simark/src/binutils-gdb/gdb/frame-unwind.c:138
>     #8  0x00005555615f512c in frame_unwind_find_by_frame (this_frame=..., this_cache=0x5210002905f8) at /home/simark/src/binutils-gdb/gdb/frame-unwind.c:209
>     #9  0x00005555616178d0 in get_frame_type (frame=...) at /home/simark/src/binutils-gdb/gdb/frame.c:2996
>     #10 0x000055556282db03 in do_print_frame_info (uiout=0x511000027500, fp_opts=..., frame=..., print_level=0, print_what=SRC_AND_LOC, print_args=1, set_current_sal=1) at /home/simark/src/binutils-gdb/gdb/stack.c:1033
>
> The problem is that function `jit_unwind_reg_get_impl` passes field
> `gdb_reg_value::value`, a gdb_byte array of 1 element (used as a
> flexible array member), as the array view parameter of
> `deprecated_frame_register_read`.  This results in an array view of size
> 1.  The assertion in `frame_register_unwind` that verifies the passed in
> buffer is larger enough to hold the unwound register value then fails.
>
> Fix this by explicitly creating an array view of the right size.
>
> At the same time, remove the `1` in the array definition, which would
> have avoided this bug (it wouldn't have compiled).

Is that part missing?


> Change-Id: Ie170da438ec9085863e7be8b455a067b531635dc
> ---
> gdb/jit.c | 9 +++++----
> 1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/gdb/jit.c b/gdb/jit.c
> index 4b9400ab2f8e..822ffb4ffe83 100644
> --- a/gdb/jit.c
> +++ b/gdb/jit.c
> @@ -983,10 +983,11 @@ jit_unwind_reg_get_impl (struct gdb_unwind_callbacks *cb, int regnum)
>
>   gdb_reg = gdbarch_dwarf2_reg_to_regnum (frame_arch, 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 = XNEWVAR (gdb_reg_value, sizeof (gdb_reg_value) + size);
> +  value->defined
> +    = deprecated_frame_register_read (priv->this_frame, gdb_reg,
> +                      gdb::make_array_view (value->value,
> +                                size));
>   value->size = size;
>   value->free = reg_value_free_impl;
>   return value;
>
> base-commit: e564115c8a0e8f9b993cf0add01fa34548005d20
> --
> 2.47.1


Hannes
Simon Marchi Jan. 11, 2025, 8:29 p.m. UTC | #2 
On 2025-01-11 06:04, Hannes Domani wrote:
>  Am Samstag, 11. Januar 2025 um 05:46:27 MEZ hat simon.marchi@polymtl.ca <simon.marchi@polymtl.ca> Folgendes geschrieben:
> 
>> From: GDB Administrator <gdbadmin@sourceware.org>
>>
>> Commit 7fcdec025c05 ("GDB: Use gdb::array_view for buffers used in
>> register reading and unwinding") introduces a regression in
>> gdb.base/jit-reader.exp:
>>
>>      $ ./gdb -q -nx --data-directory=data-directory testsuite/outputs/gdb.base/jit-reader/jit-reader -ex 'jit-reader-load /home/simark/build/binutils-gdb/gdb/testsuite/outputs/gdb.base/jit-reader/jit-reader.so' -ex r -batch
>>
>>      This GDB supports auto-downloading debuginfo from the following URLs:
>>        <https://debuginfod.archlinux.org>
>>      Enable debuginfod for this session? (y or [n]) [answered N; input not from terminal]
>>      Debuginfod has been disabled.
>>      To make this setting permanent, add 'set debuginfod enabled off' to .gdbinit.
>>      [Thread debugging using libthread_db enabled]
>>      Using host libthread_db library "/usr/lib/../lib/libthread_db.so.1".
>>
>>      Program received signal SIGTRAP, Trace/breakpoint trap.
>>      Recursive internal problem.
>>
>> The "Recusive internal problem" part is not good, but it's not the point
>> of this patch.  It still means we hit an internal error.
>>
>> The stack trace is:
>>
>>      #0  internal_error_loc (file=0x55555ebefb20 "/home/simark/src/binutils-gdb/gdb/frame.c", line=1207, fmt=0x55555ebef500 "%s: Assertion `%s' failed.") at /home/simark/src/binutils-gdb/gdbsupport/errors.cc:53
>>      #1  0x0000555561604d83 in frame_register_unwind (next_frame=..., regnum=16, optimizedp=0x7ffff12e5a20, unavailablep=0x7ffff12e5a30, lvalp=0x7ffff12e5a40, addrp=0x7ffff12e5a60, realnump=0x7ffff12e5a50, buffer=...)
>>          at /home/simark/src/binutils-gdb/gdb/frame.c:1207
>>      #2  0x0000555561608334 in deprecated_frame_register_read (frame=..., regnum=16, myaddr=...) at /home/simark/src/binutils-gdb/gdb/frame.c:1496
>>      #3  0x0000555561a74259 in jit_unwind_reg_get_impl (cb=0x7ffff1049ca0, regnum=16) at /home/simark/src/binutils-gdb/gdb/jit.c:988
>>      #4  0x00007fffd26e634e in read_register (callbacks=0x7ffff1049ca0, dw_reg=16, value=0x7fffffffb4c8) at /home/simark/src/binutils-gdb/gdb/testsuite/gdb.base/jit-reader.c:100
>>      #5  0x00007fffd26e645f in unwind_frame (self=0x50400000ac10, cbs=0x7ffff1049ca0) at /home/simark/src/binutils-gdb/gdb/testsuite/gdb.base/jit-reader.c:143
>>      #6  0x0000555561a74a12 in jit_frame_sniffer (self=0x55556374d040 <jit_frame_unwind>, this_frame=..., cache=0x5210002905f8) at /home/simark/src/binutils-gdb/gdb/jit.c:1042
>>      #7  0x00005555615f499e in frame_unwind_try_unwinder (this_frame=..., this_cache=0x5210002905f8, unwinder=0x55556374d040 <jit_frame_unwind>) at /home/simark/src/binutils-gdb/gdb/frame-unwind.c:138
>>      #8  0x00005555615f512c in frame_unwind_find_by_frame (this_frame=..., this_cache=0x5210002905f8) at /home/simark/src/binutils-gdb/gdb/frame-unwind.c:209
>>      #9  0x00005555616178d0 in get_frame_type (frame=...) at /home/simark/src/binutils-gdb/gdb/frame.c:2996
>>      #10 0x000055556282db03 in do_print_frame_info (uiout=0x511000027500, fp_opts=..., frame=..., print_level=0, print_what=SRC_AND_LOC, print_args=1, set_current_sal=1) at /home/simark/src/binutils-gdb/gdb/stack.c:1033
>>
>> The problem is that function `jit_unwind_reg_get_impl` passes field
>> `gdb_reg_value::value`, a gdb_byte array of 1 element (used as a
>> flexible array member), as the array view parameter of
>> `deprecated_frame_register_read`.  This results in an array view of size
>> 1.  The assertion in `frame_register_unwind` that verifies the passed in
>> buffer is larger enough to hold the unwound register value then fails.
>>
>> Fix this by explicitly creating an array view of the right size.
>>
>> At the same time, remove the `1` in the array definition, which would
>> have avoided this bug (it wouldn't have compiled).
> 
> Is that part missing?

Ohhh, it's because it's in jit-reader.h, which is generated, and I
modified the generated file.  I'll send a v2.

Simon
diff mbox series

Patch

diff --git a/gdb/jit.c b/gdb/jit.c
index 4b9400ab2f8e..822ffb4ffe83 100644
--- a/gdb/jit.c
+++ b/gdb/jit.c
@@ -983,10 +983,11 @@  jit_unwind_reg_get_impl (struct gdb_unwind_callbacks *cb, int regnum)
 
   gdb_reg = gdbarch_dwarf2_reg_to_regnum (frame_arch, 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 = XNEWVAR (gdb_reg_value, sizeof (gdb_reg_value) + size);
+  value->defined
+    = deprecated_frame_register_read (priv->this_frame, gdb_reg,
+				      gdb::make_array_view (value->value,
+							    size));
   value->size = size;
   value->free = reg_value_free_impl;
   return value;