From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 21891 invoked by alias); 14 Jul 2017 15:14:35 -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 21806 invoked by uid 89); 14 Jul 2017 15:14:34 -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=cpy, fpermissive, H*r:AES128-SHA, H*r:sk:static. X-HELO: mail-it0-f52.google.com Received: from mail-it0-f52.google.com (HELO mail-it0-f52.google.com) (209.85.214.52) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 14 Jul 2017 15:14:32 +0000 Received: by mail-it0-f52.google.com with SMTP id k192so26703528ith.1 for ; Fri, 14 Jul 2017 08:14:32 -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:in-reply-to:references :user-agent:date:message-id:mime-version:content-transfer-encoding; bh=n0bMpEQrQxOW70ebTPbOVJXh04K9cNQioE4Q9r5TZMk=; b=ERXOli8Y5zHWgf4/7OwB8ja8nFFIrM7Tz4tV3B5yg1LrCJkVl1nF6q46yxOMyeuAYM GuvUr/45LnWqEa8hGaypTHG06tJjQxI9ul8sdfeE2HYcqJeD916AL0PB4yu42PhWbK7N tCMTBx6TW+Sdd0eJX5oCZMaag3medw07dMHDN05bN46SvRhVxV6Lv2AP05ye8BKpXwZC ZafH6R+npjLmLIA0U88QCHRhdp9dsaIwNY6lhDKcfZPBi/BQ3Ku6CWCeMwz6u2RwAWpu FYaA6CJ6Ab2SSDEyP57oZ2PtS5REQBb/IyO+WfZAD/MzxYVLE3JCGu1x0XbdMzD1jzF5 hTMQ== X-Gm-Message-State: AIVw113hoVv1lyYntEOLRwiWKk/ptuee0bTV+RiXDs7Z1zsE/EwxmDRt bSm9Ez7P5DHmZHCr X-Received: by 10.36.3.11 with SMTP id e11mr4321944ite.79.1500045271040; Fri, 14 Jul 2017 08:14:31 -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 88sm4718874ioi.5.2017.07.14.08.14.29 (version=TLS1_2 cipher=AES128-SHA bits=128/128); Fri, 14 Jul 2017 08:14:30 -0700 (PDT) From: Yao Qi To: Alan Hayward Cc: "gdb-patches\@sourceware.org" , nd Subject: Re: [RFC] Replace regcache readonly flag with detached flag In-Reply-To: <298BA45B-4570-4A16-9C21-95F5A068F93C@arm.com> (Alan Hayward's message of "Fri, 14 Jul 2017 09:21:11 +0000") References: <8637a0r9mq.fsf@gmail.com> <298BA45B-4570-4A16-9C21-95F5A068F93C@arm.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) Date: Fri, 14 Jul 2017 15:14:00 -0000 Message-ID: <86o9snoxtc.fsf@gmail.com> 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/msg00185.txt.bz2 Alan Hayward writes: >> Compiler has the conversion check, >>=20 >> xxx.c:123:12: error: invalid conversion from =E2=80=98regcache_1*=E2=80= =99 to >> =E2=80=98regcache*=E2=80=99 [-fpermissive] >>=20 >> unless static_cast is used, but that is wrong. > > What about the other way? Accidentally casting regcache to > regcache_1/detacted_regcache. > It doesn't break anything. > This would matter if regcache overrides any of the methods in > regcache_1/detacted_regcache. > (Which I think is ok in your code.) Yes, in my code, regcache doesn't override any methods from detached_regcache. We can even mark methods in detached_regcache "final". > > (This comment is only valid if the cooked register comment in the next > block holds) > I think regcache_cpy might be broken? > The internal check needs to move from m_readonly_p to a detached > check, as there needs to > Be different behaviour for: > cpy(regcache, regcache_1) - do a save > cpy(regcache_1, regcache_1) - do a restore > cpy(regcache, regcache) - don=E2=80=99t allow > cpy(regcache_1, regcache_1) - simple memcpy > Which I why I suggested you=E2=80=99d still need a m_detached_p to ensure > incorrect casting doesn=E2=80=99t > break the above. > > regcache_cpy is too complicated, and it doesn't have to be that complicated. The current use of regcache_cpy is that src is read-only and dst is not read-only. We can simplify regcache_cpy https://sourceware.org/ml/gdb-patches/2017-06/msg00715.html (I forgot to commit it, but good if you can review it). With my patch applied, regcache_cpy becomes dst->restore (src), and restore is a regcache method, void regcache::restore (struct regcache *src). It has nothing to do with detached_regcache. >>=20 >>> For the sake of verbosity, the current regcache read/writes work as fol= lows: >>>=20 >>> raw_read - If !readonly, update from target to regcache. Read from >>> regcache. >>> 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 regcac= he. >>> Else create pseudo from multiple raw_reads. >>> cooked_write - Assert !readonly. If raw register, raw_write. >>> Else split pseudo using multiple raw_writes. >>>=20 >>> After this suggested change: >>>=20 >>> raw_read - If !detached, update from target to regcache. Read from >>> regcache. >>> 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 regcac= he. >>> Else create pseudo from multiple raw_reads. >>> cooked_write - If raw register, raw_write. >>> Else split pseudo using multiple raw_writes. >>>=20 >>=20 >> If regcache is detached, the class doesn't have >> {raw,cooked}_{read,write}_ methods at all. It only has collect and >> supply methods. >>=20 >> http://people.linaro.org/~yao.qi/gdb/doxy/regcache-split/gdb-xref/classr= egcache__1.html >>=20 >> the "regcache" is the attached one, inherited from the detached >> regcache, with new {raw,cooked}_{read,write}_ methods added. >>=20 >> http://people.linaro.org/~yao.qi/gdb/doxy/regcache-split/gdb-xref/classr= egcache.html >>=20 > > A difference between mine and your code is the cooked registers=20 > > In your code the cooked registers are a product of readonly. > In my code the cooked registers are a product of detached. > > The regcache code does become simpler if the cooked registers are a > product of readonly. > > But, I think they need to be a product of detached. > The code says "some architectures need to save/restore `cooked' > registers that live in memory.=E2=80=9D > To me, that says it=E2=80=99s required for a regcache that isn=E2=80=99t = connected to a target. I am not sure I understand you here. Are you saying that dealing with cooked registers during save and store? In my code, save and restore are done against detached regcache. See the doxygen link for regcache_1, "save" is a public method in regcache_1, it is void regcache_1::save (regcache_cooked_read_ftype *cooked_read, void *src); and restore is a private method in regcache, void regcache::restore (struct regcache_1 *src); Note that in the doxygen html, src's type is regcache rather than regcache_1, but it can be changed easily. Overall, the meaning of save/restore is that, we save the contents (from frame, for example) to a detached regcache, and restore attached regcache from a detached one. We use detached regcache because we don't want to its contents go to target, and we still keep the freedom to mark the detached regcache read-write or read-only. In the existing uses of regcache_save, we need a read-only detached regcache, but we need a read-write detached regcache to replace record_full_core_regbuf. --=20 Yao (=E9=BD=90=E5=B0=A7)