[v2] 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-arm |
success
|
Test passed
|
linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 |
fail
|
Test failed
|
Commit Message
From: GDB Administrator <gdbadmin@sourceware.org>
New in v2:
- modify gdb/jit-reader.in, instead of the generated file.
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-reader.in | 2 +-
gdb/jit.c | 9 +++++----
2 files changed, 6 insertions(+), 5 deletions(-)
base-commit: 4d7100dc04c44bee73ca6c45c69f6b20d89a67c8
Comments
Hello Simon,
simon.marchi@polymtl.ca writes:
> From: GDB Administrator <gdbadmin@sourceware.org>
>
> New in v2:
>
> - modify gdb/jit-reader.in, instead of the generated file.
>
> 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-reader.in | 2 +-
> gdb/jit.c | 9 +++++----
> 2 files changed, 6 insertions(+), 5 deletions(-)
Sorry about this regression. I haven't noticed it because jit-reader.exp
only runs on x86, and I tested only on aarch64. I should have tested
x86_64 as well.
Thanks for fixing the problem.
Reviewed-by: Thiago Jung Bauermann <thiago.bauermann@linaro.org>
--
Thiago
On 2025-01-11 18:19, Thiago Jung Bauermann wrote:
>
> Hello Simon,
>
> simon.marchi@polymtl.ca writes:
>
>> From: GDB Administrator <gdbadmin@sourceware.org>
>>
>> New in v2:
>>
>> - modify gdb/jit-reader.in, instead of the generated file.
>>
>> 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).
Actually this last bit is not good. jit-reader.h is a public interface,
and that would change the layout of a public type. I'll remove that
part and push the patch.
>>
>> Change-Id: Ie170da438ec9085863e7be8b455a067b531635dc
>> ---
>> gdb/jit-reader.in | 2 +-
>> gdb/jit.c | 9 +++++----
>> 2 files changed, 6 insertions(+), 5 deletions(-)
>
> Sorry about this regression. I haven't noticed it because jit-reader.exp
> only runs on x86, and I tested only on aarch64. I should have tested
> x86_64 as well.
>
> Thanks for fixing the problem.
>
> Reviewed-by: Thiago Jung Bauermann <thiago.bauermann@linaro.org>
Thanks,
Simon
@@ -208,7 +208,7 @@ struct gdb_reg_value
gdb_reg_value_free *free;
/* The value of the register. */
- unsigned char value[1];
+ unsigned char value[];
};
/* get_frame_id in gdb_reader_funcs is to return a gdb_frame_id
@@ -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;