From patchwork Thu Feb 11 10:12:36 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Yao Qi X-Patchwork-Id: 10817 Received: (qmail 112746 invoked by alias); 11 Feb 2016 10:12:47 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 112733 invoked by uid 89); 11 Feb 2016 10:12:47 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.6 required=5.0 tests=AWL, BAYES_00, FREEMAIL_FROM, RCVD_IN_DNSWL_LOW, SPF_PASS autolearn=ham version=3.3.2 spammy=Hx-languages-length:3442 X-HELO: mail-pa0-f53.google.com Received: from mail-pa0-f53.google.com (HELO mail-pa0-f53.google.com) (209.85.220.53) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Thu, 11 Feb 2016 10:12:45 +0000 Received: by mail-pa0-f53.google.com with SMTP id fl4so14877307pad.0 for ; Thu, 11 Feb 2016 02:12:45 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:references:date:in-reply-to :message-id:user-agent:mime-version:content-type :content-transfer-encoding; bh=8TT+K4VBxjK+htxC8ZFN2pi0NPSstzpnLOMUsBrAoLk=; b=eNXltUUo0XDn9jkO06ui1f8LHAqmBDIsDpX1SrNyRcc+hVs7OfNH3aAojyFVy5nGS5 FVfl7kl9ahMB8nTdJPr7Rlk+RY1izrQUjPsgOvkhJxgSFDqCWhnX9q4h+O0/SAw4LsBR q2DMSnsDKIPxJ7hykWsFhWLxy6CHtlNrlJq+ONEW4GNrHF1xjgvX75BhkQOu8aGHihph 0mtf5t89OoDmJgWTJxtb2gX4BZayZB5nOdCI47S+dsIt0uejSLJXOEheT3B2YfaZCdj2 jAaChWezKHYzfV6I0HplMSh6+vnMsPPvWvrRXEWBxrg9IMl4JnrUoAAHEV2TsgfmjgeH Xmww== X-Gm-Message-State: AG10YOQny9eWRLF06Bm3lDAz/nBiMMLMqcO5x1IGGrTV/VrmTNvJW7kzsQ5t6VDqBXR1xw== X-Received: by 10.66.155.167 with SMTP id vx7mr65523037pab.109.1455185564205; Thu, 11 Feb 2016 02:12:44 -0800 (PST) Received: from E107787-LIN (gcc1-power7.osuosl.org. [140.211.15.137]) by smtp.gmail.com with ESMTPSA id tv6sm11164918pab.4.2016.02.11.02.12.38 (version=TLS1_2 cipher=AES128-SHA bits=128/128); Thu, 11 Feb 2016 02:12:43 -0800 (PST) From: Yao Qi To: Pedro Alves Cc: Yao Qi , gdb-patches@sourceware.org Subject: Re: [PATCH] Clear *VAL in regcache_raw_read_unsigned References: <1455029644-6197-1-git-send-email-yao.qi@linaro.org> <86egckqztq.fsf@gmail.com> <56BB6ADB.6070909@redhat.com> <86a8n8qxyp.fsf@gmail.com> <56BB7512.2030507@redhat.com> Date: Thu, 11 Feb 2016 10:12:36 +0000 In-Reply-To: <56BB7512.2030507@redhat.com> (Pedro Alves's message of "Wed, 10 Feb 2016 17:36:18 +0000") Message-ID: <8660xvr1wr.fsf@gmail.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) MIME-Version: 1.0 X-IsSubscribed: yes Pedro Alves writes: Hi Pedro, > The issue you noticed exposed that regcache_raw_read_unsigned function > is broken for memcpy'ing a 32-bit value into a 64-bit variable, and I think > your patch papered over the problem for little endian, only. regcache_raw_read_unsigned has two orthogonal issues, one is VAL isn't cleared, and the other one is the endianness issue. My "Clear *VAL" patch fixes the former, and I didn't realize the latter then. > > enum register_status > regcache_raw_read_unsigned (struct regcache *regcache, int regnum, > ULONGEST *val) > { > int size; > > gdb_assert (regcache != NULL); > gdb_assert (regnum >= 0 && regnum < regcache->tdesc->num_registers); > > size = register_size (regcache->tdesc, regnum); > > if (size > (int) sizeof (ULONGEST)) > error (_("That operation is not available on integers of more than" > "%d bytes."), > (int) sizeof (ULONGEST)); > > collect_register (regcache, regnum, val); > > return REG_VALID; > } > > VAL is 64-bit. collect_register () writes directly into VAL, but it > only writes 32-bits. On little endian, that will leave the upper halve > of VAL as garbage. So your patch clears that. But on big endian, > that collect_register() call writes into the upper halve of VAL, and > the result is that VAL now has the wrong value. On big endian, we need to clear VAL too, otherwise the bottom half of VAL is garbage. > > E.g., if the 32-bit register's value is supposed to be 0x11223344, > after the collect register call, *VAL ends up with 0x1122334400000000, > which happens to work for little endian, but not for big endian. > > You should be able to trigger this on an ARM system with big endian code. I don't have a big endian system on my hand, but my patch below, which is copied from ppc_collect_ptrace_register, should fix the endianess problem. diff --git a/gdb/gdbserver/regcache.c b/gdb/gdbserver/regcache.c index 2af8e24..d69ed5b 100644 --- a/gdb/gdbserver/regcache.c +++ b/gdb/gdbserver/regcache.c @@ -21,6 +21,8 @@ #include "gdbthread.h" #include "tdesc.h" #include "rsp-low.h" +#include + #ifndef IN_PROCESS_AGENT struct regcache * @@ -441,7 +443,27 @@ regcache_raw_read_unsigned (struct regcache *regcache, int regnum, (int) sizeof (ULONGEST)); *val = 0; - collect_register (regcache, regnum, val); + + if (__BYTE_ORDER == __LITTLE_ENDIAN) + { + /* Little-endian values always sit at the left end of the buffer. */ + collect_register (regcache, regnum, val); + } + else if (__BYTE_ORDER == __BIG_ENDIAN) + { + /* Big-endian values sit at the right end of the buffer. In case of + registers whose sizes are smaller than sizeof (long), we must use a + padding to access them correctly. */ + int size = register_size (regcache->tdesc, regnum); + + if (size < sizeof (ULONGEST)) + { + collect_register (regcache, regnum, + (char *) val + sizeof (ULONGEST) - size); + } + else + collect_register (regcache, regnum, val); + } return REG_VALID; }