aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorSimon Marchi <simon.marchi@efficios.com>2023-11-07 11:11:18 -0500
committerSimon Marchi <simon.marchi@efficios.com>2023-11-07 11:58:58 -0500
commit8c3273ee07fae4badfd6f25f67162e5f6ec7f03e (patch)
treeb1ba3ce5e6f5a297241ea8eecd54f947933829d3
parenteab996435fe65a421541f59557c5f1fd427573a3 (diff)
downloadgdb-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>
-rw-r--r--gdb/arm-tdep.c9
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