public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: Pedro Alves <palves@redhat.com>
To: Yao Qi <qiyaoltc@gmail.com>
Cc: gdb-patches@sourceware.org
Subject: Re: [PATCH] Clear *VAL in regcache_raw_read_unsigned
Date: Wed, 10 Feb 2016 17:36:00 -0000	[thread overview]
Message-ID: <56BB7512.2030507@redhat.com> (raw)
In-Reply-To: <86a8n8qxyp.fsf@gmail.com>

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

  reply	other threads:[~2016-02-10 17:36 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-02-09 14:54 Yao Qi
2016-02-10 16:45 ` Yao Qi
2016-02-10 16:52   ` Pedro Alves
2016-02-10 17:25     ` Yao Qi
2016-02-10 17:36       ` Pedro Alves [this message]
2016-02-11 10:12         ` Yao Qi
2016-02-11 12:46           ` Pedro Alves
2016-02-11 15:15             ` Yao Qi
2016-02-11 15:51               ` Pedro Alves
2016-02-11 16:32                 ` Antoine Tremblay
2016-02-11 17:00                 ` Yao Qi
2016-02-11 17:24                   ` Pedro Alves
2016-02-12 11:15                     ` Yao Qi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=56BB7512.2030507@redhat.com \
    --to=palves@redhat.com \
    --cc=gdb-patches@sourceware.org \
    --cc=qiyaoltc@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).