From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr1-f41.google.com (mail-wr1-f41.google.com [209.85.221.41]) by sourceware.org (Postfix) with ESMTPS id 4431E3861016 for ; Wed, 8 Jul 2020 23:31:33 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 4431E3861016 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-f41.google.com with SMTP id b6so343442wrs.11 for ; Wed, 08 Jul 2020 16:31:33 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:subject:date:message-id:in-reply-to :references; bh=G6X5zVbaXbRbjQQ3ClfvW8MyKg9+58zvxFJ2+U2tj/E=; b=aCOTo+ogbDp4hKIvK0tQBoWg9ZqVaC50ANkWuECKCfeYjcsPBOZ/zCWUWQX7y2kHRb jOyBk6DXAtNUbU0eGeRBt7TOpCEDFzolI0FJ20ZDkAc/mCnu5JkOLjb86XmkKt/C6L4Y w9Ns3OpRGq1MSSlaIweOesM5xo9Zhb2SFOjrPp9i6dHyX5056TCVrcz3bWAzXHmPyB83 DZEVEy5tVuFFnHSEiCWCuBKCD6RveFgCmE/UExYyGvunwLZMx+VB4NSYnUhcV8hHiaMr IQnRtI6JPQQD01WAvnY3sOZiM2Wqk2vYNVLgO38JC2WCoFB30j49/6H6AcKpC89Qlqoa V3Sw== X-Gm-Message-State: AOAM531baE+meRviwFpVCwCgVxYJ9YezK0h5gtC0Eewma0He0dYXq2dE 4DC5XFcE1W3BcCLek239fhyi0bHw8tw= X-Google-Smtp-Source: ABdhPJzTZ6UDQGbViDq8wBYtZzgInruqyKO7xYHJOVMd4sqh0vBVt4TbktmhqThkMGwOY1AA4kP0WQ== X-Received: by 2002:adf:ea06:: with SMTP id q6mr58198485wrm.69.1594251092123; Wed, 08 Jul 2020 16:31:32 -0700 (PDT) Received: from localhost ([83.223.242.101]) by smtp.gmail.com with ESMTPSA id a12sm2259705wrv.41.2020.07.08.16.31.30 for (version=TLS1_2 cipher=ECDHE-ECDSA-CHACHA20-POLY1305 bits=256/256); Wed, 08 Jul 2020 16:31:31 -0700 (PDT) From: Pedro Alves To: gdb-patches@sourceware.org Subject: [PATCH 2/3] Fix crash if connection drops in scoped_restore_current_thread's ctor, part 2 Date: Thu, 9 Jul 2020 00:31:24 +0100 Message-Id: <20200708233125.1030-3-pedro@palves.net> X-Mailer: git-send-email 2.14.5 In-Reply-To: <20200708233125.1030-1-pedro@palves.net> References: <20200708233125.1030-1-pedro@palves.net> X-Spam-Status: No, score=-5.6 required=5.0 tests=BAYES_00, FREEMAIL_FORGED_FROMDOMAIN, FREEMAIL_FROM, GIT_PATCH_0, HEADER_FROM_DIFFERENT_DOMAINS, KAM_DMARC_STATUS, KAM_NUMSUBJECT, RCVD_IN_ABUSEAT, RCVD_IN_BARRACUDACENTRAL, 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, 08 Jul 2020 23:31:34 -0000 Running the testsuite against an Asan-enabled build of GDB makes gdb.base/multi-target.exp expose this bug. scoped_restore_current_thread's ctor calls get_frame_id to record the selected frame's ID to restore later. If the frame ID hasn't been computed yet, it will be computed on the spot, and that will usually require accessing the target's memory and registers. If the remote connection closes, while we're computing the frame ID, the remote target exits its inferiors, unpushes itself, and throws a TARGET_CLOSE_ERROR error. Exiting the inferiors deletes the inferior's threads. scoped_restore_current_thread increments the current thread's refcount to prevent the thread from being deleted from under its feet. However, the code that does that isn't considering the case of the thread being deleted from within get_frame_id. It only increments the refcount _after_ get_frame_id returns. So if the current thread is indeed deleted, the tp->incref (); statement references a stale TP pointer. Incrementing the refcounts earlier fixes it. We should probably also let the TARGET_CLOSE_ERROR error propagate in this case. That alone would fix it, though it seems better to tweak the refcount handling too. gdb/ChangeLog: * thread.c (scoped_restore_current_thread::scoped_restore_current_thread): Incref the thread before calling get_frame_id instead of after. Let TARGET_CLOSE_ERROR propagate. --- gdb/thread.c | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/gdb/thread.c b/gdb/thread.c index f0722d3588..1ec047e35b 100644 --- a/gdb/thread.c +++ b/gdb/thread.c @@ -1433,15 +1433,17 @@ scoped_restore_current_thread::~scoped_restore_current_thread () scoped_restore_current_thread::scoped_restore_current_thread () { - m_thread = NULL; m_inf = current_inferior (); + m_inf->incref (); if (inferior_ptid != null_ptid) { - thread_info *tp = inferior_thread (); + m_thread = inferior_thread (); + m_thread->incref (); + struct frame_info *frame; - m_was_stopped = tp->state == THREAD_STOPPED; + m_was_stopped = m_thread->state == THREAD_STOPPED; if (m_was_stopped && target_has_registers && target_has_stack @@ -1466,13 +1468,14 @@ scoped_restore_current_thread::scoped_restore_current_thread () { m_selected_frame_id = null_frame_id; m_selected_frame_level = -1; - } - tp->incref (); - m_thread = tp; + /* Better let this propagate. */ + if (ex.error == TARGET_CLOSE_ERROR) + throw; + } } - - m_inf->incref (); + else + m_thread = NULL; } /* See gdbthread.h. */ -- 2.14.5