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

[Pedro Alves <palves@redhat.com>]

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