Message ID | 1455029644-6197-1-git-send-email-yao.qi@linaro.org |
---|---|
State | New, archived |
Headers |
Received: (qmail 65024 invoked by alias); 9 Feb 2016 14:54:16 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: <gdb-patches.sourceware.org> List-Unsubscribe: <mailto:gdb-patches-unsubscribe-##L=##H@sourceware.org> List-Subscribe: <mailto:gdb-patches-subscribe@sourceware.org> List-Archive: <http://sourceware.org/ml/gdb-patches/> List-Post: <mailto:gdb-patches@sourceware.org> List-Help: <mailto:gdb-patches-help@sourceware.org>, <http://sourceware.org/ml/#faqs> Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 65002 invoked by uid 89); 9 Feb 2016 14:54:15 -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=clearing X-HELO: mail-pa0-f52.google.com Received: from mail-pa0-f52.google.com (HELO mail-pa0-f52.google.com) (209.85.220.52) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Tue, 09 Feb 2016 14:54:14 +0000 Received: by mail-pa0-f52.google.com with SMTP id cy9so92597044pac.0 for <gdb-patches@sourceware.org>; Tue, 09 Feb 2016 06:54:14 -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:subject:date:message-id; bh=3nnPnxY8faw52xQJt8nLUzH3icwXtmQ8OwJJ5XEHbE4=; b=UhTlufIsGJKPhBSoIwC4+K5Of5ppbzVaCT5m7UoO87tiKvjCRWiYQqKnsMqiYuz8np BwACHxESmxvUr+6Euy9+R+zwhdNcIQtuW1h68b4Aor2ezCZCXOoB9+qwugCtto0t8TuQ G+JVzglirgJt+6XQ4M82qFX9rDeDFgpF4xQFhRAsRvTj7nKulO/CEsbzlLhtBe3asj1p TIjjyTX8zkg5g2QcVLKq02JMxzO3yh8l9mj0AXN0IXi0g3dWvrfY0J7TFSQt4tTMA7Pn TBOdm8JMAhXtxC0eacHgpvejcMImC6GHbPINJcjhKPvAITnG6PvXcw929b3wvSzl+iZ5 u+SQ== X-Gm-Message-State: AG10YOTuUN820d3pyIiB9dxicwakN3o3Jy6sfP+/1eOJ5UABh0+kO1NqEDM3j+L4NHrnUg== X-Received: by 10.66.216.7 with SMTP id om7mr50617369pac.90.1455029653119; Tue, 09 Feb 2016 06:54:13 -0800 (PST) Received: from E107787-LIN.cambridge.arm.com (gcc1-power7.osuosl.org. [140.211.15.137]) by smtp.gmail.com with ESMTPSA id i23sm51260987pfj.68.2016.02.09.06.54.11 for <gdb-patches@sourceware.org> (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Tue, 09 Feb 2016 06:54:12 -0800 (PST) From: Yao Qi <qiyaoltc@gmail.com> X-Google-Original-From: Yao Qi <yao.qi@linaro.org> To: gdb-patches@sourceware.org Subject: [PATCH] Clear *VAL in regcache_raw_read_unsigned Date: Tue, 9 Feb 2016 14:54:04 +0000 Message-Id: <1455029644-6197-1-git-send-email-yao.qi@linaro.org> X-IsSubscribed: yes |
Commit Message
Yao Qi
Feb. 9, 2016, 2:54 p.m. UTC
We have function regcache_raw_read_unsigned defined in both GDB and GDBserver, so that it is used in common like this, ULONGEST value; status = regcache_raw_read_unsigned (regcache, regnum, &value); 'value' is correctly set in GDB side, but may not be correctly set in GDBserver, because &value is passed in regcache_raw_read_unsigned but collect_register may only set part of the whole variable. In my test, I see the top half of 'value' is garbage. This patch fixes this problem by clearing *VAL before calling collect_register. Regression tests are still running. I'll push it in if there is no regression in tests. gdb/gdbserver: 2016-02-09 Yao Qi <yao.qi@linaro.org> * regcache.c (regcache_raw_read_unsigned): Clear *VAL. --- gdb/gdbserver/regcache.c | 1 + 1 file changed, 1 insertion(+)
Comments
Yao Qi <qiyaoltc@gmail.com> writes: > Regression tests are still running. I'll push it in if there is no > regression in tests. > > gdb/gdbserver: > > 2016-02-09 Yao Qi <yao.qi@linaro.org> > > * regcache.c (regcache_raw_read_unsigned): Clear *VAL. Regression test on arm-linux is done. I push it in to both master and 7.11 branch. Note that this function is only used for software single step on arm-linux, so I run tests for arm-linux target.
On 02/10/2016 04:45 PM, Yao Qi wrote: > Yao Qi <qiyaoltc@gmail.com> writes: > >> Regression tests are still running. I'll push it in if there is no >> regression in tests. >> >> gdb/gdbserver: >> >> 2016-02-09 Yao Qi <yao.qi@linaro.org> >> >> * regcache.c (regcache_raw_read_unsigned): Clear *VAL. > > Regression test on arm-linux is done. I push it in to both master and > 7.11 branch. Note that this function is only used for software single > step on arm-linux, so I run tests for arm-linux target. Isn't this broken on big endian? AFAICS, we're reading 32-bits into the higher 32-bits of a 64-bit variable. Thanks, Pedro Alves
Pedro Alves <palves@redhat.com> writes: Hi Pedro, > Isn't this broken on big endian? AFAICS, we're reading 32-bits into > the higher 32-bits of a 64-bit variable. What do you mean by "this"? IIUC, "this" means regcache_raw_read_unsigned, rather than my patch. My patch just clears *VAL before passing it to collect_register, so it shouldn't break anything (I hope) on big endian.
On 02/10/2016 05:25 PM, Yao Qi wrote: > Pedro Alves <palves@redhat.com> writes: > > Hi Pedro, > >> Isn't this broken on big endian? AFAICS, we're reading 32-bits into >> the higher 32-bits of a 64-bit variable. > > What do you mean by "this"? IIUC, "this" means > regcache_raw_read_unsigned, rather than my patch. My patch just clears > *VAL before passing it to collect_register, so it shouldn't break anything > (I hope) on big endian. 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. 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. 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. Thanks, Pedro Alves
diff --git a/gdb/gdbserver/regcache.c b/gdb/gdbserver/regcache.c index 6a737ea..2af8e24 100644 --- a/gdb/gdbserver/regcache.c +++ b/gdb/gdbserver/regcache.c @@ -440,6 +440,7 @@ regcache_raw_read_unsigned (struct regcache *regcache, int regnum, "%d bytes."), (int) sizeof (ULONGEST)); + *val = 0; collect_register (regcache, regnum, val); return REG_VALID;