From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 82582 invoked by alias); 19 Nov 2019 17:22:06 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 82569 invoked by uid 89); 19 Nov 2019 17:22:05 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=BAYES_00 autolearn=unavailable version=3.3.1 spammy= X-HELO: us-smtp-1.mimecast.com Received: from us-smtp-delivery-1.mimecast.com (HELO us-smtp-1.mimecast.com) (207.211.31.120) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 19 Nov 2019 17:22:04 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1574184123; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=T7l8n+xwK9VuYXcQ39PUje1u6r8qgW7ptBVW3zbf924=; b=WYS9PSvXioxOIcK08Jfxae/rGY4mb+F9Vr8p9GPPLBF8RXS/84Lo/sSgMK8BTEG/YWWF+X 6zF51DQM7EEXM3aRUGgjLTwObuevubjgNL7G4fgACArHbM6Ku8OcfYR27kEV2yRSrld+xX pUaY6eJbPR+FzfvQMu7wn5+YnFF5Jbs= Received: from mail-wm1-f70.google.com (mail-wm1-f70.google.com [209.85.128.70]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-317-zP5pCKUqMIiBxAvmFNjcvA-1; Tue, 19 Nov 2019 12:22:01 -0500 Received: by mail-wm1-f70.google.com with SMTP id 199so2990111wmb.0 for ; Tue, 19 Nov 2019 09:22:01 -0800 (PST) Return-Path: Received: from ?IPv6:2001:8a0:f913:f700:56ee:75ff:fe8d:232b? ([2001:8a0:f913:f700:56ee:75ff:fe8d:232b]) by smtp.gmail.com with ESMTPSA id z2sm16042116wrs.89.2019.11.19.09.21.58 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 19 Nov 2019 09:21:59 -0800 (PST) Subject: Re: [review] Handle pending stops from the Windows kernel To: Tom Tromey References: <94963d2c-961d-e48b-4e24-ad69472114d6@redhat.com> <87eey4x80s.fsf@tromey.com> Cc: tromey@sourceware.org, gdb-patches@sourceware.org, "Tom Tromey (Code Review)" From: Pedro Alves Message-ID: <4f51da38-46eb-269e-001c-c71ce88f469d@redhat.com> Date: Tue, 19 Nov 2019 17:22:00 -0000 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: <87eey4x80s.fsf@tromey.com> X-Mimecast-Spam-Score: 0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable X-SW-Source: 2019-11/txt/msg00601.txt.bz2 On 11/19/19 2:20 PM, Tom Tromey wrote: >>>>>> "Pedro" =3D=3D Pedro Alves writes: > Pedro> I think you should unwind the PC here, not only when returning the= pending > Pedro> event to GDB core. Consider the case of two threads hitting a bre= akpoint > Pedro> at the same time. When that happens, and do you "info threads", y= ou want to > Pedro> see the PC of all threads pointing at a valid instruction. If you= don't > Pedro> unwind the PC of pending breakpoints, then the threads with pendin= g breakpoints > Pedro> will have their PC offset by one. >=20 > I think I tried this, but I can try again. Thanks. >=20 >>> + if (software_breakpoint_inserted_here_p (regcache->aspace (), p= c)) >=20 > Pedro> Why is software_breakpoint_inserted_here_p needed?=20=20 >=20 > Offsetting the PC did not work without this. > I tried to document my findings here: >=20 > https://sourceware.org/ml/gdb-patches/2019-10/msg00338.html >=20 Off hand that doesn't sound right. Linux doesn't do that. See linux-nat.c:save_stop_reason, in the USE_SIGTRAP_SIGINFO case (the #else case is probably and hopefully dead by now). With the software_breakpoint_inserted_here_p check in place, what I imagine would happen is: - thread A and B hit a breakpoint - the event for thread B is left pending - event for thread A is reported - user/GDB removes the breakpoint before the event for thread B is processed - user continues - windows-nat.c prepares to return the pending event for B - software_breakpoint_inserted_here_p returns false, so the PC is left unad= justed - gdb core reports a spurious SIGTRAP, with the PC left unadjusted - if the inferior is resumed, it starts execution with a bogus PC Without the check, what should happen, and is the right behavior, is: - thread A and B hit a breakpoint - the event for thread B is left pending - event for thread A is reported - user/GDB removes the breakpoint before the event for thread B is processed - user continues - windows-nat.c prepares to return the pending event for B, adjusts the PC - gdb core sees a TARGET_STOPPED_BY_SW_BREAKPOINT event, with the PC already adjusted. - there's no breakpoint at that address, so gdb re-resumes the inferior transparently, here, in infrun.c: /* A delayed software breakpoint event. Ignore the trap. */ if (debug_infrun) fprintf_unfiltered (gdb_stdlog, "infrun: delayed software breakpoint " "trap, ignoring\n"); Basically, this mechanism replaces the old moribund locations heuristic. > IIRC what happened is that gdb would sometimes resume the inferior with > wrong PC, causing it to crash. However, I don't really recall, since it > was a long time ago now. I guess I'll re-do the experiments. Thanks. I'm mystified. Pedro Alves