From: Pedro Alves <pedro@palves.net>
To: Simon Marchi <simark@simark.ca>, gdb-patches@sourceware.org
Subject: Re: [PATCH 0/3] Fix crash if connection drops in scoped_restore_current_thread's ctor
Date: Wed, 22 Jul 2020 21:37:43 +0100 [thread overview]
Message-ID: <de5cf500-304f-cbff-66dd-f7ecdd735410@palves.net> (raw)
In-Reply-To: <55d70953-e032-fe6a-1656-d7ad7080b863@simark.ca>
On 7/22/20 8:37 PM, Simon Marchi wrote:
> On 2020-07-10 7:02 p.m., Pedro Alves wrote:
>> On 7/9/20 12:31 AM, Pedro Alves wrote:
>>> (I have internet again: found a sim card of a different operator that
>>> works. This will do until the communications tower near me is
>>> repaired and get I fiber back...)
>>>
>>> This series fixes the crashes exposed by the
>>> gdb.multi/multi-target.exp testcase when run against an Asan-enabled
>>> GDB build, initially reported by Simon here:
>>>
>>> https://sourceware.org/pipermail/gdb-patches/2020-July/170222.html
>>>
>>> The first two patches fix the crashes, and we should probably put them
>>> in GDB 10.
>>>
>>> The last patch is a follow up that avoids swallowing exceptions in
>>> scoped_restore_current_thread's dtor that I'm thinking would be a bit
>>> too invasive to put in GDB 10, I think it could do with a longer
>>> baking period in master.
>>>
>>> Pedro Alves (3):
>>> Fix crash if connection drops in scoped_restore_current_thread's ctor,
>>> part 1
>>> Fix crash if connection drops in scoped_restore_current_thread's ctor,
>>> part 2
>>> Make scoped_restore_current_thread's cdtors exception free (RFC)
>>
>> I've now merged patches 1 and 2. Patch 3 will wait until after the branch
>> is cut.
>>
>
> I now see this other ASan failure when running gdb.multi/multi-target.exp, it's in the
> attached asan.log. There are colors, so it's easier to read if you "cat" it in your
> terminal. It looks familiar, because it happens in scoped_restore_current_thread's dtor
> (not ctor), but maybe it just happens to be there but could happen at any other point.
>
> It happens when starting test_continue with non-stop on, just after having completed
> test_continue with non-stop off. It's when GDB does "monitor exit".
>
> Unfortunately, the "freed by thread T0 here" stack trace is again truncated, probably
> because the stack is too deep for the portion of the stack ASan captures. But I managed
> to attach to GDB with GDB using gdb_interact and capture it (I broke on unpush_and_perror),
> here's the equivalent GDB backtrace:
>
> #0 xfree<void> (ptr=0x621004a5d900) at /home/smarchi/src/binutils-gdb/gdb/../gdbsupport/common-utils.h:63
> #1 0x0000000001626260 in call_freefun (h=0x20f8da0 <frame_cache_obstack>, old_chunk=0x621004a5d900) at /home/smarchi/src/binutils-gdb/libiberty/obstack.c:103
> #2 0x0000000001626c87 in _obstack_free (h=0x20f8da0 <frame_cache_obstack>, obj=0x0) at /home/smarchi/src/binutils-gdb/libiberty/obstack.c:280
> #3 0x000000000098ae26 in reinit_frame_cache () at /home/smarchi/src/binutils-gdb/gdb/frame.c:1856
> #4 0x0000000001098adf in switch_to_no_thread () at /home/smarchi/src/binutils-gdb/gdb/thread.c:1301
> #5 0x0000000000acf544 in switch_to_inferior_no_thread (inf=0x615000244d00) at /home/smarchi/src/binutils-gdb/gdb/inferior.c:626
> #6 0x0000000000e7c38c in remote_unpush_target (target=0x6170000c0c00) at /home/smarchi/src/binutils-gdb/gdb/remote.c:5521
> #7 0x0000000000e92db6 in unpush_and_perror (target=0x6170000c0c00, string=0x191d400 "Remote communication error. Target disconnected.") at /home/smarchi/src/binutils-gdb/gdb/remote.c:9101
> #8 0x0000000000e930c7 in remote_target::readchar (this=0x6170000c0c00, timeout=2) at /home/smarchi/src/binutils-gdb/gdb/remote.c:9141
> #9 0x0000000000e9576f in remote_target::getpkt_or_notif_sane_1 (this=0x6170000c0c00, buf=0x6170000c0c18, forever=0, expecting_notif=0, is_notif=0x0) at /home/smarchi/src/binutils-gdb/gdb/remote.c:9683
> #10 0x0000000000e961c9 in remote_target::getpkt_sane (this=0x6170000c0c00, buf=0x6170000c0c18, forever=0) at /home/smarchi/src/binutils-gdb/gdb/remote.c:9790
> #11 0x0000000000e95545 in remote_target::getpkt (this=0x6170000c0c00, buf=0x6170000c0c18, forever=0) at /home/smarchi/src/binutils-gdb/gdb/remote.c:9623
> #12 0x0000000000e91ba3 in remote_target::remote_read_bytes_1 (this=0x6170000c0c00, memaddr=0x7ffff78bc38d, myaddr=0x7fff7dca59a0 "", len_units=1, unit_size=1, xfered_len_units=0x7fff7dca58b0) at /home/smarchi/src/binutils-gdb/gdb/remote.c:8860
> #13 0x0000000000e9240c in remote_target::remote_read_bytes (this=0x6170000c0c00, memaddr=0x7ffff78bc38d, myaddr=0x7fff7dca59a0 "", len=1, unit_size=1, xfered_len=0x7fff7dca58b0) at /home/smarchi/src/binutils-gdb/gdb/remote.c:8987
> #14 0x0000000000e9b821 in remote_target::xfer_partial (this=0x6170000c0c00, object=TARGET_OBJECT_MEMORY, annex=0x0, readbuf=0x7fff7dca59a0 "", writebuf=0x0, offset=140737346519949, len=1, xfered_len=0x7fff7dca58b0) at /home/smarchi/src/binutils-gdb/gdb/remote.c:10987
> #15 0x000000000104fd3a in raw_memory_xfer_partial (ops=0x6170000c0c00, readbuf=0x7fff7dca59a0 "", writebuf=0x0, memaddr=140737346519949, len=1, xfered_len=0x7fff7dca58b0) at /home/smarchi/src/binutils-gdb/gdb/target.c:918
> #16 0x0000000001050425 in memory_xfer_partial_1 (ops=0x6170000c0c00, object=TARGET_OBJECT_MEMORY, readbuf=0x7fff7dca59a0 "", writebuf=0x0, memaddr=140737346519949, len=1, xfered_len=0x7fff7dca58b0) at /home/smarchi/src/binutils-gdb/gdb/target.c:1047
> #17 0x0000000001050608 in memory_xfer_partial (ops=0x6170000c0c00, object=TARGET_OBJECT_MEMORY, readbuf=0x7fff7dca59a0 "", writebuf=0x0, memaddr=140737346519949, len=1, xfered_len=0x7fff7dca58b0) at /home/smarchi/src/binutils-gdb/gdb/target.c:1076
> #18 0x0000000001050b92 in target_xfer_partial (ops=0x6170000c0c00, object=TARGET_OBJECT_MEMORY, annex=0x0, readbuf=0x7fff7dca59a0 "", writebuf=0x0, offset=140737346519949, len=1, xfered_len=0x7fff7dca58b0) at /home/smarchi/src/binutils-gdb/gdb/target.c:1133
> #19 0x0000000001051a7b in target_read_partial (ops=0x6170000c0c00, object=TARGET_OBJECT_MEMORY, annex=0x0, buf=0x7fff7dca59a0 "", offset=140737346519949, len=1, xfered_len=0x7fff7dca58b0) at /home/smarchi/src/binutils-gdb/gdb/target.c:1379
> #20 0x0000000001051c59 in target_read (ops=0x6170000c0c00, object=TARGET_OBJECT_MEMORY, annex=0x0, buf=0x7fff7dca59a0 "", offset=140737346519949, len=1) at /home/smarchi/src/binutils-gdb/gdb/target.c:1419
> #21 0x0000000001051178 in target_read_memory (memaddr=0x7ffff78bc38d, myaddr=0x7fff7dca59a0 "", len=1) at /home/smarchi/src/binutils-gdb/gdb/target.c:1222
> #22 0x00000000004b4731 in amd64_stack_frame_destroyed_p (gdbarch=0x6210027e8510, pc=0x7ffff78bc38d) at /home/smarchi/src/binutils-gdb/gdb/amd64-tdep.c:2909
> #23 0x00000000004b4822 in amd64_epilogue_frame_sniffer (self=0x169df00 <amd64_epilogue_frame_unwind>, this_frame=0x621004a5d9e0, this_prologue_cache=0x621004a5d9f8) at /home/smarchi/src/binutils-gdb/gdb/amd64-tdep.c:2924
> #24 0x0000000000981048 in frame_unwind_try_unwinder (this_frame=0x621004a5d9e0, this_cache=0x621004a5d9f8, unwinder=0x169df00 <amd64_epilogue_frame_unwind>) at /home/smarchi/src/binutils-gdb/gdb/frame-unwind.c:128
> #25 0x000000000098126d in frame_unwind_find_by_frame (this_frame=0x621004a5d9e0, this_cache=0x621004a5d9f8) at /home/smarchi/src/binutils-gdb/gdb/frame-unwind.c:186
> #26 0x0000000000983c9d in compute_frame_id (fi=0x621004a5d9e0) at /home/smarchi/src/binutils-gdb/gdb/frame.c:546
> #27 0x0000000000984167 in get_frame_id (fi=0x621004a5d9e0) at /home/smarchi/src/binutils-gdb/gdb/frame.c:582
> #28 0x0000000001098eef in restore_selected_frame (a_frame_id=..., frame_level=0) at /home/smarchi/src/binutils-gdb/gdb/thread.c:1355
> #29 0x00000000010992f8 in scoped_restore_current_thread::restore (this=0x7fff7dca5f30) at /home/smarchi/src/binutils-gdb/gdb/thread.c:1411
> #30 0x0000000001099355 in scoped_restore_current_thread::~scoped_restore_current_thread (this=0x7fff7dca5f30, __in_chrg=<optimized out>) at /home/smarchi/src/binutils-gdb/gdb/thread.c:1420
> #31 0x0000000000aeab84 in do_target_wait (wait_ptid=..., ecs=0x7fff7dca6290, options=1) at /home/smarchi/src/binutils-gdb/gdb/infrun.c:3670
> #32 0x0000000000aecbe3 in fetch_inferior_event () at /home/smarchi/src/binutils-gdb/gdb/infrun.c:3965
> #33 0x0000000000aa8097 in inferior_event_handler (event_type=INF_REG_EVENT) at /home/smarchi/src/binutils-gdb/gdb/inf-loop.c:42
> #34 0x0000000000eab8b7 in remote_async_inferior_event_handler (data=0x6170000d6a00) at /home/smarchi/src/binutils-gdb/gdb/remote.c:14166
> #35 0x00000000004ca110 in check_async_event_handlers () at /home/smarchi/src/binutils-gdb/gdb/async-event.c:295
> #36 0x00000000015bef41 in gdb_do_one_event () at /home/smarchi/src/binutils-gdb/gdbsupport/event-loop.cc:194
> #37 0x0000000000bfd50e in start_event_loop () at /home/smarchi/src/binutils-gdb/gdb/main.c:356
> #38 0x0000000000bfd816 in captured_command_loop () at /home/smarchi/src/binutils-gdb/gdb/main.c:416
> #39 0x0000000000c00c25 in captured_main (data=0x7fff7dca65d0) at /home/smarchi/src/binutils-gdb/gdb/main.c:1253
> #40 0x0000000000c00cb5 in gdb_main (args=0x7fff7dca65d0) at /home/smarchi/src/binutils-gdb/gdb/main.c:1268
> #41 0x0000000000414d9e in main (argc=5, argv=0x7fff7dca6738) at /home/smarchi/src/binutils-gdb/gdb/gdb.c:32
>
Bah.
>
> The problem seems to be:
>
> - We create a new frame_info object in restore_selected_frame (by calling find_relative_frame)
> - The frame is allocated on the frame_cache_obstack
> - In frame_unwind_try_unwinder, we try to find an unwinder for that frame
> - While trying unwinders, memory read fails because the remote target closes, because of "monitor exit"
> - That calls reinit_frame_cache (as shown above), which resets frame_cache_obstack
> - When handling the exception in frame_unwind_try_unwinder, we try to set some things on the frame_info
> object (like *this_cache, which in fact tries to write into frame_info::prologue_cache), but the
> frame_info object is no more, it went away with the obstack.
I'm thinking that to fix this we will need a generation counter in
reinit_frame_cache. Then in frame_unwind_try_unwinder, don't call
frame_cleanup_after_sniffer if the generation is not the same as it was
on entry.
Something like this. Does it fix it for you? I can't seem to reproduce
the crash here.
From 202b20db082969cfa156468c3443888761629dee Mon Sep 17 00:00:00 2001
From: Pedro Alves <pedro@palves.net>
Date: Wed, 22 Jul 2020 20:53:59 +0100
Subject: [PATCH] Fix yet another bug exposed by ASAN + multi-target.exp
---
gdb/frame-unwind.c | 13 ++++++++++---
gdb/frame.c | 13 +++++++++++++
gdb/frame.h | 4 ++++
3 files changed, 27 insertions(+), 3 deletions(-)
diff --git a/gdb/frame-unwind.c b/gdb/frame-unwind.c
index 3334c472d02..ba25e19172e 100644
--- a/gdb/frame-unwind.c
+++ b/gdb/frame-unwind.c
@@ -121,6 +121,8 @@ frame_unwind_try_unwinder (struct frame_info *this_frame, void **this_cache,
{
int res = 0;
+ unsigned int entry_generation = get_frame_cache_generation ();
+
frame_prepare_for_sniffer (this_frame, unwinder);
try
@@ -130,9 +132,14 @@ frame_unwind_try_unwinder (struct frame_info *this_frame, void **this_cache,
catch (const gdb_exception &ex)
{
/* Catch all exceptions, caused by either interrupt or error.
- Reset *THIS_CACHE. */
- *this_cache = NULL;
- frame_cleanup_after_sniffer (this_frame);
+ Reset *THIS_CACHE, unless something reinitialized the frame
+ cache meanwhile, in which case THIS_FRAME is now
+ dangling. */
+ if (get_frame_cache_generation () == entry_generation)
+ {
+ *this_cache = NULL;
+ frame_cleanup_after_sniffer (this_frame);
+ }
if (ex.error == NOT_AVAILABLE_ERROR)
{
diff --git a/gdb/frame.c b/gdb/frame.c
index ac1016b083f..4ac958a1e95 100644
--- a/gdb/frame.c
+++ b/gdb/frame.c
@@ -53,6 +53,17 @@
static struct frame_info *sentinel_frame;
+/* Number of calls to reinit_frame_cache. */
+static unsigned int frame_cache_generation = 0;
+
+/* See frame.h. */
+
+unsigned int
+get_frame_cache_generation ()
+{
+ return frame_cache_generation;
+}
+
/* The values behind the global "set backtrace ..." settings. */
set_backtrace_options user_set_backtrace_options;
@@ -1843,6 +1854,8 @@ reinit_frame_cache (void)
{
struct frame_info *fi;
+ ++frame_cache_generation;
+
/* Tear down all frame caches. */
for (fi = sentinel_frame; fi != NULL; fi = fi->prev)
{
diff --git a/gdb/frame.h b/gdb/frame.h
index cfc15022ed5..8d029cc065d 100644
--- a/gdb/frame.h
+++ b/gdb/frame.h
@@ -949,6 +949,10 @@ extern const gdb::option::option_def set_backtrace_option_defs[2];
/* The values behind the global "set backtrace ..." settings. */
extern set_backtrace_options user_set_backtrace_options;
+/* Get the number of calls to reinit_frame_cache. */
+
+unsigned int get_frame_cache_generation ();
+
/* Mark that the PC value is masked for the previous frame. */
extern void set_frame_previous_pc_masked (struct frame_info *frame);
base-commit: 32fa152e3bfcf021ce49767be547fae5129d922b
--
2.14.5
next prev parent reply other threads:[~2020-07-22 20:37 UTC|newest]
Thread overview: 31+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-07-08 23:31 Pedro Alves
2020-07-08 23:31 ` [PATCH 1/3] Fix crash if connection drops in scoped_restore_current_thread's ctor, part 1 Pedro Alves
2020-07-09 3:17 ` Simon Marchi
2020-07-09 10:51 ` Pedro Alves
2020-07-09 14:13 ` Simon Marchi
2020-07-08 23:31 ` [PATCH 2/3] Fix crash if connection drops in scoped_restore_current_thread's ctor, part 2 Pedro Alves
2020-07-09 3:31 ` Simon Marchi
2020-07-09 11:12 ` Pedro Alves
2020-07-09 14:16 ` Simon Marchi
2020-07-09 17:23 ` Pedro Alves
2020-07-09 17:28 ` Simon Marchi
2020-07-08 23:31 ` [PATCH 3/3] Make scoped_restore_current_thread's cdtors exception free (RFC) Pedro Alves
2020-07-09 3:49 ` Simon Marchi
2020-07-09 11:56 ` Pedro Alves
2020-07-09 12:09 ` Pedro Alves
2020-07-09 15:40 ` Simon Marchi
2020-07-09 22:22 ` Pedro Alves
2020-07-10 2:55 ` Simon Marchi
2020-10-30 1:13 ` Pedro Alves
2020-10-30 1:37 ` [pushed] Move lookup_selected_frame to frame.c Pedro Alves
2020-10-30 7:44 ` [PATCH 3/3] Make scoped_restore_current_thread's cdtors exception free (RFC) Aktemur, Tankut Baris
2020-10-30 11:32 ` Pedro Alves
2020-10-31 14:35 ` [PATCH] Fix frame cycle detection (Re: [PATCH 3/3] Make scoped_restore_current_thread's cdtors exception free (RFC)) Pedro Alves
2020-11-09 14:05 ` Aktemur, Tankut Baris
2020-11-16 13:48 ` Tom de Vries
2020-11-16 14:57 ` Pedro Alves
2020-07-10 23:02 ` [PATCH 0/3] Fix crash if connection drops in scoped_restore_current_thread's ctor Pedro Alves
2020-07-22 19:37 ` Simon Marchi
2020-07-22 20:37 ` Pedro Alves [this message]
2020-07-22 20:47 ` Simon Marchi
2020-07-23 15:28 ` [pushed] Don't touch frame_info objects if frame cache was reinitialized (was: Re: [PATCH 0/3] Fix crash if connection drops in scoped_restore_current_thread's ctor) Pedro Alves
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=de5cf500-304f-cbff-66dd-f7ecdd735410@palves.net \
--to=pedro@palves.net \
--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).