From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 103833 invoked by alias); 13 Jul 2017 09:04:09 -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 102616 invoked by uid 89); 13 Jul 2017 09:04:08 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=0.6 required=5.0 tests=BAYES_00,FREEMAIL_FROM,RCVD_IN_DNSWL_NONE,RCVD_IN_SORBS_SPAM,SPF_PASS,UNSUBSCRIBE_BODY autolearn=no version=3.3.2 spammy= X-HELO: mail-it0-f43.google.com Received: from mail-it0-f43.google.com (HELO mail-it0-f43.google.com) (209.85.214.43) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 13 Jul 2017 09:04:05 +0000 Received: by mail-it0-f43.google.com with SMTP id m84so38045966ita.0 for ; Thu, 13 Jul 2017 02:04:05 -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:cc:subject:references:date:in-reply-to :message-id:user-agent:mime-version:content-transfer-encoding; bh=TGmXtAGITk6jP/VtjzdGLfX5G1EMM1aAEFcMn0Os49U=; b=CI5uhbU5Yabsbl1TKK7zcAhwOnD44XywDDLlXhVXLkAtbj4XL3Q4juezqC53nnyq6K 1fbZ/T/TLUyhHMHKG3Fp8Xhkpsxj2BolI6UEAY1gzfEijt+YJhEosJTaYx++bisEGdYs +uYBHd3BgVrL+T1MbPApoLAnPeDE/bzQgRorzahv2y8aJUbqMfp8WnBkMlyGk8QRKt84 wqRkADjL/SD9miLCP14fnH/w2J1MYJr8O4EYn5Ya2wrtDPXv8Ds6FtMIXBSHYyDlW2S4 cefyPug+iZkg+NNPrzdndQ/z0JRrqGZgFQmJctXh3EdLtzRoPsyWgW5MxuE4TRxb3C2F XnzA== X-Gm-Message-State: AIVw1139bs+5y3T+k1/phpZp5+RwUHBGwtxODrixKjOImjV9mrLyskgx z9nKFwE7OW/zA+N8 X-Received: by 10.36.41.67 with SMTP id p64mr19389039itp.14.1499936644073; Thu, 13 Jul 2017 02:04:04 -0700 (PDT) Received: from E107787-LIN (static.42.136.251.148.clients.your-server.de. [148.251.136.42]) by smtp.gmail.com with ESMTPSA id s68sm2562315ita.29.2017.07.13.02.04.02 (version=TLS1_2 cipher=AES128-SHA bits=128/128); Thu, 13 Jul 2017 02:04:03 -0700 (PDT) From: Yao Qi To: Alan Hayward Cc: "gdb-patches\@sourceware.org" , nd Subject: Re: [RFC] Replace regcache readonly flag with detached flag References: Date: Thu, 13 Jul 2017 09:04:00 -0000 In-Reply-To: (Alan Hayward's message of "Wed, 5 Jul 2017 14:54:43 +0000") Message-ID: <8637a0r9mq.fsf@gmail.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes X-SW-Source: 2017-07/txt/msg00127.txt.bz2 Alan Hayward writes: > Therefore I'd like to propose removing m_readonly_p and replacing it with: > > /* Is this a detached cache? A detached cache is not attached to a tar= get. > It is used for saving the target's register state (e.g, across an in= ferior > function call or just before forcing a function return). A detached = cache > can be written to and read from, however the values will not be pass= ed > through to a target. > Using the copy constructor or regcache_dup on a regcache will always > create a detached regcache. */ > bool m_detached_p; > > In most cases this is a 1:1 substitution of m_readonly_p for m_detached_p, > except it for the write functions, where we now allow writing to the > regcache buffers. I am not sure this replacement is reasonable. The regcache can be detached from target, and read-only or read-write. The regcache can be attached to target, and read-write. I can't think of a case that regcache is attached to target and read-only. > > I've attached a patch below to show this in action. > > If people are still against removing the readonly flag, then there is the > option of re-introducing readonly as an additional flag which can optiona= lly > be set when calling the copy constructor or regcache_dup. > This would have the advantage of extra security of protecting against any > accidental writes to detached caches we really don't want to change. > A regcache would then have both a m_detached_p and m_readonly_p. > Yes, regcache has these two orthogonal attributes. However, adding a new m_readonly_p makes regcache even more complicated. > In a previous email ("Re: [PATCH] Replace regbuf with regcache in record-= full.c"), > Yao made the suggestion of splitting the regcache into a detached regcache > and an attached regcache that subclasses the detached regcache. The probl= em > with this approach is it adds a whole lot of complexity, we still > probably need What is the complexity? I thought my suggestion simplified regcache. regcache now has ~29 public methods, and there are two groups of apis which are not related to the other (read/write vs supply/collect). If we split them, each class has ~15 public methods, it improves the readability, IMO. What is more, the class regcache_detached can be propagated and "simplify" other part of GDB, like use it in target_ops.to_{fetch,store}_regsters, so that it enforces all target layer implementation only use supply/collect methods. IMO, using an object having ~15 public methods is simpler than using an object having ~29 public methods. To be clear, this is one benefit of splitting regcache, but you don't have to do this. > to keep the bool flags for safety checks, and it would be very easy > for the old We don't need that bool flag m_detached_p in my suggestion. > "non-class" regcache_ functions (eg regcache_raw_write) to accidentally c= ast to > the wrong class. > Compiler has the conversion check, xxx.c:123:12: error: invalid conversion from =E2=80=98regcache_1*=E2=80=99 = to =E2=80=98regcache*=E2=80=99 [-fpermissive] unless static_cast is used, but that is wrong. > For the sake of verbosity, the current regcache read/writes work as follo= ws: > > raw_read - If !readonly, update from target to regcache. Read from regcac= he. > raw_write - Assert !readonly. Write to regcache. Write to target. > raw_collect - Read from regcache. > raw_supply - Assert !readonly. Write to regcache. > cooked_read - If raw register, raw_read. Elif readonly read from regcache. > Else create pseudo from multiple raw_reads. > cooked_write - Assert !readonly. If raw register, raw_write. > Else split pseudo using multiple raw_writes. > > After this suggested change: > > raw_read - If !detached, update from target to regcache. Read from regcac= he. > raw_write - Write to regcache. If !detached, Write to target. > raw_collect - Read from regcache. > raw_supply - Write to regcache. > cooked_read - If raw register, raw_read. Elif detached read from regcache. > Else create pseudo from multiple raw_reads. > cooked_write - If raw register, raw_write. > Else split pseudo using multiple raw_writes. > If regcache is detached, the class doesn't have {raw,cooked}_{read,write}_ methods at all. It only has collect and supply methods. http://people.linaro.org/~yao.qi/gdb/doxy/regcache-split/gdb-xref/classregc= ache__1.html the "regcache" is the attached one, inherited from the detached regcache, with new {raw,cooked}_{read,write}_ methods added. http://people.linaro.org/~yao.qi/gdb/doxy/regcache-split/gdb-xref/classregc= ache.html > After this suggested change with additional readonly change: > > raw_read - If !detached, update from target to regcache. Read from regcac= he. > raw_write - Assert !readonly. Write to regcache. If !detached, Write to t= arget. > raw_collect - Read from regcache. > raw_supply - Assert !readonly. Write to regcache. > cooked_read - If raw register, raw_read. Elif detached read from regcache. > Else create pseudo from multiple raw_reads. > cooked_write - Assert !readonly. If raw register, raw_write. > Else split pseudo using multiple raw_writes. > --=20 Yao (=E9=BD=90=E5=B0=A7)