Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* [PATCH][gdb/testsuite] Add have_mpx in lib/gdb.exp
@ 2021-01-12 10:59 Tom de Vries
  2021-01-12 15:35 ` Simon Marchi via Gdb-patches
  0 siblings, 1 reply; 3+ messages in thread
From: Tom de Vries @ 2021-01-12 10:59 UTC (permalink / raw)
  To: gdb-patches

Hi,

The sources for the test-cases gdb.arch/i386-mpx*.exp contain have_mpx
functions that test whether the processor supports mpx instructions.

OTOH, the test-cases are compiled using -mmpx -fcheck-pointer-bounds, which
instrument all functions with mpx instructions.

So, the function that is supposed to test whether mpx instruction are
supported contains mpx instructions, which is a bit odd.

We could fix this by:
- factoring out the have_mpx function into a single source file, and
- compiling it without "-mmpx -fcheck-pointer-bounds".

But having the mpx support test as part of the test-cases seems like an
unnecesary complication that makes the test-cases more difficult to analyze,
reason about and modify.

So we go one step further and factor out the mpx support test in into a
gdb_caching_proc.

Tested on x86_64-linux.

Any comments?

Thanks,
- Tom

[gdb/testsuite] Add have_mpx in lib/gdb.exp

gdb/testsuite/ChangeLog:

2021-01-12  Tom de Vries  <tdevries@suse.de>

	* gdb.arch/i386-mpx-call.c (have_mpx): Remove.
	(main): Remove call to have_mpx.
	* gdb.arch/i386-mpx-call.exp: Use have_mpx.
	* gdb.arch/i386-mpx-map.c (have_mpx): Remove.
	(main): Remote call to have_mpx.
	* gdb.arch/i386-mpx-map.exp: Use have_mpx.
	* gdb.arch/i386-mpx-sigsegv.c (have_mpx): Remove.
	(main): Remove call to have_mpx.
	* gdb.arch/i386-mpx-sigsegv.exp: Use have_mpx.
	* gdb.arch/i386-mpx-simple_segv.c (have_mpx): Remove.
	(main): Remove call to have_mpx.
	* gdb.arch/i386-mpx-simple_segv.exp: Use have_mpx.
	* gdb.arch/i386-mpx.c (have_mpx): Remove.
	(main): Remote call to have_mpx.
	* gdb.arch/i386-mpx.exp: Use have_mpx.
	* lib/gdb.exp (have_mpx): New proc.

---
 gdb/testsuite/gdb.arch/i386-mpx-call.c          | 78 +++++++-------------
 gdb/testsuite/gdb.arch/i386-mpx-call.exp        | 17 ++---
 gdb/testsuite/gdb.arch/i386-mpx-map.c           | 45 ++----------
 gdb/testsuite/gdb.arch/i386-mpx-map.exp         | 23 ++----
 gdb/testsuite/gdb.arch/i386-mpx-sigsegv.c       | 80 +++++++-------------
 gdb/testsuite/gdb.arch/i386-mpx-sigsegv.exp     | 15 ++--
 gdb/testsuite/gdb.arch/i386-mpx-simple_segv.c   | 40 ++--------
 gdb/testsuite/gdb.arch/i386-mpx-simple_segv.exp | 15 ++--
 gdb/testsuite/gdb.arch/i386-mpx.c               | 98 ++++++++-----------------
 gdb/testsuite/gdb.arch/i386-mpx.exp             | 22 ++----
 gdb/testsuite/lib/gdb.exp                       | 52 +++++++++++++
 11 files changed, 172 insertions(+), 313 deletions(-)

diff --git a/gdb/testsuite/gdb.arch/i386-mpx-call.c b/gdb/testsuite/gdb.arch/i386-mpx-call.c
index d0fe71afed4..cc37edb1d04 100644
--- a/gdb/testsuite/gdb.arch/i386-mpx-call.c
+++ b/gdb/testsuite/gdb.arch/i386-mpx-call.c
@@ -17,34 +17,10 @@
 
 #include <stdlib.h>
 #include <string.h>
-#include "x86-cpuid.h"
 
 /* Defined size for arrays.  */
 #define ARRAY_LENGTH    5
 
-unsigned int
-have_mpx (void)
-{
-  unsigned int eax, ebx, ecx, edx;
-
-  if (!__get_cpuid (1, &eax, &ebx, &ecx, &edx))
-    return 0;
-
-  if ((ecx & bit_OSXSAVE) == bit_OSXSAVE)
-    {
-      if (__get_cpuid_max (0, NULL) < 7)
-	return 0;
-
-      __cpuid_count (7, 0, eax, ebx, ecx, edx);
-
-      if ((ebx & bit_MPX) == bit_MPX)
-	return 1;
-      else
-	return 0;
-    }
-  return 0;
-}
-
 
 int
 upper (int *a, int *b, int *c, int *d, int len)
@@ -99,33 +75,31 @@ char_lower (char *str, int length)
 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);
-
-      mchar = char_upper (hello, 10);
-      mchar = char_lower (hello, 10);
-
-      free (x);
-      free (a);
-      free (b);
-      free (c);
-      free (d);
-    }
+  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);
+
+  mchar = char_upper (hello, 10);
+  mchar = char_lower (hello, 10);
+
+  free (x);
+  free (a);
+  free (b);
+  free (c);
+  free (d);
+
   return 0;
 }
diff --git a/gdb/testsuite/gdb.arch/i386-mpx-call.exp b/gdb/testsuite/gdb.arch/i386-mpx-call.exp
index ea96b998c23..360cc68295a 100644
--- a/gdb/testsuite/gdb.arch/i386-mpx-call.exp
+++ b/gdb/testsuite/gdb.arch/i386-mpx-call.exp
@@ -25,6 +25,11 @@ if { ![supports_mpx_check_pointer_bounds] } {
     return -1
 }
 
+if { ![have_mpx] } {
+    unsupported "processor does not support MPX"
+    return -1
+}
+
 set comp_flags "-mmpx -fcheck-pointer-bounds -I${srcdir}/../nat"
 
 if {[prepare_for_testing "failed to prepare" ${testfile} ${srcfile} \
@@ -37,18 +42,6 @@ if ![runto_main] {
     return -1
 }
 
-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
-    }
-}
-
 set bounds_table 0
 gdb_test_multiple "disassemble upper" "" {
     -re -wrap "bndldx.*" {
diff --git a/gdb/testsuite/gdb.arch/i386-mpx-map.c b/gdb/testsuite/gdb.arch/i386-mpx-map.c
index 527f2dacdd1..541d54aa756 100644
--- a/gdb/testsuite/gdb.arch/i386-mpx-map.c
+++ b/gdb/testsuite/gdb.arch/i386-mpx-map.c
@@ -21,41 +21,10 @@
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
 #include <stdlib.h>
-#include "x86-cpuid.h"
-
-#ifndef NOINLINE
-#define NOINLINE __attribute__ ((noinline))
-#endif
-
 #define SIZE  5
 
 typedef int T;
 
-unsigned int have_mpx (void) NOINLINE;
-
-unsigned int NOINLINE
-have_mpx (void)
-{
-  unsigned int eax, ebx, ecx, edx;
-
-  if (!__get_cpuid (1, &eax, &ebx, &ecx, &edx))
-    return 0;
-
-  if ((ecx & bit_OSXSAVE) == bit_OSXSAVE)
-    {
-      if (__get_cpuid_max (0, NULL) < 7)
-	return 0;
-
-      __cpuid_count (7, 0, eax, ebx, ecx, edx);
-
-      if ((ebx & bit_MPX) == bit_MPX)
-	return 1;
-      else
-	return 0;
-    }
-  return 0;
-}
-
 void
 foo (T *p)
 {
@@ -78,17 +47,15 @@ foo (T *p)
 int
 main (void)
 {
-  if (have_mpx ())
-    {
-      T *a = NULL;
+  T *a = NULL;
 
-      a = calloc (SIZE, sizeof (T));	/* after-decl */
+  a = calloc (SIZE, sizeof (T));	/* after-decl */
 #if defined  __GNUC__ && !defined __INTEL_COMPILER
-      __bnd_store_ptr_bounds (a, &a);
+  __bnd_store_ptr_bounds (a, &a);
 #endif
 
-      foo (a);				/* after-alloc */
-      free (a);
-    }
+  foo (a);				/* after-alloc */
+  free (a);
+
   return 0;
 }
diff --git a/gdb/testsuite/gdb.arch/i386-mpx-map.exp b/gdb/testsuite/gdb.arch/i386-mpx-map.exp
index a98194dca61..55b70d1de0f 100644
--- a/gdb/testsuite/gdb.arch/i386-mpx-map.exp
+++ b/gdb/testsuite/gdb.arch/i386-mpx-map.exp
@@ -27,6 +27,11 @@ if { ![supports_mpx_check_pointer_bounds] } {
     return -1
 }
 
+if { ![have_mpx] } {
+    unsupported "processor does not support MPX"
+    return -1
+}
+
 set comp_flags "-mmpx -fcheck-pointer-bounds -I${srcdir}/../nat/"
 
 if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile} \
@@ -39,24 +44,6 @@ if ![runto_main] {
 	return -1
 }
 
-set supports_mpx 0
-set test "probe MPX support"
-
-gdb_test_multiple "print have_mpx()" $test {
-    -re ".. = 1\r\n$gdb_prompt $" {
-        pass $test
-        set supports_mpx 1
-    }
-    -re ".. = 0\r\n$gdb_prompt $" {
-        pass $test
-    }
-}
-
-if { !$supports_mpx } {
-    unsupported "processor does not support MPX"
-    return
-}
-
 gdb_breakpoint [ gdb_get_line_number "after-decl" ]
 gdb_breakpoint [ gdb_get_line_number "after-alloc" ]
 gdb_breakpoint [ gdb_get_line_number "after-assign" ]
diff --git a/gdb/testsuite/gdb.arch/i386-mpx-sigsegv.c b/gdb/testsuite/gdb.arch/i386-mpx-sigsegv.c
index 517fb11abde..09e1b834e3e 100644
--- a/gdb/testsuite/gdb.arch/i386-mpx-sigsegv.c
+++ b/gdb/testsuite/gdb.arch/i386-mpx-sigsegv.c
@@ -15,9 +15,6 @@
    You should have received a copy of the GNU General Public License
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
-#include "x86-cpuid.h"
-#include <stdio.h>
-
 #define OUR_SIZE    5
 
 int gx[OUR_SIZE];
@@ -26,29 +23,6 @@ int gb[OUR_SIZE];
 int gc[OUR_SIZE];
 int gd[OUR_SIZE];
 
-unsigned int
-have_mpx (void)
-{
-  unsigned int eax, ebx, ecx, edx;
-
-  if (!__get_cpuid (1, &eax, &ebx, &ecx, &edx))
-    return 0;
-
-  if ((ecx & bit_OSXSAVE) == bit_OSXSAVE)
-    {
-      if (__get_cpuid_max (0, NULL) < 7)
-	return 0;
-
-      __cpuid_count (7, 0, eax, ebx, ecx, edx);
-
-      if ((ebx & bit_MPX) == bit_MPX)
-	return 1;
-      else
-	return 0;
-    }
-  return 0;
-}
-
 int
 bp1 (int value)
 {
@@ -87,34 +61,32 @@ lower (int * p, int * a, int * b, int * c, int * d, int len)
 int
 main (void)
 {
-  if (have_mpx ())
-    {
-      int sx[OUR_SIZE];
-      int sa[OUR_SIZE];
-      int sb[OUR_SIZE];
-      int sc[OUR_SIZE];
-      int sd[OUR_SIZE];
-      int *x, *a, *b, *c, *d;
-
-      x = calloc (OUR_SIZE, sizeof (int));
-      a = calloc (OUR_SIZE, sizeof (int));
-      b = calloc (OUR_SIZE, sizeof (int));
-      c = calloc (OUR_SIZE, sizeof (int));
-      d = calloc (OUR_SIZE, sizeof (int));
-
-      upper (x, a, b, c, d, OUR_SIZE + 2);
-      upper (sx, sa, sb, sc, sd, OUR_SIZE + 2);
-      upper (gx, ga, gb, gc, gd, OUR_SIZE + 2);
-      lower (x, a, b, c, d, 1);
-      lower (sx, sa, sb, sc, sd, 1);
-      bp1 (*x);
-      lower (gx, ga, gb, gc, gd, 1);
+  int sx[OUR_SIZE];
+  int sa[OUR_SIZE];
+  int sb[OUR_SIZE];
+  int sc[OUR_SIZE];
+  int sd[OUR_SIZE];
+  int *x, *a, *b, *c, *d;
+
+  x = calloc (OUR_SIZE, sizeof (int));
+  a = calloc (OUR_SIZE, sizeof (int));
+  b = calloc (OUR_SIZE, sizeof (int));
+  c = calloc (OUR_SIZE, sizeof (int));
+  d = calloc (OUR_SIZE, sizeof (int));
+
+  upper (x, a, b, c, d, OUR_SIZE + 2);
+  upper (sx, sa, sb, sc, sd, OUR_SIZE + 2);
+  upper (gx, ga, gb, gc, gd, OUR_SIZE + 2);
+  lower (x, a, b, c, d, 1);
+  lower (sx, sa, sb, sc, sd, 1);
+  bp1 (*x);
+  lower (gx, ga, gb, gc, gd, 1);
+
+  free (x);
+  free (a);
+  free (b);
+  free (c);
+  free (d);
 
-      free (x);
-      free (a);
-      free (b);
-      free (c);
-      free (d);
-    }
   return 0;
 }
diff --git a/gdb/testsuite/gdb.arch/i386-mpx-sigsegv.exp b/gdb/testsuite/gdb.arch/i386-mpx-sigsegv.exp
index 739bb8d25fc..ef8fd68486b 100644
--- a/gdb/testsuite/gdb.arch/i386-mpx-sigsegv.exp
+++ b/gdb/testsuite/gdb.arch/i386-mpx-sigsegv.exp
@@ -27,6 +27,11 @@ if { ![supports_mpx_check_pointer_bounds] } {
     return -1
 }
 
+if { ![have_mpx] } {
+    unsupported "processor does not support MPX"
+    return -1
+}
+
 set comp_flags "-mmpx -fcheck-pointer-bounds -I${srcdir}/../nat/"
 
 if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile} \
@@ -39,16 +44,6 @@ if ![runto_main] {
     return -1
 }
 
-gdb_test_multiple "print have_mpx ()" "have mpx" {
-    -re ".. = 1\r\n$gdb_prompt " {
-        pass "check whether processor supports MPX"
-    }
-    -re ".. = 0\r\n$gdb_prompt " {
-        verbose "processor does not support MPX; skipping MPX tests"
-        return
-    }
-}
-
 set u_fault [multi_line "Program received signal SIGSEGV, Segmentation fault" \
                         "Upper bound violation while accessing address $hex" \
                         "Bounds: \\\[lower = $hex, upper = $hex\\\]"]
diff --git a/gdb/testsuite/gdb.arch/i386-mpx-simple_segv.c b/gdb/testsuite/gdb.arch/i386-mpx-simple_segv.c
index 54b6f33164c..97baed74c93 100644
--- a/gdb/testsuite/gdb.arch/i386-mpx-simple_segv.c
+++ b/gdb/testsuite/gdb.arch/i386-mpx-simple_segv.c
@@ -15,34 +15,8 @@
    You should have received a copy of the GNU General Public License
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
-#include "x86-cpuid.h"
-#include <stdio.h>
-
 #define OUR_SIZE    5
 
-unsigned int
-have_mpx (void)
-{
-  unsigned int eax, ebx, ecx, edx;
-
-  if (!__get_cpuid (1, &eax, &ebx, &ecx, &edx))
-    return 0;
-
-  if ((ecx & bit_OSXSAVE) == bit_OSXSAVE)
-    {
-      if (__get_cpuid_max (0, NULL) < 7)
-	return 0;
-
-      __cpuid_count (7, 0, eax, ebx, ecx, edx);
-
-      if ((ebx & bit_MPX) == bit_MPX)
-	return 1;
-      else
-	return 0;
-    }
-  return 0;
-}
-
 void
 upper (int * p, int len)
 {
@@ -54,13 +28,9 @@ upper (int * p, int len)
 int
 main (void)
 {
-  if (have_mpx ())
-    {
-      int a = 0;			/* Dummy variable for debugging purposes.  */
-      int sx[OUR_SIZE];
-      a++;				/* register-eval.  */
-      upper (sx, OUR_SIZE + 2);
-      return sx[1];
-    }
-  return 0;
+  int a = 0;			/* Dummy variable for debugging purposes.  */
+  int sx[OUR_SIZE];
+  a++;				/* register-eval.  */
+  upper (sx, OUR_SIZE + 2);
+  return sx[1];
 }
diff --git a/gdb/testsuite/gdb.arch/i386-mpx-simple_segv.exp b/gdb/testsuite/gdb.arch/i386-mpx-simple_segv.exp
index 51b15818466..e5b2a890f3c 100644
--- a/gdb/testsuite/gdb.arch/i386-mpx-simple_segv.exp
+++ b/gdb/testsuite/gdb.arch/i386-mpx-simple_segv.exp
@@ -33,6 +33,11 @@ if { ![supports_mpx_check_pointer_bounds] } {
     return -1
 }
 
+if { ![have_mpx] } {
+    unsupported "processor does not support MPX"
+    return -1
+}
+
 set comp_flags "-mmpx -fcheck-pointer-bounds -I${srcdir}/../nat/"
 
 if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile} \
@@ -45,16 +50,6 @@ if ![runto_main] {
     return -1
 }
 
-gdb_test_multiple "print have_mpx ()" "have mpx" {
-    -re ".. = 1\r\n$gdb_prompt " {
-        pass "check whether processor supports MPX"
-    }
-    -re ".. = 0\r\n$gdb_prompt " {
-        verbose "processor does not support MPX; skipping MPX tests"
-        return
-    }
-}
-
 set violation [multi_line "Program received signal SIGSEGV, Segmentation fault" \
                           "Upper bound violation while accessing address $hex" \
                           "Bounds: \\\[lower = $hex, upper = $hex\\\]"]
diff --git a/gdb/testsuite/gdb.arch/i386-mpx.c b/gdb/testsuite/gdb.arch/i386-mpx.c
index 178c34a330c..b96da5478af 100644
--- a/gdb/testsuite/gdb.arch/i386-mpx.c
+++ b/gdb/testsuite/gdb.arch/i386-mpx.c
@@ -17,77 +17,43 @@
    You should have received a copy of the GNU General Public License
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
-#include <stdio.h>
-#include "x86-cpuid.h"
-
-#ifndef NOINLINE
-#define NOINLINE __attribute__ ((noinline))
-#endif
-
-unsigned int have_mpx (void) NOINLINE;
-
-unsigned int NOINLINE
-have_mpx (void)
-{
-  unsigned int eax, ebx, ecx, edx;
-
-  if (!__get_cpuid (1, &eax, &ebx, &ecx, &edx))
-    return 0;
-
-  if ((ecx & bit_OSXSAVE) == bit_OSXSAVE)
-    {
-      if (__get_cpuid_max (0, NULL) < 7)
-	return 0;
-
-      __cpuid_count (7, 0, eax, ebx, ecx, edx);
-
-      if ((ebx & bit_MPX) == bit_MPX)
-	return 1;
-      else
-	return 0;
-    }
-  return 0;
-}
-
 int
 main (int argc, char **argv)
 {
-  if (have_mpx ())
-    {
 #ifdef __x86_64__
-      asm ("mov $10, %rax\n\t"
-	  "mov $9, %rdx\n\t"
-	  "bndmk (%rax,%rdx), %bnd0\n\t"
-	  "mov $20, %rax\n\t"
-	  "mov $9, %rdx\n\t"
-	  "bndmk (%rax,%rdx), %bnd1\n\t"
-	  "mov $30, %rax\n\t"
-	  "mov $9, %rdx\n\t"
-	  "bndmk (%rax,%rdx), %bnd2\n\t"
-	  "mov $40, %rax\n\t"
-	  "mov $9, %rdx\n\t"
-	  "bndmk (%rax,%rdx), %bnd3\n\t"
-	  "bndstx %bnd3, (%rax) \n\t"
-	  "nop\n\t"
-         );
+  asm ("mov $10, %rax\n\t"
+       "mov $9, %rdx\n\t"
+       "bndmk (%rax,%rdx), %bnd0\n\t"
+       "mov $20, %rax\n\t"
+       "mov $9, %rdx\n\t"
+       "bndmk (%rax,%rdx), %bnd1\n\t"
+       "mov $30, %rax\n\t"
+       "mov $9, %rdx\n\t"
+       "bndmk (%rax,%rdx), %bnd2\n\t"
+       "mov $40, %rax\n\t"
+       "mov $9, %rdx\n\t"
+       "bndmk (%rax,%rdx), %bnd3\n\t"
+       "bndstx %bnd3, (%rax) \n\t"
+       "nop\n\t"
+       );
 #else
-      asm ("mov $10, %eax\n\t"
-	   "mov $9, %edx\n\t"
-	   "bndmk (%eax,%edx), %bnd0\n\t"
-	   "mov $20, %eax\n\t"
-	   "mov $9, %edx\n\t"
-	   "bndmk (%eax,%edx), %bnd1\n\t"
-	   "mov $30, %eax\n\t"
-	   "mov $9, %edx\n\t"
-	   "bndmk (%eax,%edx), %bnd2\n\t"
-	   "mov $40, %eax\n\t"
-	   "mov $9, %edx\n\t"
-	   "bndmk (%eax,%edx), %bnd3\n\t"
-	   "bndstx  %bnd3, (%eax)\n\t"
-	   "nop\n\t"
-	  );
+  asm ("mov $10, %eax\n\t"
+       "mov $9, %edx\n\t"
+       "bndmk (%eax,%edx), %bnd0\n\t"
+       "mov $20, %eax\n\t"
+       "mov $9, %edx\n\t"
+       "bndmk (%eax,%edx), %bnd1\n\t"
+       "mov $30, %eax\n\t"
+       "mov $9, %edx\n\t"
+       "bndmk (%eax,%edx), %bnd2\n\t"
+       "mov $40, %eax\n\t"
+       "mov $9, %edx\n\t"
+       "bndmk (%eax,%edx), %bnd3\n\t"
+       "bndstx  %bnd3, (%eax)\n\t"
+       "nop\n\t"
+       );
 #endif
-	asm ("nop\n\t");	/* break here.  */
-    }
+  asm ("nop\n\t");	/* break here.  */
+
   return 0;
 }
diff --git a/gdb/testsuite/gdb.arch/i386-mpx.exp b/gdb/testsuite/gdb.arch/i386-mpx.exp
index 639f7cdc3bb..a6b74ceda6b 100644
--- a/gdb/testsuite/gdb.arch/i386-mpx.exp
+++ b/gdb/testsuite/gdb.arch/i386-mpx.exp
@@ -31,6 +31,11 @@ if { ![supports_mpx_check_pointer_bounds] } {
     return -1
 }
 
+if { ![have_mpx] } {
+    unsupported "processor does not support MPX"
+    return -1
+}
+
 set comp_flags "-mmpx -fcheck-pointer-bounds -I${srcdir}/../nat/"
 
 if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile} \
@@ -43,23 +48,6 @@ if ![runto_main] {
     return -1
 }
 
-set supports_mpx 0
-set test "probe MPX support"
-gdb_test_multiple "print have_mpx()" $test {
-    -re ".. = 1\r\n$gdb_prompt $" {
-        pass $test
-        set supports_mpx 1
-    }
-    -re ".. = 0\r\n$gdb_prompt $" {
-        pass $test
-    }
-}
-
-if { !$supports_mpx } {
-    unsupported "processor does not support MPX"
-    return
-}
-
 # Test bndcfg register and bndstatus at startup
 set test_string "\\\{raw = 0x\[0-9a-f\]+, config = \\\{base = \[0-9\]+,\
 reserved = \[0-9\]+, preserved = \[0-9\]+, enabled = \[0-9\]+\\\}\\\}"
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index 140e3960a32..3a57453da04 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -7771,5 +7771,57 @@ gdb_caching_proc supports_gnuc {
     return [gdb_simple_compile $me $src object ""]
 }
 
+# Return 1 if target supports mpx, otherwise return 0.
+gdb_caching_proc have_mpx {
+    global srcdir
+
+    set me "have_mpx"
+    if { ![istarget "i?86-*-*"] && ![istarget "x86_64-*-*"] } {
+        verbose "$me: target does not support mpx, returning 0" 2
+        return 0
+    }
+
+    # Compile a test program.
+    set src {
+       #include "nat/x86-cpuid.h"
+
+        int main() {
+	  unsigned int eax, ebx, ecx, edx;
+
+	  if (!__get_cpuid (1, &eax, &ebx, &ecx, &edx))
+	    return 0;
+
+	  if ((ecx & bit_OSXSAVE) == bit_OSXSAVE)
+	    {
+	      if (__get_cpuid_max (0, (void *)0) < 7)
+		return 0;
+
+		__cpuid_count (7, 0, eax, ebx, ecx, edx);
+
+		if ((ebx & bit_MPX) == bit_MPX)
+		  return 1;
+
+	    }
+	  return 0;
+	}
+    }
+    set compile_flags "incdir=${srcdir}/.."
+    if {![gdb_simple_compile $me $src executable $compile_flags]} {
+        return 0
+    }
+
+    set result [remote_exec target $obj]
+    set status [lindex $result 0]
+    set output [lindex $result 1]
+    if { $output != "" } {
+	return 0
+    }
+
+    remote_file build delete $obj
+
+    verbose "$me:  returning $status" 2
+    return $status
+}
+
 # Always load compatibility stuff.
 load_lib future.exp

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH][gdb/testsuite] Add have_mpx in lib/gdb.exp
  2021-01-12 10:59 [PATCH][gdb/testsuite] Add have_mpx in lib/gdb.exp Tom de Vries
@ 2021-01-12 15:35 ` Simon Marchi via Gdb-patches
  2021-01-12 16:39   ` Tom de Vries
  0 siblings, 1 reply; 3+ messages in thread
From: Simon Marchi via Gdb-patches @ 2021-01-12 15:35 UTC (permalink / raw)
  To: Tom de Vries, gdb-patches



On 2021-01-12 5:59 a.m., Tom de Vries wrote:
> Hi,
> 
> The sources for the test-cases gdb.arch/i386-mpx*.exp contain have_mpx
> functions that test whether the processor supports mpx instructions.
> 
> OTOH, the test-cases are compiled using -mmpx -fcheck-pointer-bounds, which
> instrument all functions with mpx instructions.
> 
> So, the function that is supposed to test whether mpx instruction are
> supported contains mpx instructions, which is a bit odd.
> 
> We could fix this by:
> - factoring out the have_mpx function into a single source file, and
> - compiling it without "-mmpx -fcheck-pointer-bounds".
> 
> But having the mpx support test as part of the test-cases seems like an
> unnecesary complication that makes the test-cases more difficult to analyze,
> reason about and modify.
> 
> So we go one step further and factor out the mpx support test in into a
> gdb_caching_proc.
> 
> Tested on x86_64-linux.
> 
> Any comments?
> 
> Thanks,
> - Tom
> 
> [gdb/testsuite] Add have_mpx in lib/gdb.exp
> 
> gdb/testsuite/ChangeLog:
> 
> 2021-01-12  Tom de Vries  <tdevries@suse.de>
> 
> 	* gdb.arch/i386-mpx-call.c (have_mpx): Remove.
> 	(main): Remove call to have_mpx.
> 	* gdb.arch/i386-mpx-call.exp: Use have_mpx.
> 	* gdb.arch/i386-mpx-map.c (have_mpx): Remove.
> 	(main): Remote call to have_mpx.
> 	* gdb.arch/i386-mpx-map.exp: Use have_mpx.
> 	* gdb.arch/i386-mpx-sigsegv.c (have_mpx): Remove.
> 	(main): Remove call to have_mpx.
> 	* gdb.arch/i386-mpx-sigsegv.exp: Use have_mpx.
> 	* gdb.arch/i386-mpx-simple_segv.c (have_mpx): Remove.
> 	(main): Remove call to have_mpx.
> 	* gdb.arch/i386-mpx-simple_segv.exp: Use have_mpx.
> 	* gdb.arch/i386-mpx.c (have_mpx): Remove.
> 	(main): Remote call to have_mpx.
> 	* gdb.arch/i386-mpx.exp: Use have_mpx.
> 	* lib/gdb.exp (have_mpx): New proc.
> 
> ---
>  gdb/testsuite/gdb.arch/i386-mpx-call.c          | 78 +++++++-------------
>  gdb/testsuite/gdb.arch/i386-mpx-call.exp        | 17 ++---
>  gdb/testsuite/gdb.arch/i386-mpx-map.c           | 45 ++----------
>  gdb/testsuite/gdb.arch/i386-mpx-map.exp         | 23 ++----
>  gdb/testsuite/gdb.arch/i386-mpx-sigsegv.c       | 80 +++++++-------------
>  gdb/testsuite/gdb.arch/i386-mpx-sigsegv.exp     | 15 ++--
>  gdb/testsuite/gdb.arch/i386-mpx-simple_segv.c   | 40 ++--------
>  gdb/testsuite/gdb.arch/i386-mpx-simple_segv.exp | 15 ++--
>  gdb/testsuite/gdb.arch/i386-mpx.c               | 98 ++++++++-----------------
>  gdb/testsuite/gdb.arch/i386-mpx.exp             | 22 ++----
>  gdb/testsuite/lib/gdb.exp                       | 52 +++++++++++++
>  11 files changed, 172 insertions(+), 313 deletions(-)
> 
> diff --git a/gdb/testsuite/gdb.arch/i386-mpx-call.c b/gdb/testsuite/gdb.arch/i386-mpx-call.c
> index d0fe71afed4..cc37edb1d04 100644
> --- a/gdb/testsuite/gdb.arch/i386-mpx-call.c
> +++ b/gdb/testsuite/gdb.arch/i386-mpx-call.c
> @@ -17,34 +17,10 @@
>  
>  #include <stdlib.h>
>  #include <string.h>
> -#include "x86-cpuid.h"
>  
>  /* Defined size for arrays.  */
>  #define ARRAY_LENGTH    5
>  
> -unsigned int
> -have_mpx (void)
> -{
> -  unsigned int eax, ebx, ecx, edx;
> -
> -  if (!__get_cpuid (1, &eax, &ebx, &ecx, &edx))
> -    return 0;
> -
> -  if ((ecx & bit_OSXSAVE) == bit_OSXSAVE)
> -    {
> -      if (__get_cpuid_max (0, NULL) < 7)
> -	return 0;
> -
> -      __cpuid_count (7, 0, eax, ebx, ecx, edx);
> -
> -      if ((ebx & bit_MPX) == bit_MPX)
> -	return 1;
> -      else
> -	return 0;
> -    }
> -  return 0;
> -}
> -
>  
>  int
>  upper (int *a, int *b, int *c, int *d, int len)
> @@ -99,33 +75,31 @@ char_lower (char *str, int length)
>  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);
> -
> -      mchar = char_upper (hello, 10);
> -      mchar = char_lower (hello, 10);
> -
> -      free (x);
> -      free (a);
> -      free (b);
> -      free (c);
> -      free (d);
> -    }
> +  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);
> +
> +  mchar = char_upper (hello, 10);
> +  mchar = char_lower (hello, 10);
> +
> +  free (x);
> +  free (a);
> +  free (b);
> +  free (c);
> +  free (d);
> +
>    return 0;
>  }
> diff --git a/gdb/testsuite/gdb.arch/i386-mpx-call.exp b/gdb/testsuite/gdb.arch/i386-mpx-call.exp
> index ea96b998c23..360cc68295a 100644
> --- a/gdb/testsuite/gdb.arch/i386-mpx-call.exp
> +++ b/gdb/testsuite/gdb.arch/i386-mpx-call.exp
> @@ -25,6 +25,11 @@ if { ![supports_mpx_check_pointer_bounds] } {
>      return -1
>  }
>  
> +if { ![have_mpx] } {
> +    unsupported "processor does not support MPX"
> +    return -1
> +}
> +
>  set comp_flags "-mmpx -fcheck-pointer-bounds -I${srcdir}/../nat"
>  
>  if {[prepare_for_testing "failed to prepare" ${testfile} ${srcfile} \
> @@ -37,18 +42,6 @@ if ![runto_main] {
>      return -1
>  }
>  
> -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
> -    }
> -}
> -
>  set bounds_table 0
>  gdb_test_multiple "disassemble upper" "" {
>      -re -wrap "bndldx.*" {
> diff --git a/gdb/testsuite/gdb.arch/i386-mpx-map.c b/gdb/testsuite/gdb.arch/i386-mpx-map.c
> index 527f2dacdd1..541d54aa756 100644
> --- a/gdb/testsuite/gdb.arch/i386-mpx-map.c
> +++ b/gdb/testsuite/gdb.arch/i386-mpx-map.c
> @@ -21,41 +21,10 @@
>     along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
>  
>  #include <stdlib.h>
> -#include "x86-cpuid.h"
> -
> -#ifndef NOINLINE
> -#define NOINLINE __attribute__ ((noinline))
> -#endif
> -
>  #define SIZE  5
>  
>  typedef int T;
>  
> -unsigned int have_mpx (void) NOINLINE;
> -
> -unsigned int NOINLINE
> -have_mpx (void)
> -{
> -  unsigned int eax, ebx, ecx, edx;
> -
> -  if (!__get_cpuid (1, &eax, &ebx, &ecx, &edx))
> -    return 0;
> -
> -  if ((ecx & bit_OSXSAVE) == bit_OSXSAVE)
> -    {
> -      if (__get_cpuid_max (0, NULL) < 7)
> -	return 0;
> -
> -      __cpuid_count (7, 0, eax, ebx, ecx, edx);
> -
> -      if ((ebx & bit_MPX) == bit_MPX)
> -	return 1;
> -      else
> -	return 0;
> -    }
> -  return 0;
> -}
> -
>  void
>  foo (T *p)
>  {
> @@ -78,17 +47,15 @@ foo (T *p)
>  int
>  main (void)
>  {
> -  if (have_mpx ())
> -    {
> -      T *a = NULL;
> +  T *a = NULL;
>  
> -      a = calloc (SIZE, sizeof (T));	/* after-decl */
> +  a = calloc (SIZE, sizeof (T));	/* after-decl */
>  #if defined  __GNUC__ && !defined __INTEL_COMPILER
> -      __bnd_store_ptr_bounds (a, &a);
> +  __bnd_store_ptr_bounds (a, &a);
>  #endif
>  
> -      foo (a);				/* after-alloc */
> -      free (a);
> -    }
> +  foo (a);				/* after-alloc */
> +  free (a);
> +
>    return 0;
>  }
> diff --git a/gdb/testsuite/gdb.arch/i386-mpx-map.exp b/gdb/testsuite/gdb.arch/i386-mpx-map.exp
> index a98194dca61..55b70d1de0f 100644
> --- a/gdb/testsuite/gdb.arch/i386-mpx-map.exp
> +++ b/gdb/testsuite/gdb.arch/i386-mpx-map.exp
> @@ -27,6 +27,11 @@ if { ![supports_mpx_check_pointer_bounds] } {
>      return -1
>  }
>  
> +if { ![have_mpx] } {
> +    unsupported "processor does not support MPX"
> +    return -1
> +}
> +
>  set comp_flags "-mmpx -fcheck-pointer-bounds -I${srcdir}/../nat/"
>  
>  if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile} \
> @@ -39,24 +44,6 @@ if ![runto_main] {
>  	return -1
>  }
>  
> -set supports_mpx 0
> -set test "probe MPX support"
> -
> -gdb_test_multiple "print have_mpx()" $test {
> -    -re ".. = 1\r\n$gdb_prompt $" {
> -        pass $test
> -        set supports_mpx 1
> -    }
> -    -re ".. = 0\r\n$gdb_prompt $" {
> -        pass $test
> -    }
> -}
> -
> -if { !$supports_mpx } {
> -    unsupported "processor does not support MPX"
> -    return
> -}
> -
>  gdb_breakpoint [ gdb_get_line_number "after-decl" ]
>  gdb_breakpoint [ gdb_get_line_number "after-alloc" ]
>  gdb_breakpoint [ gdb_get_line_number "after-assign" ]
> diff --git a/gdb/testsuite/gdb.arch/i386-mpx-sigsegv.c b/gdb/testsuite/gdb.arch/i386-mpx-sigsegv.c
> index 517fb11abde..09e1b834e3e 100644
> --- a/gdb/testsuite/gdb.arch/i386-mpx-sigsegv.c
> +++ b/gdb/testsuite/gdb.arch/i386-mpx-sigsegv.c
> @@ -15,9 +15,6 @@
>     You should have received a copy of the GNU General Public License
>     along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
>  
> -#include "x86-cpuid.h"
> -#include <stdio.h>
> -
>  #define OUR_SIZE    5
>  
>  int gx[OUR_SIZE];
> @@ -26,29 +23,6 @@ int gb[OUR_SIZE];
>  int gc[OUR_SIZE];
>  int gd[OUR_SIZE];
>  
> -unsigned int
> -have_mpx (void)
> -{
> -  unsigned int eax, ebx, ecx, edx;
> -
> -  if (!__get_cpuid (1, &eax, &ebx, &ecx, &edx))
> -    return 0;
> -
> -  if ((ecx & bit_OSXSAVE) == bit_OSXSAVE)
> -    {
> -      if (__get_cpuid_max (0, NULL) < 7)
> -	return 0;
> -
> -      __cpuid_count (7, 0, eax, ebx, ecx, edx);
> -
> -      if ((ebx & bit_MPX) == bit_MPX)
> -	return 1;
> -      else
> -	return 0;
> -    }
> -  return 0;
> -}
> -
>  int
>  bp1 (int value)
>  {
> @@ -87,34 +61,32 @@ lower (int * p, int * a, int * b, int * c, int * d, int len)
>  int
>  main (void)
>  {
> -  if (have_mpx ())
> -    {
> -      int sx[OUR_SIZE];
> -      int sa[OUR_SIZE];
> -      int sb[OUR_SIZE];
> -      int sc[OUR_SIZE];
> -      int sd[OUR_SIZE];
> -      int *x, *a, *b, *c, *d;
> -
> -      x = calloc (OUR_SIZE, sizeof (int));
> -      a = calloc (OUR_SIZE, sizeof (int));
> -      b = calloc (OUR_SIZE, sizeof (int));
> -      c = calloc (OUR_SIZE, sizeof (int));
> -      d = calloc (OUR_SIZE, sizeof (int));
> -
> -      upper (x, a, b, c, d, OUR_SIZE + 2);
> -      upper (sx, sa, sb, sc, sd, OUR_SIZE + 2);
> -      upper (gx, ga, gb, gc, gd, OUR_SIZE + 2);
> -      lower (x, a, b, c, d, 1);
> -      lower (sx, sa, sb, sc, sd, 1);
> -      bp1 (*x);
> -      lower (gx, ga, gb, gc, gd, 1);
> +  int sx[OUR_SIZE];
> +  int sa[OUR_SIZE];
> +  int sb[OUR_SIZE];
> +  int sc[OUR_SIZE];
> +  int sd[OUR_SIZE];
> +  int *x, *a, *b, *c, *d;
> +
> +  x = calloc (OUR_SIZE, sizeof (int));
> +  a = calloc (OUR_SIZE, sizeof (int));
> +  b = calloc (OUR_SIZE, sizeof (int));
> +  c = calloc (OUR_SIZE, sizeof (int));
> +  d = calloc (OUR_SIZE, sizeof (int));
> +
> +  upper (x, a, b, c, d, OUR_SIZE + 2);
> +  upper (sx, sa, sb, sc, sd, OUR_SIZE + 2);
> +  upper (gx, ga, gb, gc, gd, OUR_SIZE + 2);
> +  lower (x, a, b, c, d, 1);
> +  lower (sx, sa, sb, sc, sd, 1);
> +  bp1 (*x);
> +  lower (gx, ga, gb, gc, gd, 1);
> +
> +  free (x);
> +  free (a);
> +  free (b);
> +  free (c);
> +  free (d);
>  
> -      free (x);
> -      free (a);
> -      free (b);
> -      free (c);
> -      free (d);
> -    }
>    return 0;
>  }
> diff --git a/gdb/testsuite/gdb.arch/i386-mpx-sigsegv.exp b/gdb/testsuite/gdb.arch/i386-mpx-sigsegv.exp
> index 739bb8d25fc..ef8fd68486b 100644
> --- a/gdb/testsuite/gdb.arch/i386-mpx-sigsegv.exp
> +++ b/gdb/testsuite/gdb.arch/i386-mpx-sigsegv.exp
> @@ -27,6 +27,11 @@ if { ![supports_mpx_check_pointer_bounds] } {
>      return -1
>  }
>  
> +if { ![have_mpx] } {
> +    unsupported "processor does not support MPX"
> +    return -1
> +}
> +
>  set comp_flags "-mmpx -fcheck-pointer-bounds -I${srcdir}/../nat/"
>  
>  if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile} \
> @@ -39,16 +44,6 @@ if ![runto_main] {
>      return -1
>  }
>  
> -gdb_test_multiple "print have_mpx ()" "have mpx" {
> -    -re ".. = 1\r\n$gdb_prompt " {
> -        pass "check whether processor supports MPX"
> -    }
> -    -re ".. = 0\r\n$gdb_prompt " {
> -        verbose "processor does not support MPX; skipping MPX tests"
> -        return
> -    }
> -}
> -
>  set u_fault [multi_line "Program received signal SIGSEGV, Segmentation fault" \
>                          "Upper bound violation while accessing address $hex" \
>                          "Bounds: \\\[lower = $hex, upper = $hex\\\]"]
> diff --git a/gdb/testsuite/gdb.arch/i386-mpx-simple_segv.c b/gdb/testsuite/gdb.arch/i386-mpx-simple_segv.c
> index 54b6f33164c..97baed74c93 100644
> --- a/gdb/testsuite/gdb.arch/i386-mpx-simple_segv.c
> +++ b/gdb/testsuite/gdb.arch/i386-mpx-simple_segv.c
> @@ -15,34 +15,8 @@
>     You should have received a copy of the GNU General Public License
>     along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
>  
> -#include "x86-cpuid.h"
> -#include <stdio.h>
> -
>  #define OUR_SIZE    5
>  
> -unsigned int
> -have_mpx (void)
> -{
> -  unsigned int eax, ebx, ecx, edx;
> -
> -  if (!__get_cpuid (1, &eax, &ebx, &ecx, &edx))
> -    return 0;
> -
> -  if ((ecx & bit_OSXSAVE) == bit_OSXSAVE)
> -    {
> -      if (__get_cpuid_max (0, NULL) < 7)
> -	return 0;
> -
> -      __cpuid_count (7, 0, eax, ebx, ecx, edx);
> -
> -      if ((ebx & bit_MPX) == bit_MPX)
> -	return 1;
> -      else
> -	return 0;
> -    }
> -  return 0;
> -}
> -
>  void
>  upper (int * p, int len)
>  {
> @@ -54,13 +28,9 @@ upper (int * p, int len)
>  int
>  main (void)
>  {
> -  if (have_mpx ())
> -    {
> -      int a = 0;			/* Dummy variable for debugging purposes.  */
> -      int sx[OUR_SIZE];
> -      a++;				/* register-eval.  */
> -      upper (sx, OUR_SIZE + 2);
> -      return sx[1];
> -    }
> -  return 0;
> +  int a = 0;			/* Dummy variable for debugging purposes.  */
> +  int sx[OUR_SIZE];
> +  a++;				/* register-eval.  */
> +  upper (sx, OUR_SIZE + 2);
> +  return sx[1];
>  }
> diff --git a/gdb/testsuite/gdb.arch/i386-mpx-simple_segv.exp b/gdb/testsuite/gdb.arch/i386-mpx-simple_segv.exp
> index 51b15818466..e5b2a890f3c 100644
> --- a/gdb/testsuite/gdb.arch/i386-mpx-simple_segv.exp
> +++ b/gdb/testsuite/gdb.arch/i386-mpx-simple_segv.exp
> @@ -33,6 +33,11 @@ if { ![supports_mpx_check_pointer_bounds] } {
>      return -1
>  }
>  
> +if { ![have_mpx] } {
> +    unsupported "processor does not support MPX"
> +    return -1
> +}
> +
>  set comp_flags "-mmpx -fcheck-pointer-bounds -I${srcdir}/../nat/"
>  
>  if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile} \
> @@ -45,16 +50,6 @@ if ![runto_main] {
>      return -1
>  }
>  
> -gdb_test_multiple "print have_mpx ()" "have mpx" {
> -    -re ".. = 1\r\n$gdb_prompt " {
> -        pass "check whether processor supports MPX"
> -    }
> -    -re ".. = 0\r\n$gdb_prompt " {
> -        verbose "processor does not support MPX; skipping MPX tests"
> -        return
> -    }
> -}
> -
>  set violation [multi_line "Program received signal SIGSEGV, Segmentation fault" \
>                            "Upper bound violation while accessing address $hex" \
>                            "Bounds: \\\[lower = $hex, upper = $hex\\\]"]
> diff --git a/gdb/testsuite/gdb.arch/i386-mpx.c b/gdb/testsuite/gdb.arch/i386-mpx.c
> index 178c34a330c..b96da5478af 100644
> --- a/gdb/testsuite/gdb.arch/i386-mpx.c
> +++ b/gdb/testsuite/gdb.arch/i386-mpx.c
> @@ -17,77 +17,43 @@
>     You should have received a copy of the GNU General Public License
>     along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
>  
> -#include <stdio.h>
> -#include "x86-cpuid.h"
> -
> -#ifndef NOINLINE
> -#define NOINLINE __attribute__ ((noinline))
> -#endif
> -
> -unsigned int have_mpx (void) NOINLINE;
> -
> -unsigned int NOINLINE
> -have_mpx (void)
> -{
> -  unsigned int eax, ebx, ecx, edx;
> -
> -  if (!__get_cpuid (1, &eax, &ebx, &ecx, &edx))
> -    return 0;
> -
> -  if ((ecx & bit_OSXSAVE) == bit_OSXSAVE)
> -    {
> -      if (__get_cpuid_max (0, NULL) < 7)
> -	return 0;
> -
> -      __cpuid_count (7, 0, eax, ebx, ecx, edx);
> -
> -      if ((ebx & bit_MPX) == bit_MPX)
> -	return 1;
> -      else
> -	return 0;
> -    }
> -  return 0;
> -}
> -
>  int
>  main (int argc, char **argv)
>  {
> -  if (have_mpx ())
> -    {
>  #ifdef __x86_64__
> -      asm ("mov $10, %rax\n\t"
> -	  "mov $9, %rdx\n\t"
> -	  "bndmk (%rax,%rdx), %bnd0\n\t"
> -	  "mov $20, %rax\n\t"
> -	  "mov $9, %rdx\n\t"
> -	  "bndmk (%rax,%rdx), %bnd1\n\t"
> -	  "mov $30, %rax\n\t"
> -	  "mov $9, %rdx\n\t"
> -	  "bndmk (%rax,%rdx), %bnd2\n\t"
> -	  "mov $40, %rax\n\t"
> -	  "mov $9, %rdx\n\t"
> -	  "bndmk (%rax,%rdx), %bnd3\n\t"
> -	  "bndstx %bnd3, (%rax) \n\t"
> -	  "nop\n\t"
> -         );
> +  asm ("mov $10, %rax\n\t"
> +       "mov $9, %rdx\n\t"
> +       "bndmk (%rax,%rdx), %bnd0\n\t"
> +       "mov $20, %rax\n\t"
> +       "mov $9, %rdx\n\t"
> +       "bndmk (%rax,%rdx), %bnd1\n\t"
> +       "mov $30, %rax\n\t"
> +       "mov $9, %rdx\n\t"
> +       "bndmk (%rax,%rdx), %bnd2\n\t"
> +       "mov $40, %rax\n\t"
> +       "mov $9, %rdx\n\t"
> +       "bndmk (%rax,%rdx), %bnd3\n\t"
> +       "bndstx %bnd3, (%rax) \n\t"
> +       "nop\n\t"
> +       );
>  #else
> -      asm ("mov $10, %eax\n\t"
> -	   "mov $9, %edx\n\t"
> -	   "bndmk (%eax,%edx), %bnd0\n\t"
> -	   "mov $20, %eax\n\t"
> -	   "mov $9, %edx\n\t"
> -	   "bndmk (%eax,%edx), %bnd1\n\t"
> -	   "mov $30, %eax\n\t"
> -	   "mov $9, %edx\n\t"
> -	   "bndmk (%eax,%edx), %bnd2\n\t"
> -	   "mov $40, %eax\n\t"
> -	   "mov $9, %edx\n\t"
> -	   "bndmk (%eax,%edx), %bnd3\n\t"
> -	   "bndstx  %bnd3, (%eax)\n\t"
> -	   "nop\n\t"
> -	  );
> +  asm ("mov $10, %eax\n\t"
> +       "mov $9, %edx\n\t"
> +       "bndmk (%eax,%edx), %bnd0\n\t"
> +       "mov $20, %eax\n\t"
> +       "mov $9, %edx\n\t"
> +       "bndmk (%eax,%edx), %bnd1\n\t"
> +       "mov $30, %eax\n\t"
> +       "mov $9, %edx\n\t"
> +       "bndmk (%eax,%edx), %bnd2\n\t"
> +       "mov $40, %eax\n\t"
> +       "mov $9, %edx\n\t"
> +       "bndmk (%eax,%edx), %bnd3\n\t"
> +       "bndstx  %bnd3, (%eax)\n\t"
> +       "nop\n\t"
> +       );
>  #endif
> -	asm ("nop\n\t");	/* break here.  */
> -    }
> +  asm ("nop\n\t");	/* break here.  */
> +
>    return 0;
>  }
> diff --git a/gdb/testsuite/gdb.arch/i386-mpx.exp b/gdb/testsuite/gdb.arch/i386-mpx.exp
> index 639f7cdc3bb..a6b74ceda6b 100644
> --- a/gdb/testsuite/gdb.arch/i386-mpx.exp
> +++ b/gdb/testsuite/gdb.arch/i386-mpx.exp
> @@ -31,6 +31,11 @@ if { ![supports_mpx_check_pointer_bounds] } {
>      return -1
>  }
>  
> +if { ![have_mpx] } {
> +    unsupported "processor does not support MPX"
> +    return -1
> +}
> +
>  set comp_flags "-mmpx -fcheck-pointer-bounds -I${srcdir}/../nat/"
>  
>  if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile} \
> @@ -43,23 +48,6 @@ if ![runto_main] {
>      return -1
>  }
>  
> -set supports_mpx 0
> -set test "probe MPX support"
> -gdb_test_multiple "print have_mpx()" $test {
> -    -re ".. = 1\r\n$gdb_prompt $" {
> -        pass $test
> -        set supports_mpx 1
> -    }
> -    -re ".. = 0\r\n$gdb_prompt $" {
> -        pass $test
> -    }
> -}
> -
> -if { !$supports_mpx } {
> -    unsupported "processor does not support MPX"
> -    return
> -}
> -
>  # Test bndcfg register and bndstatus at startup
>  set test_string "\\\{raw = 0x\[0-9a-f\]+, config = \\\{base = \[0-9\]+,\
>  reserved = \[0-9\]+, preserved = \[0-9\]+, enabled = \[0-9\]+\\\}\\\}"
> diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
> index 140e3960a32..3a57453da04 100644
> --- a/gdb/testsuite/lib/gdb.exp
> +++ b/gdb/testsuite/lib/gdb.exp
> @@ -7771,5 +7771,57 @@ gdb_caching_proc supports_gnuc {
>      return [gdb_simple_compile $me $src object ""]
>  }
>  
> +# Return 1 if target supports mpx, otherwise return 0.
> +gdb_caching_proc have_mpx {
> +    global srcdir
> +
> +    set me "have_mpx"
> +    if { ![istarget "i?86-*-*"] && ![istarget "x86_64-*-*"] } {
> +        verbose "$me: target does not support mpx, returning 0" 2
> +        return 0
> +    }
> +
> +    # Compile a test program.
> +    set src {
> +       #include "nat/x86-cpuid.h"
> +
> +        int main() {
> +	  unsigned int eax, ebx, ecx, edx;
> +
> +	  if (!__get_cpuid (1, &eax, &ebx, &ecx, &edx))
> +	    return 0;
> +
> +	  if ((ecx & bit_OSXSAVE) == bit_OSXSAVE)
> +	    {
> +	      if (__get_cpuid_max (0, (void *)0) < 7)
> +		return 0;
> +
> +		__cpuid_count (7, 0, eax, ebx, ecx, edx);
> +
> +		if ((ebx & bit_MPX) == bit_MPX)
> +		  return 1;
> +
> +	    }
> +	  return 0;
> +	}
> +    }
> +    set compile_flags "incdir=${srcdir}/.."
> +    if {![gdb_simple_compile $me $src executable $compile_flags]} {
> +        return 0
> +    }
> +
> +    set result [remote_exec target $obj]
> +    set status [lindex $result 0]
> +    set output [lindex $result 1]
> +    if { $output != "" } {
> +	return 0
> +    }
> +
> +    remote_file build delete $obj


Should we delete the obj file in the `if { $output != "" }`?

When is that path taken?

Otherwise, that LGTM.

Simon

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH][gdb/testsuite] Add have_mpx in lib/gdb.exp
  2021-01-12 15:35 ` Simon Marchi via Gdb-patches
