From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 5051 invoked by alias); 30 Sep 2013 17:50:56 -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 5040 invoked by uid 89); 30 Sep 2013 17:50:55 -0000 Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 30 Sep 2013 17:50:55 +0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.3 required=5.0 tests=AWL,BAYES_00,RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com Received: from int-mx01.intmail.prod.int.phx2.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r8UHopNQ003853 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Mon, 30 Sep 2013 13:50:52 -0400 Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.ams2.redhat.com [10.39.146.11]) by int-mx01.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id r8UHonhv015340; Mon, 30 Sep 2013 13:50:50 -0400 Message-ID: <5249B9F9.4030901@redhat.com> Date: Mon, 30 Sep 2013 17:50:00 -0000 From: Pedro Alves User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130625 Thunderbird/17.0.7 MIME-Version: 1.0 To: Yao Qi CC: Mark Kettenis , gdb-patches@sourceware.org Subject: Re: [PATCH 0/7 V2] Trust readonly sections if target has memory protection References: <1378432920-7731-1-git-send-email-yao@codesourcery.com> <1378641807-24256-1-git-send-email-yao@codesourcery.com> <201309091916.r89JGbpf009986@glazunov.sibelius.xs4all.nl> <522E9A8A.7040509@codesourcery.com> <52317B66.3020602@codesourcery.com> <201309120949.r8C9nFsJ016506@glazunov.sibelius.xs4all.nl> <5232C9EC.2040707@codesourcery.com> In-Reply-To: <5232C9EC.2040707@codesourcery.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-SW-Source: 2013-09/txt/msg01013.txt.bz2 On 09/13/2013 09:16 AM, Yao Qi wrote: > On 09/12/2013 05:49 PM, Mark Kettenis wrote: >> I'm certainly not outright rejecting it. But you'll certainly need to >> rethink in which contexts it is safe/acceptable that "auto" actually >> turns on the trust-readonly-sections feature. That decision should >> probably be done on a per-architecture, per-OS basis, and only for >> remote debugging. > > Yeah, that is reasonable to me. Then, the proposed scope could be > {x86, x86_64}-{linux,mingw,cygwin} for remote debugging only. What do > you think? This makes me extremely nervous. I think it's completely wrong to distinguish local/remote debugging. Local or remote doesn't make the feature more or less safe. We should aim for local/remote feature (and experience/safeness) parity. At some point, native debugging may well always go through a "remote" server behind the scenes (like connecting to a local gdbserver that gdb itself spawns), so we could see this enablement now for remote targets as a (unintended) "trojan" for having it silently enabled for native targets at some point. Having the debug session behave differently with a native target vs a remote target is just fundamentally wrong, IMO. I don't want to have to say or explain to people that they should "debug natively to be safer". I've been background-thinking about taking a step back and understand why each use case that is sped up with this patch is slow to begin with. That is, the series jumps to a solution, but we haven't done our due diligence with analyzing the problem thoroughly, IMO. E.g., for the disassembly use case, presented in the v1 series, I'd think that the problem is that GDB is fetching data off the target instruction by instruction, instead of requesting a block of memory and working with that. More aggressive over fetching could be a better/safer approach. We have similar infrastructure already, in dcache.c -- we use it for stack memory nowadays, and if the memory region is marked as cacheable. We used to support caching more than just stack, but that was never enabled by default because it may not be safe to read memory outside of the range the caller is specifying, because of things like memory mapped registers, etc. (In the case of stack, we assume stack is allocated in page chunks, so that dcache never steps on memory is should not). But in cases like disassembly, we're being driven by debug info or user input. As GDB knows upfront the whole range of memory it'll be fetching, accessing a bigger chunk upfront, as long as it doesn't step out of the range we read piecemeal anyway, should have no effect on correctness. Another orthogonal idea (we could/should have both) would be to check whether sections/blocks/pages of memory haven't been changed in the target with target_verify_memory/qCRC packet, and iff not, fall back to reading from the file. IOW, don't fallback to reading from file if the memory has been changed in the target (or if we can't tell). These things could in addition be predicated on whether the target is completely stopped (we have a crude version of that today in prepare_execute_command). -- Pedro Alves