OpenE2K/binutils-gdb
12
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

GDBserver: Fix "Cond. jump or move depends on uninit value" in x87 code

Browse Source 
Running gdbserver under Valgrind I get:

  ==26925== Conditional jump or move depends on uninitialised value(s)
  ==26925==    at 0x473E7F: i387_cache_to_xsave(regcache*, void*) (i387-fp.c:579)
  ==26925==    by 0x46E3ED: x86_fill_xstateregset(regcache*, void*) (linux-x86-low.c:418)
  ==26925==    by 0x45E747: regsets_store_inferior_registers(regsets_info*, regcache*) (linux-low.c:5456)
  ==26925==    by 0x45EEF8: linux_store_registers(regcache*, int) (linux-low.c:5731)
  ==26925==    by 0x426441: regcache_invalidate_thread(thread_info*) (regcache.c:89)
  ==26925==    by 0x45CCAF: linux_resume_one_lwp_throw(lwp_info*, int, int, siginfo_t*) (linux-low.c:4447)
  ==26925==    by 0x45CE2A: linux_resume_one_lwp(lwp_info*, int, int, siginfo_t*) (linux-low.c:4519)
  ==26925==    by 0x45E17C: proceed_one_lwp(thread_info*, lwp_info*) (linux-low.c:5216)
  ==26925==    by 0x45DC81: linux_resume_one_thread(thread_info*, bool) (linux-low.c:5031)
  ==26925==    by 0x45DD34: linux_resume(thread_resume*, unsigned long)::{lambda(thread_info*)#2}::operator()(thread_info*) const (linux-low.c:5095)
  ==26925==    by 0x462907: void for_each_thread<linux_resume(thread_resume*, unsigned long)::{lambda(thread_info*)#2}>(linux_resume(thread_resume*, unsigned long)::{lambda(thread_info*)#2}) (gdbthread.h:150)
  ==26925==    by 0x45DE62: linux_resume(thread_resume*, unsigned long) (linux-low.c:5093)
  ==26925==
  ==26925== Conditional jump or move depends on uninitialised value(s)
  ==26925==    at 0x473EBD: i387_cache_to_xsave(regcache*, void*) (i387-fp.c:586)
  ==26925==    by 0x46E3ED: x86_fill_xstateregset(regcache*, void*) (linux-x86-low.c:418)
  ==26925==    by 0x45E747: regsets_store_inferior_registers(regsets_info*, regcache*) (linux-low.c:5456)
  ==26925==    by 0x45EEF8: linux_store_registers(regcache*, int) (linux-low.c:5731)
  ==26925==    by 0x426441: regcache_invalidate_thread(thread_info*) (regcache.c:89)
  ==26925==    by 0x45CCAF: linux_resume_one_lwp_throw(lwp_info*, int, int, siginfo_t*) (linux-low.c:4447)
  ==26925==    by 0x45CE2A: linux_resume_one_lwp(lwp_info*, int, int, siginfo_t*) (linux-low.c:4519)
  ==26925==    by 0x45E17C: proceed_one_lwp(thread_info*, lwp_info*) (linux-low.c:5216)
  ==26925==    by 0x45DC81: linux_resume_one_thread(thread_info*, bool) (linux-low.c:5031)
  ==26925==    by 0x45DD34: linux_resume(thread_resume*, unsigned long)::{lambda(thread_info*)#2}::operator()(thread_info*) const (linux-low.c:5095)
  ==26925==    by 0x462907: void for_each_thread<linux_resume(thread_resume*, unsigned long)::{lambda(thread_info*)#2}>(linux_resume(thread_resume*, unsigned long)::{lambda(thread_info*)#2}) (gdbthread.h:150)
  ==26925==    by 0x45DE62: linux_resume(thread_resume*, unsigned long) (linux-low.c:5093)

The problem is a type/width mismatch in code like this, in
gdbserver/i387-fp.c:

  /* Some registers are 16-bit.  */
  collect_register_by_name (regcache, "fctrl", &val);
  fp->fctrl = val;

In the above code:

 #1 - 'val' is a 64-bit unsigned long.

 #2 - "fctrl" is 32-bit in the register cache, thus half of 'val' is
      left uninitialized by collect_register_by_name, which works with
      an untyped raw buffer output (i.e., void*).

 #3 - fp->fctrl is an unsigned short (16-bit).  For some such
      registers we're masking off the uninitialized bits with 0xffff,
      but not in all cases.

We end up in such a fragile situation because
collect_registers_by_name works with an untyped output buffer pointer,
making it easy to pass a pointer to a variable of the wrong size.

Fix this by using regcache_raw_get_unsigned instead (actually a new
regcache_raw_get_unsigned_by_name wrapper), which always returns a
zero-extended ULONGEST register value.  It ends up simplifying the
i387-tdep.c code a bit, even.

gdb/gdbserver/ChangeLog:
2018-07-11  Pedro Alves  <palves@redhat.com>

	* i387-fp.c (i387_cache_to_fsave, cache_to_fxsave)
	(i387_cache_to_xsave): Use regcache_raw_get_unsigned_by_name
	instead of collect_register_by_name.
	* regcache.c (regcache_raw_get_unsigned_by_name): New.
	* regcache.h (regcache_raw_get_unsigned_by_name): New.
This commit is contained in:
Pedro Alves 2018-07-11 19:49:19 +01:00
parent c597cc3d6e
commit cb19713281
4 changed files with 50 additions and 46 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
gdb/gdbserver/ChangeLog
View File

@ -1,3 +1,11 @@
2018-07-11 Pedro Alves <palves@redhat.com>
* i387-fp.c (i387_cache_to_fsave, cache_to_fxsave)
(i387_cache_to_xsave): Use regcache_raw_get_unsigned_by_name
instead of collect_register_by_name.
* regcache.c (regcache_raw_get_unsigned_by_name): New.
* regcache.h (regcache_raw_get_unsigned_by_name): New.
2018-07-04 Vyacheslav Barinov <v.barinov@samsung.com>
Pedro Alves <palves@redhat.com>

71
gdb/gdbserver/i387-fp.c
View File

@ -149,38 +149,25 @@ i387_cache_to_fsave (struct regcache *regcache, void *buf)
struct i387_fsave *fp = (struct i387_fsave *) buf;
int i;
int st0_regnum = find_regno (regcache->tdesc, "st0");
unsigned long val, val2;
unsigned long val2;
for (i = 0; i < 8; i++)
collect_register (regcache, i + st0_regnum,
((char *) &fp->st_space[0]) + i * 10);
collect_register_by_name (regcache, "fioff", &fp->fioff);
collect_register_by_name (regcache, "fooff", &fp->fooff);
fp->fioff = regcache_raw_get_unsigned_by_name (regcache, "fioff");
fp->fooff = regcache_raw_get_unsigned_by_name (regcache, "fooff");
/* This one's 11 bits... */
collect_register_by_name (regcache, "fop", &val2);
val2 = regcache_raw_get_unsigned_by_name (regcache, "fop");
fp->fop = (val2 & 0x7FF) | (fp->fop & 0xF800);
/* Some registers are 16-bit. */
collect_register_by_name (regcache, "fctrl", &val);
fp->fctrl = val;
collect_register_by_name (regcache, "fstat", &val);
val &= 0xFFFF;
fp->fstat = val;
collect_register_by_name (regcache, "ftag", &val);
val &= 0xFFFF;
fp->ftag = val;
collect_register_by_name (regcache, "fiseg", &val);
val &= 0xFFFF;
fp->fiseg = val;
collect_register_by_name (regcache, "foseg", &val);
val &= 0xFFFF;
fp->foseg = val;
fp->fctrl = regcache_raw_get_unsigned_by_name (regcache, "fctrl");
fp->fstat = regcache_raw_get_unsigned_by_name (regcache, "fstat");
fp->ftag = regcache_raw_get_unsigned_by_name (regcache, "ftag");
fp->fiseg = regcache_raw_get_unsigned_by_name (regcache, "fiseg");
fp->foseg = regcache_raw_get_unsigned_by_name (regcache, "foseg");
}
void
@ -237,24 +224,20 @@ i387_cache_to_fxsave (struct regcache *regcache, void *buf)
collect_register (regcache, i + xmm0_regnum,
((char *) &fp->xmm_space[0]) + i * 16);
collect_register_by_name (regcache, "fioff", &fp->fioff);
collect_register_by_name (regcache, "fooff", &fp->fooff);
collect_register_by_name (regcache, "mxcsr", &fp->mxcsr);
fp->fioff = regcache_raw_get_unsigned_by_name (regcache, "fioff");
fp->fooff = regcache_raw_get_unsigned_by_name (regcache, "fooff");
fp->mxcsr = regcache_raw_get_unsigned_by_name (regcache, "mxcsr");
/* This one's 11 bits... */
collect_register_by_name (regcache, "fop", &val2);
val2 = regcache_raw_get_unsigned_by_name (regcache, "fop");
fp->fop = (val2 & 0x7FF) | (fp->fop & 0xF800);
/* Some registers are 16-bit. */
collect_register_by_name (regcache, "fctrl", &val);
fp->fctrl = val;
collect_register_by_name (regcache, "fstat", &val);
fp->fstat = val;
fp->fctrl = regcache_raw_get_unsigned_by_name (regcache, "fctrl");
fp->fstat = regcache_raw_get_unsigned_by_name (regcache, "fstat");
/* Convert to the simplifed tag form stored in fxsave data. */
collect_register_by_name (regcache, "ftag", &val);
val &= 0xFFFF;
val = regcache_raw_get_unsigned_by_name (regcache, "ftag");
val2 = 0;
for (i = 7; i >= 0; i--)
{
@ -265,11 +248,8 @@ i387_cache_to_fxsave (struct regcache *regcache, void *buf)
}
fp->ftag = val2;
collect_register_by_name (regcache, "fiseg", &val);
fp->fiseg = val;
collect_register_by_name (regcache, "foseg", &val);
fp->foseg = val;
fp->fiseg = regcache_raw_get_unsigned_by_name (regcache, "fiseg");
fp->foseg = regcache_raw_get_unsigned_by_name (regcache, "foseg");
}
void
@ -566,7 +546,7 @@ i387_cache_to_xsave (struct regcache *regcache, void *buf)
}
/* This one's 11 bits... */
collect_register_by_name (regcache, "fop", &val2);
val2 = regcache_raw_get_unsigned_by_name (regcache, "fop");
val2 = (val2 & 0x7FF) | (fp->fop & 0xF800);
if (fp->fop != val2)
{
@ -575,14 +555,14 @@ i387_cache_to_xsave (struct regcache *regcache, void *buf)
}
/* Some registers are 16-bit. */
collect_register_by_name (regcache, "fctrl", &val);
val = regcache_raw_get_unsigned_by_name (regcache, "fctrl");
if (fp->fctrl != val)
{
xstate_bv |= X86_XSTATE_X87;
fp->fctrl = val;
}
collect_register_by_name (regcache, "fstat", &val);
val = regcache_raw_get_unsigned_by_name (regcache, "fstat");
if (fp->fstat != val)
{
xstate_bv |= X86_XSTATE_X87;
@ -590,8 +570,7 @@ i387_cache_to_xsave (struct regcache *regcache, void *buf)
}
/* Convert to the simplifed tag form stored in fxsave data. */
collect_register_by_name (regcache, "ftag", &val);
val &= 0xFFFF;
val = regcache_raw_get_unsigned_by_name (regcache, "ftag");
val2 = 0;
for (i = 7; i >= 0; i--)
{
@ -606,14 +585,14 @@ i387_cache_to_xsave (struct regcache *regcache, void *buf)
fp->ftag = val2;
}
collect_register_by_name (regcache, "fiseg", &val);
val = regcache_raw_get_unsigned_by_name (regcache, "fiseg");
if (fp->fiseg != val)
{
xstate_bv |= X86_XSTATE_X87;
fp->fiseg = val;
}
collect_register_by_name (regcache, "foseg", &val);
val = regcache_raw_get_unsigned_by_name (regcache, "foseg");
if (fp->foseg != val)
{
xstate_bv |= X86_XSTATE_X87;

10
gdb/gdbserver/regcache.c
View File

@ -448,6 +448,16 @@ regcache_raw_read_unsigned (struct regcache *regcache, int regnum,
#ifndef IN_PROCESS_AGENT
/* See regcache.h. */
ULONGEST
regcache_raw_get_unsigned_by_name (struct regcache *regcache,
const char *name)
{
return regcache_raw_get_unsigned (regcache,
find_regno (regcache->tdesc, name));
}
void
collect_register_as_string (struct regcache *regcache, int n, char *buf)
{

7
gdb/gdbserver/regcache.h
View File

@ -131,4 +131,11 @@ void collect_register_as_string (struct regcache *regcache, int n, char *buf);
void collect_register_by_name (struct regcache *regcache,
const char *name, void *buf);
/* Read a raw register as an unsigned integer. Convenience wrapper
around regcache_raw_get_unsigned that takes a register name instead
of a register number. */
ULONGEST regcache_raw_get_unsigned_by_name (struct regcache *regcache,
const char *name);
#endif /* REGCACHE_H */