From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr1-f68.google.com (mail-wr1-f68.google.com [209.85.221.68]) by sourceware.org (Postfix) with ESMTPS id 603183858D38 for ; Wed, 22 Jul 2020 20:37:47 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 603183858D38 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=palves.net Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=alves.ped@gmail.com Received: by mail-wr1-f68.google.com with SMTP id r4so180588wrx.9 for ; Wed, 22 Jul 2020 13:37:47 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=WxOW1c03IbdZemgiwYeIlaIG1ioUNaIs1rPVjE5Fob8=; b=tG372AiP953x6jZMTNKiMTSnFhCD7BUAru00T1+C6JwdAq0fmx3ZcGzqGXmbI6s2Tz SZk9bMV6QO3GMBpZ7n8KpLojy+DGSuB6TLQT/eJJSBCLr+wTzUmoKNuGfh2oiR+Op/wc 7zNJpWQFOnBLLncxdYWxsk/jzjny3vkbYgsPWjZn5a9UpiotUTxHS5xOPhphwhF4+OVo XCkU25585PqBnOHqseKGWhvEqIziMYQNLtXw1czs44r3PDd78Sg0ZMMm1qYIbQb/hatI +U6heiNZdggJpZKxzNgWDFz7wXi4bm2BIylL3Mgy8K/S7c7ptpQOe/jSmgvlwrgJ+XFb j+bg== X-Gm-Message-State: AOAM53373NRneQ+ZmIo2W4fBOBP+gvpXcplUT2noQBRsjKMhiYiqDrll n+1UFBbLw6PCFTje+rKUpu7dabZNuqg= X-Google-Smtp-Source: ABdhPJzoT3FxSYdLpZgCxBV7DwyWi0m3vUGcL9mXW1WM5KuaaYNlaQ8WE5DEaHhF1c0OZsyGJpKWLw== X-Received: by 2002:a5d:6a8b:: with SMTP id s11mr1093604wru.222.1595450264924; Wed, 22 Jul 2020 13:37:44 -0700 (PDT) Received: from ?IPv6:2001:8a0:f91a:c400:56ee:75ff:fe8d:232b? ([2001:8a0:f91a:c400:56ee:75ff:fe8d:232b]) by smtp.gmail.com with ESMTPSA id t141sm864166wmt.26.2020.07.22.13.37.43 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 22 Jul 2020 13:37:43 -0700 (PDT) Subject: Re: [PATCH 0/3] Fix crash if connection drops in scoped_restore_current_thread's ctor To: Simon Marchi , gdb-patches@sourceware.org References: <20200708233125.1030-1-pedro@palves.net> <55d70953-e032-fe6a-1656-d7ad7080b863@simark.ca> From: Pedro Alves Message-ID: Date: Wed, 22 Jul 2020 21:37:43 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.2.1 MIME-Version: 1.0 In-Reply-To: <55d70953-e032-fe6a-1656-d7ad7080b863@simark.ca> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-8.7 required=5.0 tests=BAYES_00, FREEMAIL_FORGED_FROMDOMAIN, FREEMAIL_FROM, GIT_PATCH_0, HEADER_FROM_DIFFERENT_DOMAINS, KAM_DMARC_STATUS, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 22 Jul 2020 20:37:49 -0000 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 (ptr=0x621004a5d900) at /home/smarchi/src/binutils-gdb/gdb/../gdbsupport/common-utils.h:63 > #1 0x0000000001626260 in call_freefun (h=0x20f8da0 , old_chunk=0x621004a5d900) at /home/smarchi/src/binutils-gdb/libiberty/obstack.c:103 > #2 0x0000000001626c87 in _obstack_free (h=0x20f8da0 , 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 , 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 ) 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=) 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 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