From: Tom de Vries <tdevries@suse.de>
To: Simon Marchi <simark@simark.ca>, gdb-patches@sourceware.org
Subject: Re: [PATCH] [gdb] Fix assert in delete_breakpoint
Date: Tue, 21 Nov 2023 17:52:52 +0100 [thread overview]
Message-ID: <7794354b-0b08-48f2-8cde-499787b5850c@suse.de> (raw)
In-Reply-To: <114c9994-ed81-4a14-bcc3-bd8086e46340@simark.ca>
On 11/21/23 17:11, Simon Marchi wrote:
> On 11/15/23 06:12, Tom de Vries wrote:
>> Because it's deleted twice.
>>
>> The last thing we see in gdb.log before the internal error is:
>> ...
>> [infrun] handle_signal_stop: [13633.13633.0] hit its single-step breakpoint^M
>> ...
>>
>> And the statement following the infrun_debug_printf printing that message is:
>> ...
>> delete_just_stopped_threads_single_step_breakpoints ();
>> ...
>>
>> So, the following events happen:
>> - the single-step breakpoint is hit
>> - delete_just_stopped_threads_single_step_breakpoints is called
>> - during delete_just_stopped_threads_single_step_breakpoints,
>> delete_breakpoint is called which removes the breakpoint from the
>> breakpoint chain
>> - gdb is interrupted by SIGTERM before finish delete_breakpoint
>> - the SIGTERM triggers a SCOPE_EXIT cleanup, calling
>> delete_just_stopped_threads_infrun_breakpoints which ends up
>> calling delete_breakpoint again for the same breakpoint.
>>
>> The proposed fix tries to handle delete_breakpoint being interrupted, and called again.
>>
>> This is an alternative fix:
>> ...
>> diff --git a/gdb/thread.c b/gdb/thread.c
>> index 47cc5c9cd14..7f029f2414c 100644
>> --- a/gdb/thread.c
>> +++ b/gdb/thread.c
>> @@ -93,11 +93,12 @@ inferior_thread (void)
>> static void
>> delete_thread_breakpoint (struct breakpoint **bp_p)
>> {
>> - if (*bp_p != NULL)
>> - {
>> - delete_breakpoint (*bp_p);
>> - *bp_p = NULL;
>> - }
>> + struct breakpoint *bp = *bp_p;
>> +
>> + *bp_p = nullptr;
>> +
>> + if (bp != nullptr)
>> + delete_breakpoint (bp);
>> }
>>
>> void
>> ...
>>
>> It makes sure delete_breakpoint is called only once, but I don't think this is the way to go, since it prevents the cleanup.
>
> Err, my intuition is that it doesn't make sense for operations like
> that, that update GDB's state (especially the tricky infrun / breakpoint
> state) in reaction to target events, to be interruptible by SIGTERM.
> It's just a recipe for ending up with half-baked state that is not
> up-to-date with the reality.
>
> I suppose that after receiving the SIGTERM, execution is interrupted by
> a QUIT macro and a gdb_exception_quit is thrown? Are you able to tell
> which QUIT causes this to happen, and show what the stack looks like at
> this point?
It's the QUIT in target_write_with_progress.
Backtrace:
...
(gdb) bt
#0 0xf75906d2 in __cxa_throw () from
/usr/lib/arm-linux-gnueabihf/libstdc++.so.6
#1 0xab1ce512 in throw_it (reason=RETURN_FORCED_QUIT,
error=GDB_NO_ERROR, fmt=0xab303268 "SIGTERM", ap=...)
at /home/rock/gdb/src/gdbsupport/common-exceptions.cc:202
#2 0xab1ce622 in throw_forced_quit (fmt=0xab303268 "SIGTERM")
at /home/rock/gdb/src/gdbsupport/common-exceptions.cc:247
#3 0xab022576 in quit () at /home/rock/gdb/src/gdb/utils.c:639
#4 0xab0225f0 in maybe_quit () at /home/rock/gdb/src/gdb/utils.c:666
#5 0xaaf867e4 in target_write_with_progress (ops=0xab43d7b0
<the_arm_linux_nat_target>,
object=TARGET_OBJECT_RAW_MEMORY, annex=0x0, buf=0xab51f4dc
"\376\347", offset=2863310110, len=2, progress=0x0,
baton=0x0) at /home/rock/gdb/src/gdb/target.c:2213
#6 0xaaf86828 in target_write (ops=0xab43d7b0
<the_arm_linux_nat_target>, object=TARGET_OBJECT_RAW_MEMORY,
annex=0x0, buf=0xab51f4dc "\376\347", offset=2863310110, len=2) at
/home/rock/gdb/src/gdb/target.c:2226
#7 0xaaf85d30 in target_write_raw_memory (memaddr=2863310110,
myaddr=0xab51f4dc "\376\347", len=2)
at /home/rock/gdb/src/gdb/target.c:1849
#8 0xaae48d64 in default_memory_remove_breakpoint (gdbarch=0xab519290,
bp_tgt=0xab51f4c0)
at /home/rock/gdb/src/gdb/mem-break.c:83
#9 0xaab57bee in gdbarch_memory_remove_breakpoint (gdbarch=0xab519290,
bp_tgt=0xab51f4c0)
at /home/rock/gdb/src/gdb/gdbarch.c:2892
#10 0xaae48da2 in memory_remove_breakpoint (ops=0xab43d7b0
<the_arm_linux_nat_target>, gdbarch=0xab519290,
bp_tgt=0xab51f4c0, reason=REMOVE_BREAKPOINT) at
/home/rock/gdb/src/gdb/mem-break.c:100
#11 0xaab6a9b8 in
memory_breakpoint_target<process_stratum_target>::remove_breakpoint (
this=0xab43d7b0 <the_arm_linux_nat_target>, gdbarch=0xab519290,
bp_tgt=0xab51f4c0, reason=REMOVE_BREAKPOINT)
at /home/rock/gdb/src/gdb/target.h:2440
#12 0xaaf86b70 in target_remove_breakpoint (gdbarch=0xab519290,
bp_tgt=0xab51f4c0, reason=REMOVE_BREAKPOINT)
at /home/rock/gdb/src/gdb/target.c:2381
#13 0xaabc6db8 in code_breakpoint::remove_location (this=0xab51f3a8,
bl=0xab51f450, reason=REMOVE_BREAKPOINT)
at /home/rock/gdb/src/gdb/breakpoint.c:12002
#14 0xaabb65f8 in remove_breakpoint_1 (bl=0xab51f450,
reason=REMOVE_BREAKPOINT)
at /home/rock/gdb/src/gdb/breakpoint.c:4109
#15 0xaabb688a in remove_breakpoint (bl=0xab51f450) at
/home/rock/gdb/src/gdb/breakpoint.c:4214
#16 0xaabc6136 in update_global_location_list (insert_mode=UGLL_DONT_INSERT)
at /home/rock/gdb/src/gdb/breakpoint.c:11554
#17 0xaabc7ff8 in delete_breakpoint (bpt=0xab51f3a8) at
/home/rock/gdb/src/gdb/breakpoint.c:12657
#18 0xaaf9d556 in delete_thread_breakpoint (bp_p=0xab4d3650) at
/home/rock/gdb/src/gdb/thread.c:98
#19 0xaaf9d5bc in delete_single_step_breakpoints (tp=0xab4d3618) at
/home/rock/gdb/src/gdb/thread.c:123
#20 0xaadce6ee in for_each_just_stopped_thread (func=0xaaf9d5a5
<delete_single_step_breakpoints(thread_info*)>)
at /home/rock/gdb/src/gdb/infrun.c:3920
#21 0xaadce7a6 in delete_just_stopped_threads_single_step_breakpoints ()
at /home/rock/gdb/src/gdb/infrun.c:3945
#22 0xaadd4a52 in handle_signal_stop (ecs=0xfffeeda0) at
/home/rock/gdb/src/gdb/infrun.c:6908
#23 0xaadd39b2 in handle_inferior_event (ecs=0xfffeeda0) at
/home/rock/gdb/src/gdb/infrun.c:6494
#24 0xaadd008e in fetch_inferior_event () at
/home/rock/gdb/src/gdb/infrun.c:4654
#25 0xaadb3554 in inferior_event_handler (event_type=INF_REG_EVENT) at
/home/rock/gdb/src/gdb/inf-loop.c:42
#26 0xaae0c700 in handle_target_event (error=0, client_data=0x0) at
/home/rock/gdb/src/gdb/linux-nat.c:4316
#27 0xab1d2b80 in handle_file_event (file_ptr=0xab5e3348, ready_mask=1)
at /home/rock/gdb/src/gdbsupport/event-loop.cc:573
#28 0xab1d2e74 in gdb_wait_for_event (block=0) at
/home/rock/gdb/src/gdbsupport/event-loop.cc:694
#29 0xab1d205e in gdb_do_one_event (mstimeout=-1) at
/home/rock/gdb/src/gdbsupport/event-loop.cc:217
#30 0xaafa6f8e in wait_sync_command_done () at
/home/rock/gdb/src/gdb/top.c:427
#31 0xaafa7012 in maybe_wait_sync_command_done (was_sync=0) at
/home/rock/gdb/src/gdb/top.c:444
#32 0xaafa757a in execute_command (p=0xfffef148 "", from_tty=0) at
/home/rock/gdb/src/gdb/top.c:577
#33 0xaad5cb96 in command_handler (command=0xfffef144 "step") at
/home/rock/gdb/src/gdb/event-top.c:566
#34 0xaafa6da4 in read_command_file (stream=0xab4ba590) at
/home/rock/gdb/src/gdb/top.c:342
#35 0xaac3657e in script_from_file (stream=0xab4ba590, file=0xfffef7e2
"outputs/gdb.base/gdb-sigterm/gdb.in.1")
at /home/rock/gdb/src/gdb/cli/cli-script.c:1642
#36 0xaac203ca in source_script_from_stream (stream=0xab4ba590,
file=0xfffef7e2 "outputs/gdb.base/gdb-sigterm/gdb.in.1",
file_to_open=0xab466da8 "outputs/gdb.base/gdb-sigterm/gdb.in.1") at
/home/rock/gdb/src/gdb/cli/cli-cmds.c:730
#37 0xaac204d0 in source_script_with_search (file=0xfffef7e2
"outputs/gdb.base/gdb-sigterm/gdb.in.1", from_tty=0,
search_path=0) at /home/rock/gdb/src/gdb/cli/cli-cmds.c:775
#38 0xaac20524 in source_script (file=0xfffef7e2
"outputs/gdb.base/gdb-sigterm/gdb.in.1", from_tty=0)
at /home/rock/gdb/src/gdb/cli/cli-cmds.c:784
#39 0xaae33760 in catch_command_errors (command=0xaac20511
<source_script(char const*, int)>,
arg=0xfffef7e2 "outputs/gdb.base/gdb-sigterm/gdb.in.1", from_tty=0,
do_bp_actions=false)
at /home/rock/gdb/src/gdb/main.c:513
#40 0xaae338da in execute_cmdargs (cmdarg_vec=0xfffef3d4,
file_type=CMDARG_FILE, cmd_type=CMDARG_COMMAND,
ret=0xfffef3c4) at /home/rock/gdb/src/gdb/main.c:609
#41 0xaae34c9a in captured_main_1 (context=0xfffef514) at
/home/rock/gdb/src/gdb/main.c:1293
#42 0xaae34e52 in captured_main (data=0xfffef514) at
/home/rock/gdb/src/gdb/main.c:1314
#43 0xaae34ebc in gdb_main (args=0xfffef514) at
/home/rock/gdb/src/gdb/main.c:1343
#44 0xaaaee67a in main (argc=5, argv=0xfffef684) at
/home/rock/gdb/src/gdb/gdb.c:39
(gdb)
...
Thanks,
- Tom
next prev parent reply other threads:[~2023-11-21 16:50 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-11-13 15:26 Tom de Vries
2023-11-14 15:09 ` Simon Marchi
2023-11-15 11:12 ` Tom de Vries
2023-11-15 12:12 ` Tom de Vries
2023-11-21 12:22 ` Tom de Vries
2023-11-21 16:11 ` Simon Marchi
2023-11-21 16:52 ` Tom de Vries [this message]
2023-11-22 12:44 ` Tom de Vries
2023-11-22 14:49 ` Tom de Vries
2023-11-27 10:19 ` Tom de Vries
2023-11-27 16:29 ` Simon Marchi
2023-11-28 15:22 ` Tom de Vries
2023-11-28 15:30 ` Tom Tromey
2023-11-29 12:08 ` Tom de Vries
2023-11-29 20:46 ` Tom de Vries
2023-11-29 21:33 ` Tom de Vries
2023-11-30 17:08 ` Simon Marchi
2023-11-21 16:40 ` Luis Machado
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=7794354b-0b08-48f2-8cde-499787b5850c@suse.de \
--to=tdevries@suse.de \
--cc=gdb-patches@sourceware.org \
--cc=simark@simark.ca \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).