diff options
author | Simon Marchi <simon.marchi@efficios.com> | 2023-11-07 11:11:18 -0500 |
---|---|---|
committer | Simon Marchi <simon.marchi@efficios.com> | 2023-11-07 11:58:58 -0500 |
commit | 8c3273ee07fae4badfd6f25f67162e5f6ec7f03e (patch) | |
tree | b1ba3ce5e6f5a297241ea8eecd54f947933829d3 /gdb | |
parent | eab996435fe65a421541f59557c5f1fd427573a3 (diff) | |
download | gdb-8c3273ee07fae4badfd6f25f67162e5f6ec7f03e.zip gdb-8c3273ee07fae4badfd6f25f67162e5f6ec7f03e.tar.gz gdb-8c3273ee07fae4badfd6f25f67162e5f6ec7f03e.tar.bz2 |
gdb/arm: remove thumb bit in arm_adjust_breakpoint_address
When compiling gdb with -fsanitize=address on ARM, I get a crash in test
gdb.arch/arm-disp-step.exp, reproduced easily with:
$ ./gdb -nx -q --data-directory=data-directory testsuite/outputs/gdb.arch/arm-disp-step/arm-disp-step -ex "break *test_call_end"
Reading symbols from testsuite/outputs/gdb.arch/arm-disp-step/arm-disp-step...
=================================================================
==23295==ERROR: AddressSanitizer: heap-buffer-overflow on address 0xb4a14fd1 at pc 0x01a48871 bp 0xbeab8490 sp 0xbeab8494
Since it doesn't require running the program, it can be reproduced locally on a
dev machine other than ARM, after acquiring the test binary.
The length of the allocate buffer `buf` is 1, and we try to extract an
integer of size 2 from it. The length of 1 comes from the subtraction
`bpaddr - boundary`. Normally, on ARM, all instructions are aligned on
a multiple of 2, so it's weird for this subtraction to result in 1. In
this case, boundary comes from the result of find_pc_partial_function
returning 0x549:
(gdb) p/x bpaddr
$2 = 0x54a
(gdb) p/x boundary
$3 = 0x549
(gdb) p/x bpaddr - boundary
$4 = 0x1
0x549 is the address of the test_call_subr label, 0x548, with the thumb
bit enabled. Before doing some math with the address, I think we need
to strip the thumb bit, like is done elsewhere (for instance for bpaddr
earlier in the same function).
I wonder if find_pc_partial_function should do that itself, in order to
return an address that is suitable for arithmetic. In any case, that
would be a change with a broad impact, so for now just fix the issue
locally.
After the patch:
$ ./gdb -nx -q --data-directory=data-directory testsuite/outputs/gdb.arch/arm-disp-step/arm-disp-step -ex "break *test_call_end"
Reading symbols from testsuite/outputs/gdb.arch/arm-disp-step/arm-disp-step...
Breakpoint 1 at 0x54a: file /home/smarchi/src/binutils-gdb/gdb/testsuite/gdb.arch/arm-disp-step.S, line 103.
Change-Id: I74fc458dbea0d2c1e1f5eadd90755188df089288
Approved-By: Luis Machado <luis.machado@arm.com>
Diffstat (limited to 'gdb')
-rw-r--r-- | gdb/arm-tdep.c | 9 |
1 files changed, 6 insertions, 3 deletions
diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c index a9c43b2..d4047dd 100644 --- a/gdb/arm-tdep.c +++ b/gdb/arm-tdep.c @@ -5337,9 +5337,12 @@ arm_adjust_breakpoint_address (struct gdbarch *gdbarch, CORE_ADDR bpaddr) bpaddr = gdbarch_addr_bits_remove (gdbarch, bpaddr); - if (find_pc_partial_function (bpaddr, NULL, &func_start, NULL) - && func_start > boundary) - boundary = func_start; + if (find_pc_partial_function (bpaddr, NULL, &func_start, NULL)) + { + func_start = gdbarch_addr_bits_remove (gdbarch, func_start); + if (func_start > boundary) + boundary = func_start; + } /* Search for a candidate IT instruction. We have to do some fancy footwork to distinguish a real IT instruction from the second |