Re: [PATCH v9] amd64-mpx: initialize BND register before performing inferior calls.

["Tedeschi, Walfred" <walfred.tedeschi@intel.com>]

On 03/07/2017 12:18 PM, Pedro Alves wrote:
> Hi Walfred,
>
>>     /* Determine the total space required for arguments and struct
>>        return address in a first pass (allowing for 16-byte-aligned
>>        arguments), then push arguments in a second pass.  */
>> diff --git a/gdb/i387-tdep.c b/gdb/i387-tdep.c
>> index c986e39..4ce1208 100644
>> --- a/gdb/i387-tdep.c
>> +++ b/gdb/i387-tdep.c
>> @@ -1860,3 +1860,20 @@ i387_return_value (struct gdbarch *gdbarch, struct regcache *regcache)
>>     regcache_raw_write_unsigned (regcache, I387_FTAG_REGNUM (tdep), 0x3fff);
>>   
>>   }
>> +
>> +  /* See i387-tdep.h.  */
> Please fix indentation here.
>
>> +
>> +void
>> +i387_reset_bnd_regs (struct gdbarch *gdbarch, struct regcache *regcache)
>> +{
> In the test:
>
>> +
>> +int
>> +lower (int *a, int *b, int *c, int *d, int len)
>> +{
>> +  int value;
>> +
>> +  value = *(a - len);
>> +  value = *(b - len);
>> +  value = *(c - len);
>> +  value = *(d - len);
>> +
>> +
> Spurious double empty line.
>
>> +  value = value - *a + 1;
>> +  return value;
>> +}
>> +
>> +
>> +char
>> +char_upper (char *str, int lenght)
>> +{
>> +  char ch;
>> +  ch = *(str + lenght);
> More "lenght" typos.  Please do a global search/replace in the patch.
>
>> +
>> +  return ch;
>> +}
>> +
>
>> +int
>> +main (void)
>> +{
>> +  if (have_mpx ())
>> +    {
>> +      int sa[ARRAY_LENGTH];
>> +      int sb[ARRAY_LENGTH];
>> +      int sc[ARRAY_LENGTH];
>> +      int sd[ARRAY_LENGTH];
>> +      int *x, *a, *b, *c, *d;
>> +      char mchar;
>> +      char hello[] = "Hello";
>> +
>> +      x = malloc (sizeof (int) * ARRAY_LENGTH);
>> +      a = malloc (sizeof (int) * ARRAY_LENGTH);
>> +      b = malloc (sizeof (int) * ARRAY_LENGTH);
>> +      c = malloc (sizeof (int) * ARRAY_LENGTH);
>> +      d = malloc (sizeof (int) * ARRAY_LENGTH);
>> +
>> +      *x = upper (sa, sb, sc, sd, 0);  /* bkpt 1.  */
>> +      *x = lower (a, b, c, d, 0);  /* bkpt 1.  */
> Two lines with "bkpt 1" doesn't look right.  The second one should
> probably be removed.
>
>> +
>> +      mchar = char_upper (hello, 10);
>> +      mchar = char_lower (hello, 10);
>> +
>> +      free (x);
>> +      free (a);
>> +      free (b);
>> +      free (c);
>> +      free (d);
>> +    }
>> +  return 0; /* bkpt 3.  */
> This "bkpt 3" marker isn't used anywhere AFAICS.  Remove it.
>
>> +}
>
>> +gdb_test_multiple "print have_mpx ()" "have mpx" {
>                                            ^^^^^^^^
>
> This ...
>
>> +    -re ".*= 1\r\n$gdb_prompt " {
>> +        pass "check whether processor supports MPX"
>                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>
> ... and this test message are inconsistent.  See below.
>
>> +    }
>> +    -re ".*= 0\r\n$gdb_prompt " {
>> +        untested "processor does not support MPX; skipping tests"
>> +        return
>> +    }
>> +}
>> +
> Write:
>
> set test "check whether processor supports MPX"
> gdb_test_multiple "print have_mpx ()" $test {
>      -re ".*= 1\r\n$gdb_prompt " {
>          pass $test
>      }
>      -re ".*= 0\r\n$gdb_prompt " {
>          pass $test
>          untested "processor does not support MPX; skipping tests"
>          return
>      }
> }
>
> OK with those changes.
>
> Thanks,
> Pedro Alves
>
Hello Pedro,

Thanks a lot!

Best regards,
/Fred
Intel Deutschland GmbH
Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Christin Eisenschmid, Christian Lamprechter
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928