@ 2021-01-12 16:39   ` Tom de Vries
  0 siblings, 0 replies; 3+ messages in thread
From: Tom de Vries @ 2021-01-12 16:39 UTC (permalink / raw)
  To: Simon Marchi, gdb-patches

On 1/12/21 4:35 PM, Simon Marchi wrote:
> On 2021-01-12 5:59 a.m., Tom de Vries wrote:
>> +    set compile_flags "incdir=${srcdir}/.."
>> +    if {![gdb_simple_compile $me $src executable $compile_flags]} {
>> +        return 0
>> +    }
>> +
>> +    set result [remote_exec target $obj]
>> +    set status [lindex $result 0]
>> +    set output [lindex $result 1]
>> +    if { $output != "" } {
>> +	return 0
>> +    }
>> +
>> +    remote_file build delete $obj
> 
> 
> Should we delete the obj file in the `if { $output != "" }`?
> 

Yes, thanks for catching that.  Fixed and committed.

> When is that path taken?
> 

When the executable produces some output, which it shouldn't.

Thanks,
- Tom

> Otherwise, that LGTM.
> 
> Simon
> 

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2021-01-12 16:39 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-12 10:59 [PATCH][gdb/testsuite] Add have_mpx in lib/gdb.exp Tom de Vries
2021-01-12 15:35 ` Simon Marchi via Gdb-patches
2021-01-12 16:39   ` Tom de Vries

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox