[RFC] Set process affinity in test to work around ARM ptrace bug

[RFC] Set process affinity in test to work around ARM ptrace bug

[Yao Qi]

We recently found a ARM kernel ptrace bug
http://lists.infradead.org/pipermail/linux-arm-kernel/2016-May/431962.html
As a result of this bug, after GDB ptrace set VFP registers, the hardware
registers may not be updated.  This bug causes some intermittent fails in
tests, like return.exp, call-rt-st.exp, callfuncs.exp, etc.

The bug is fixed in ARM kernel tree, but it is impractical to upgrade
linux kernel from git tree or most recently release (I don't know when
the fix can be shipped in the mainline kernel release).  I am wondering
we can workaround this kernel bug somehow.

My first attempt is to workaround it in GDB, so that GDB still writes
the VFP registers and sync them to hardware.  The kernel patch is quite
simple, which moves vfp_flush_hwstate one line below.  Probably, we can
call ptrace set vfp registers twice, and then the second vfp set can
flush the state correctly.  Unfortunately, it doesn't work, because
every time of ptrace set, kernel loads VFP registers from hardware first,
which might be out of date after the first ptrace set.  That is to say,
we can't workaround this kernel bug in GDB.

Then, I am thinking we can workaround this bug in testing, because the
intermittent fails are confusing in comparing test results, by binding
both tracer and tracee on the same core.  For example, we can start GDB
or GDBserver with "taskset -c 0 ", but this is a global change, may
have some affects on gdb.threads tests.  I also think about doing
"taskset -p PID -c 0" in test harness after the inferior is started,
and do the same to the parent process of inferior (which is either GDB
or GDBserver).

The approach in this patch is to have a small c function which sets
both process affinity and its parent's affinity to core 0.  This
function should be called in these tests explicitly, but other tests
are not affected at all.  This patch is posted to get comments on the
necessity of workaround this kernel bug, and the proper to workaround
this bug.  There are still some test cases affected by this kernel bug,
but this patch doesn't touch them yet.

gdb/testsuite:

2016-06-30  Yao Qi  <yao.qi@linaro.org>

	* lib/set_process_affinity.c: New file.
	* gdb.base/call-rt-st.c: Include lib/set_process_affinity.c.
	(main): Call set_process_affinity.
	* gdb.base/gnu_vector.c: Likewise.
	* gdb.base/return.c: Likewise.
	* gdb.base/gnu_vector.exp: Adjust test.
---
 gdb/testsuite/gdb.base/call-rt-st.c      |  2 ++
 gdb/testsuite/gdb.base/gnu_vector.c      |  4 +++-
 gdb/testsuite/gdb.base/gnu_vector.exp    |  3 +++
 gdb/testsuite/gdb.base/return.c          |  2 ++
 gdb/testsuite/lib/set_process_affinity.c | 41 ++++++++++++++++++++++++++++++++
 5 files changed, 51 insertions(+), 1 deletion(-)
 create mode 100644 gdb/testsuite/lib/set_process_affinity.c

diff --git a/gdb/testsuite/gdb.base/call-rt-st.c b/gdb/testsuite/gdb.base/call-rt-st.c
index 072ea86..ad97e28 100644
--- a/gdb/testsuite/gdb.base/call-rt-st.c
+++ b/gdb/testsuite/gdb.base/call-rt-st.c
@@ -1,3 +1,4 @@
+#include "../lib/set_process_affinity.c"
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
@@ -565,6 +566,7 @@ int main ()  {
    struct two_floats_t      *f3;
 
   gdb_unbuffer_output ();
+  set_process_affinity ();
 
   /* Allocate space for large structures 
    */
diff --git a/gdb/testsuite/gdb.base/gnu_vector.c b/gdb/testsuite/gdb.base/gnu_vector.c
index ee03ac1..8e0d6a8 100644
--- a/gdb/testsuite/gdb.base/gnu_vector.c
+++ b/gdb/testsuite/gdb.base/gnu_vector.c
@@ -18,6 +18,7 @@
    Contributed by Ken Werner <ken.werner@de.ibm.com>  */
 
 #include <stdarg.h>
+#include "../lib/set_process_affinity.c"
 
 #define VECTOR(n, type)					\
   type __attribute__ ((vector_size (n * sizeof(type))))
@@ -137,7 +138,8 @@ main ()
 {
   int4 res;
 
-  res = add_some_intvecs (i4a, i4a + i4b, i4b);
+  set_process_affinity ();
+  res = add_some_intvecs (i4a, i4a + i4b, i4b); /* breakpoint here */
 
   res = add_some_intvecs (i4a, i4a + i4b, i4b);
 
diff --git a/gdb/testsuite/gdb.base/gnu_vector.exp b/gdb/testsuite/gdb.base/gnu_vector.exp
index aafaedd..1e57a26 100644
--- a/gdb/testsuite/gdb.base/gnu_vector.exp
+++ b/gdb/testsuite/gdb.base/gnu_vector.exp
@@ -55,6 +55,9 @@ gdb_test_multiple "show endian" "show endian" {
     }
 }
 
+gdb_breakpoint [gdb_get_line_number "breakpoint here"]
+gdb_continue_to_breakpoint "breakpoint here"
+
 # Test printing of character vector types
 gdb_test "print c4" "\\\$$decimal = \\{1, 2, 3, 4\\}"
 gdb_test "print c4\[2\]" "\\\$$decimal = 3"
diff --git a/gdb/testsuite/gdb.base/return.c b/gdb/testsuite/gdb.base/return.c
index c365e88..6ff38e6 100644
--- a/gdb/testsuite/gdb.base/return.c
+++ b/gdb/testsuite/gdb.base/return.c
@@ -15,6 +15,7 @@
    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 "../lib/set_process_affinity.c"
 #include <stdio.h>
 /*  Test "return" command.  */
 
@@ -40,6 +41,7 @@ double tmp3;
 
 int main ()
 {
+  set_process_affinity ();
   func1 ();
   printf("in main after func1\n");
   tmp2 = func2 ();
diff --git a/gdb/testsuite/lib/set_process_affinity.c b/gdb/testsuite/lib/set_process_affinity.c
new file mode 100644
index 0000000..2615965
--- /dev/null
+++ b/gdb/testsuite/lib/set_process_affinity.c
@@ -0,0 +1,41 @@
+/* Copyright (C) 2016 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#if defined(__arm__) && defined(__linux__)
+#define _GNU_SOURCE
+#include <sched.h>
+#include <unistd.h>
+#include <sys/types.h>
+#endif
+
+static int
+set_process_affinity (void)
+{
+#if defined(__arm__) && defined(__linux__)
+  cpu_set_t my_set;
+
+  /* Set both process and parent process (GDB)'s affinity on core 0
+     to workaround ARM linux kernel ptrace bug which doesn't flush the
+     VFP state to hardware after ptrace set VFP registers.  */
+
+  CPU_ZERO (&my_set);
+  CPU_SET (0, &my_set);
+
+  sched_setaffinity (0, sizeof(cpu_set_t), &my_set);
+  sched_setaffinity (getppid (), sizeof(cpu_set_t), &my_set);
+#endif
+}
-- 
1.9.1

Re: [RFC] Set process affinity in test to work around ARM ptrace bug

[Antoine Tremblay]

Yao Qi writes:

> We recently found a ARM kernel ptrace bug
> http://lists.infradead.org/pipermail/linux-arm-kernel/2016-May/431962.html
> As a result of this bug, after GDB ptrace set VFP registers, the hardware
> registers may not be updated.  This bug causes some intermittent fails in
> tests, like return.exp, call-rt-st.exp, callfuncs.exp, etc.
>
> The bug is fixed in ARM kernel tree, but it is impractical to upgrade
> linux kernel from git tree or most recently release (I don't know when
> the fix can be shipped in the mainline kernel release).  I am wondering
> we can workaround this kernel bug somehow.
>
> My first attempt is to workaround it in GDB, so that GDB still writes
> the VFP registers and sync them to hardware.  The kernel patch is quite
> simple, which moves vfp_flush_hwstate one line below.  Probably, we can
> call ptrace set vfp registers twice, and then the second vfp set can
> flush the state correctly.  Unfortunately, it doesn't work, because
> every time of ptrace set, kernel loads VFP registers from hardware first,
> which might be out of date after the first ptrace set.  That is to say,
> we can't workaround this kernel bug in GDB.
>
> Then, I am thinking we can workaround this bug in testing, because the
> intermittent fails are confusing in comparing test results, by binding
> both tracer and tracee on the same core.  For example, we can start GDB
> or GDBserver with "taskset -c 0 ", but this is a global change, may
> have some affects on gdb.threads tests.  I also think about doing
> "taskset -p PID -c 0" in test harness after the inferior is started,
> and do the same to the parent process of inferior (which is either GDB
> or GDBserver).
>
> The approach in this patch is to have a small c function which sets
> both process affinity and its parent's affinity to core 0.  This
> function should be called in these tests explicitly, but other tests
> are not affected at all.  This patch is posted to get comments on the
> necessity of workaround this kernel bug, and the proper to workaround
> this bug.  There are still some test cases affected by this kernel bug,
> but this patch doesn't touch them yet.
>

I like the idea, this has been a pain for a while however from my
testing there is a lot of intermitent tests and I'm not sure if this
ptrace fix fixes them all.

I think we just make sure that we don't hide other ptrace bugs so that
we can find them. I had another bug in the Odroid UX4 SoC causing
similar problems.

Also to consider is that this could apply to a lot of tests here's my
list of intermittent test from about 40 runs with Sergio's script:

argv0-symlink.exp array_bounds.exp array_ptr_renaming.exp
array_subscript_addr.exp auxv.exp bp-permanent.exp bp_enum_homonym.exp
bp_range_type.exp branch-to-self.exp break-precsave.exp
breakpoint-in-ro-region.exp catch_ex.exp char_enum.exp class2.exp
consecutive-precsave.exp converts.exp coredump-filter.exp dot_all.exp
exprs.exp fin_fun_out.exp finish-precsave.exp finish-reverse-bkpt.exp
finish-reverse.exp fixed_points.exp float_param.exp frame-args.exp
fstatat-reverse.exp fun_overload_menu.exp fun_renaming.exp
funcall_char.exp gcore-buffer-overflow.exp gcore-relro-pie.exp
gcore-relro.exp gcore.exp gdb-index.exp gdb1555.exp
getresuid-reverse.exp gnu-ifunc.exp gnu_vector.exp info-proc.exp
info-threads.exp interrupted-hand-call.exp jmisc.exp jprint.exp jump.exp
lang_switch.exp machinestate-precsave.exp mi_dyn_arr.exp
mi_interface.exp mi_task_arg.exp mi_task_info.exp mi_var_array.exp
multi-forks.exp next-while-other-thread-longjmps.exp operators.exp
optim_drec.exp out_of_line_in_inlined.exp pckd_arr_ren.exp
pipe-reverse.exp print-symbol-loading.exp print_chars.exp
process-dies-while-handling-bp.exp pthreads.exp py-strfns.exp python.exp
queue-signal.exp readv-reverse.exp rec_return.exp sigall-precsave.exp
sigall-reverse.exp siginfo-obj.exp siginfo-thread.exp skip-solib.exp
small_reg_param.exp solib-precsave.exp solib-reverse.exp str_uninit.exp
taft_type.exp task_bp.exp type_coercion.exp until-reverse.exp
watch-precsave.exp whatis_array_val.exp watch-bitfields.exp
packed_array.exp formatted_ref.exp vec_comps.exp solib-intra-step.exp
waitpid-reverse.exp mi-tsv-changed.exp"

These a from a few weeks ago and I think a lof of reverse tests may not
be valid... not sure still it's quite a list.

I'll retest with the patched kernel over the weekend see how many go
away...

Thanks for looking into this!
Antoine

Re: [RFC] Set process affinity in test to work around ARM ptrace bug

[Pedro Alves]

On 06/30/2016 02:57 PM, Yao Qi wrote:

> Then, I am thinking we can workaround this bug in testing, because the
> intermittent fails are confusing in comparing test results, by binding
> both tracer and tracee on the same core.  For example, we can start GDB
> or GDBserver with "taskset -c 0 ", but this is a global change, may
> have some affects on gdb.threads tests.
> I also think about doing
> "taskset -p PID -c 0" in test harness after the inferior is started,
> and do the same to the parent process of inferior (which is either GDB
> or GDBserver).
> 
> The approach in this patch is to have a small c function which sets
> both process affinity and its parent's affinity to core 0.  This
> function should be called in these tests explicitly, but other tests
> are not affected at all.  This patch is posted to get comments on the
> necessity of workaround this kernel bug, and the proper to workaround
> this bug.  There are still some test cases affected by this kernel bug,
> but this patch doesn't touch them yet.

Pushing people to update their kernels would be better, but I
understand how that's complicated on ARM, given that in many cases
it's not even possible to have access to the kernel's sources...

Still, it'd think that a fix in gdb/gdbserver itself would be
better  for _users_.

Also having to manually determine whether a test is misbehaving
because of this problem or not seems like recipe for continued pain.

I also think that whatever workaround, if any, should be limited
to known-broken kernels.  Otherwise, this is likely to mask
other problems going forward.  Maybe all we have is the version
number to work with, but that's still better than unconditionally
enabling this on arm.

Thanks,
Pedro Alves

Re: [RFC] Set process affinity in test to work around ARM ptrace bug

[Yao Qi]

Pedro Alves <palves@redhat.com> writes:

> I also think that whatever workaround, if any, should be limited
> to known-broken kernels.  Otherwise, this is likely to mask
> other problems going forward.  Maybe all we have is the version
> number to work with, but that's still better than unconditionally
> enabling this on arm.

The updated version adds a linux kernel version check.

-- 
Yao (齐尧)
From 27fe094e6a99929f8f281d88beaa599771550025 Mon Sep 17 00:00:00 2001
From: Yao Qi <yao.qi@linaro.org>
Date: Mon, 27 Jun 2016 08:45:16 +0100
Subject: [PATCH] Set process affinity in test to work around ARM ptrace bug

We recently found a ARM kernel ptrace bug
http://lists.infradead.org/pipermail/linux-arm-kernel/2016-May/431962.html
As a result of this bug, after GDB ptrace set VFP registers, the hardware
registers may not be updated.  This bug causes some intermittent fails in
tests, like return.exp, call-rt-st.exp, callfuncs.exp, etc.

The bug was introduced by 8130b9d7b9d858aa04ce67805e8951e3cb6e9b2f
in 2012 and is fixed in e2dfb4b880146bfd4b6aa8e138c0205407cebbaf in May.
The bug is fixed in ARM kernel tree, but it is impractical to upgrade
linux kernel from git tree or most recently release.  I am wondering
we can workaround this kernel bug somehow.

My first attempt is to workaround it in GDB, so that GDB still writes
the VFP registers and sync them to hardware.  The kernel patch is quite
simple, which moves vfp_flush_hwstate one line below.  Probably, we can
call ptrace set vfp registers twice, and then the second vfp set can
flush the state correctly.  Unfortunately, it doesn't work, because
every time of ptrace set, kernel loads VFP registers from hardware first,
which might be out of date after the first ptrace set.  That is to say,
we can't workaround this kernel bug in GDB.

Then, I am thinking we can workaround this bug in testing, because the
intermittent fails are confusing in comparing test results.  We can bind
both tracer and tracee on the same core.  For example, we can start GDB
or GDBserver with "taskset -c 0 ", but this is a global change, may
have some affects on gdb.threads tests.  I also think about doing
"taskset -p PID -c 0" in test harness after the inferior is started,
and do the same to the parent process of inferior (which is either GDB
or GDBserver), but don't know how to get GDB (in remote host) and
GDBserver's process id.

The approach in this patch is to have a small c function which sets
both process affinity and its parent's affinity to core 0 if the target
is arm linux and the kernel version is known broken having the ptrace
bug setting VFP registers.  The function set_process_affinity should
be called in these tests explicitly, but other tests are not affected
at all.

Note that this kernel bug only exists between commits
8130b9d7b9d858aa04ce67805e8951e3cb6e9b2f and e2dfb4b880146bfd4b6aa8e138c0205407cebbaf
However, a certain commit will be merged to many branches and releases,
which makes version checks complicated.  I checked all released kernels,
and get a list of versions that this bug is fixed.  Not all longterm
kernels on kernel.org have this bug fix, I don't know why, for example,
some 3.x kernels doesn't have this bug fix.

Secondly, kernels older than 8130b9d7b9d858aa04ce67805e8951e3cb6e9b2f
are not affected by this bug, so the official kernel releases older
than 3.0.21 or 3.2.6 are not affected by this bug, but I think the
distro may backport the commit to their older kernel, so it makes few
sense to check kernel is older than some versions (3.0.21 and 3.2.6).

gdb/testsuite:

2016-07-04  Yao Qi  <yao.qi@linaro.org>

	* lib/set_process_affinity.c: New file.

	* gdb.arch/arm-neon.c: Include lib/set_process_affinity.c.
	(main): Call set_process_affinity.
	* gdb.base/callfuncs.c: Likewise.
	* gdb.base/call-rt-st.c: Likewise.
	* gdb.base/gnu_vector.c: Likewise.
	* gdb.base/return.c: Likewise.
	* gdb.base/return2.c: Likewise.
	* gdb.base/store.c: Likewise.
	* gdb.base/structs.c: Likewise.
	* gdb.arch/arm-neon.exp: Set breakpoint and continue to
	breakpoint.
	* gdb.base/gnu_vector.exp: Likewise.

diff --git a/gdb/testsuite/gdb.arch/arm-neon.c b/gdb/testsuite/gdb.arch/arm-neon.c
index c67191c..f090b63 100644
--- a/gdb/testsuite/gdb.arch/arm-neon.c
+++ b/gdb/testsuite/gdb.arch/arm-neon.c
@@ -15,6 +15,7 @@
    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 "../lib/set_process_affinity.c"
 #include <arm_neon.h>
 
 #define DEF_FUNC1(N, TYPE, VALUE...)	\
@@ -98,5 +99,6 @@ DEF_FUNC2 (3)
 int
 main (void)
 {
-  return 0;
+  set_process_affinity ();
+  return 0; /* breakpoint here */
 }
diff --git a/gdb/testsuite/gdb.arch/arm-neon.exp b/gdb/testsuite/gdb.arch/arm-neon.exp
index 053170f..d7a149d 100644
--- a/gdb/testsuite/gdb.arch/arm-neon.exp
+++ b/gdb/testsuite/gdb.arch/arm-neon.exp
@@ -31,6 +31,9 @@ if ![runto_main] {
     return -1
 }
 
+gdb_breakpoint [gdb_get_line_number "breakpoint here"]
+gdb_continue_to_breakpoint "breakpoint here"
+
 # Test passing vectors in function argument in the inferior call.
 
 for {set i 1} {$i <= 18} {incr i} {
diff --git a/gdb/testsuite/gdb.base/call-rt-st.c b/gdb/testsuite/gdb.base/call-rt-st.c
index 072ea86..ad97e28 100644
--- a/gdb/testsuite/gdb.base/call-rt-st.c
+++ b/gdb/testsuite/gdb.base/call-rt-st.c
@@ -1,3 +1,4 @@
+#include "../lib/set_process_affinity.c"
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
@@ -565,6 +566,7 @@ int main ()  {
    struct two_floats_t      *f3;
 
   gdb_unbuffer_output ();
+  set_process_affinity ();
 
   /* Allocate space for large structures 
    */
diff --git a/gdb/testsuite/gdb.base/callfuncs.c b/gdb/testsuite/gdb.base/callfuncs.c
index 317e7c4..cbc1977 100644
--- a/gdb/testsuite/gdb.base/callfuncs.c
+++ b/gdb/testsuite/gdb.base/callfuncs.c
@@ -25,6 +25,7 @@
 #define PARAMS(paramlist) paramlist
 #endif
 
+#include "../lib/set_process_affinity.c"
 # include <stdlib.h>
 # include <string.h>
 
@@ -644,7 +645,7 @@ voidfunc (void)
 
 int main ()
 {
-  void *p = malloc (1);
+  void *p = malloc (1); set_process_affinity ();
   t_double_values(double_val1, double_val2);
   t_structs_c(struct_val1);
   free (p);
diff --git a/gdb/testsuite/gdb.base/gnu_vector.c b/gdb/testsuite/gdb.base/gnu_vector.c
index ee03ac1..8e0d6a8 100644
--- a/gdb/testsuite/gdb.base/gnu_vector.c
+++ b/gdb/testsuite/gdb.base/gnu_vector.c
@@ -18,6 +18,7 @@
    Contributed by Ken Werner <ken.werner@de.ibm.com>  */
 
 #include <stdarg.h>
+#include "../lib/set_process_affinity.c"
 
 #define VECTOR(n, type)					\
   type __attribute__ ((vector_size (n * sizeof(type))))
@@ -137,7 +138,8 @@ main ()
 {
   int4 res;
 
-  res = add_some_intvecs (i4a, i4a + i4b, i4b);
+  set_process_affinity ();
+  res = add_some_intvecs (i4a, i4a + i4b, i4b); /* breakpoint here */
 
   res = add_some_intvecs (i4a, i4a + i4b, i4b);
 
diff --git a/gdb/testsuite/gdb.base/gnu_vector.exp b/gdb/testsuite/gdb.base/gnu_vector.exp
index aafaedd..1e57a26 100644
--- a/gdb/testsuite/gdb.base/gnu_vector.exp
+++ b/gdb/testsuite/gdb.base/gnu_vector.exp
@@ -55,6 +55,9 @@ gdb_test_multiple "show endian" "show endian" {
     }
 }
 
+gdb_breakpoint [gdb_get_line_number "breakpoint here"]
+gdb_continue_to_breakpoint "breakpoint here"
+
 # Test printing of character vector types
 gdb_test "print c4" "\\\$$decimal = \\{1, 2, 3, 4\\}"
 gdb_test "print c4\[2\]" "\\\$$decimal = 3"
diff --git a/gdb/testsuite/gdb.base/return.c b/gdb/testsuite/gdb.base/return.c
index c365e88..6ff38e6 100644
--- a/gdb/testsuite/gdb.base/return.c
+++ b/gdb/testsuite/gdb.base/return.c
@@ -15,6 +15,7 @@
    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 "../lib/set_process_affinity.c"
 #include <stdio.h>
 /*  Test "return" command.  */
 
@@ -40,6 +41,7 @@ double tmp3;
 
 int main ()
 {
+  set_process_affinity ();
   func1 ();
   printf("in main after func1\n");
   tmp2 = func2 ();
diff --git a/gdb/testsuite/gdb.base/return2.c b/gdb/testsuite/gdb.base/return2.c
index ced472a..53e292f 100644
--- a/gdb/testsuite/gdb.base/return2.c
+++ b/gdb/testsuite/gdb.base/return2.c
@@ -15,6 +15,7 @@
    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 "../lib/set_process_affinity.c"
 /* Test gdb's "return" command.  */
 
 int void_test = 0;
@@ -90,6 +91,7 @@ int main (int argc, char **argv)
   double double_resultval;
   int i;
 
+  set_process_affinity ();
   /* A "test load" that will insure that the function really returns 
      a ${type} (as opposed to just a truncated or part of a ${type}).  */
   for (i = 0; i < sizeof (testval.ffff); i++)
diff --git a/gdb/testsuite/gdb.base/store.c b/gdb/testsuite/gdb.base/store.c
index 545515d..d878142 100644
--- a/gdb/testsuite/gdb.base/store.c
+++ b/gdb/testsuite/gdb.base/store.c
@@ -7,6 +7,8 @@
    function calls within main even when no optimization flags were
    passed.  */
 
+#include "../lib/set_process_affinity.c"
+
 typedef signed char charest;
 
 charest
@@ -254,6 +256,7 @@ wack_field_4 (void)
 int
 main ()
 {
+  set_process_affinity ();
   /* These calls are for current frame test.  */
   wack_charest (-1, -2);
   wack_short (-1, -2);
diff --git a/gdb/testsuite/gdb.base/structs.c b/gdb/testsuite/gdb.base/structs.c
index b5832cc..7be1fe0 100644
--- a/gdb/testsuite/gdb.base/structs.c
+++ b/gdb/testsuite/gdb.base/structs.c
@@ -15,6 +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 "../lib/set_process_affinity.c"
+
 /* Useful abreviations.  */
 typedef void t;
 typedef char tc;
@@ -313,6 +315,8 @@ int main()
 {
   int i;
 
+  set_process_affinity ();
+
   for (i = 0; i < 256; i++)
     chartest[i].c = i;
   chartest[0].c = 0;  /* chartest-done */
diff --git a/gdb/testsuite/lib/set_process_affinity.c b/gdb/testsuite/lib/set_process_affinity.c
new file mode 100644
index 0000000..1d2a0e4
--- /dev/null
+++ b/gdb/testsuite/lib/set_process_affinity.c
@@ -0,0 +1,98 @@
+/* Copyright (C) 2016 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#if defined(__arm__) && defined(__linux__)
+#define _GNU_SOURCE
+#include <sched.h>
+#include <unistd.h>
+#include <sys/types.h>
+#include <sys/utsname.h>
+#include <stdlib.h>
+
+struct version
+{
+  long major;
+  long minor;
+  long patch;
+};
+
+/* Probe the kernel version into V, and return 0 on success.  */
+
+static int
+probe_kernel_version (struct version *v)
+{
+  struct utsname buffer;
+
+  if (uname (&buffer) == 0)
+    {
+      char *start, *end;
+
+      start = buffer.release;
+      v->major = strtol (start, &end, 10);
+
+      start = end + 1;
+      v->minor = strtol (start, &end, 10);
+
+      start = end + 1;
+      v->patch = strtol (start, &end, 10);
+      return 0;
+    }
+  else
+    return -1;
+}
+
+#define VERSION_NEWER_THAN(VER, MAJOR, MINOR, PATCH) \
+  VER.major == MAJOR && VER.minor == MINOR && VER.patch >= PATCH
+
+#endif
+
+static void
+set_process_affinity (void)
+{
+#if defined(__arm__) && defined(__linux__)
+  struct version kernel;
+  cpu_set_t my_set;
+
+  if (probe_kernel_version (&kernel))
+    {
+      /* Can't get kernel version, do nothing.  */
+      return;
+    }
+
+  if (kernel.major >= 5
+      || (kernel.major == 4 && kernel.minor >= 7) /* 4.7 and later */
+      || VERSION_NEWER_THAN (kernel, 4, 6, 3)
+      || VERSION_NEWER_THAN (kernel, 4, 4, 14)
+      || VERSION_NEWER_THAN (kernel, 4, 1, 27)
+      || VERSION_NEWER_THAN (kernel, 3, 18, 36)
+      || VERSION_NEWER_THAN (kernel, 3, 14, 73))
+    {
+      /* Kernel is new enough to have bug fixed, do nothing.  */
+      return;
+    }
+
+  /* Set both process and parent process (GDB)'s affinity on core 0 to
+     workaround ARM linux kernel ptrace bug which doesn't flush the VFP
+     state to hardware after ptrace set VFP registers.  */
+
+  CPU_ZERO (&my_set);
+  CPU_SET (0, &my_set);
+
+  sched_setaffinity (0, sizeof(cpu_set_t), &my_set);
+  sched_setaffinity (getppid (), sizeof(cpu_set_t), &my_set);
+#endif
+}

Re: [RFC] Set process affinity in test to work around ARM ptrace bug

[Yao Qi]

Ping.

On Mon, Jul 4, 2016 at 11:49 AM, Yao Qi <qiyaoltc@gmail.com> wrote:
> Pedro Alves <palves@redhat.com> writes:
>
>> I also think that whatever workaround, if any, should be limited
>> to known-broken kernels.  Otherwise, this is likely to mask
>> other problems going forward.  Maybe all we have is the version
>> number to work with, but that's still better than unconditionally
>> enabling this on arm.
>
> The updated version adds a linux kernel version check.
>
> --
> Yao (齐尧)
> From 27fe094e6a99929f8f281d88beaa599771550025 Mon Sep 17 00:00:00 2001
> From: Yao Qi <yao.qi@linaro.org>
> Date: Mon, 27 Jun 2016 08:45:16 +0100
> Subject: [PATCH] Set process affinity in test to work around ARM ptrace bug
>
> We recently found a ARM kernel ptrace bug
> http://lists.infradead.org/pipermail/linux-arm-kernel/2016-May/431962.html
> As a result of this bug, after GDB ptrace set VFP registers, the hardware
> registers may not be updated.  This bug causes some intermittent fails in
> tests, like return.exp, call-rt-st.exp, callfuncs.exp, etc.
>
> The bug was introduced by 8130b9d7b9d858aa04ce67805e8951e3cb6e9b2f
> in 2012 and is fixed in e2dfb4b880146bfd4b6aa8e138c0205407cebbaf in May.
> The bug is fixed in ARM kernel tree, but it is impractical to upgrade
> linux kernel from git tree or most recently release.  I am wondering
> we can workaround this kernel bug somehow.
>
> My first attempt is to workaround it in GDB, so that GDB still writes
> the VFP registers and sync them to hardware.  The kernel patch is quite
> simple, which moves vfp_flush_hwstate one line below.  Probably, we can
> call ptrace set vfp registers twice, and then the second vfp set can
> flush the state correctly.  Unfortunately, it doesn't work, because
> every time of ptrace set, kernel loads VFP registers from hardware first,
> which might be out of date after the first ptrace set.  That is to say,
> we can't workaround this kernel bug in GDB.
>
> Then, I am thinking we can workaround this bug in testing, because the
> intermittent fails are confusing in comparing test results.  We can bind
> both tracer and tracee on the same core.  For example, we can start GDB
> or GDBserver with "taskset -c 0 ", but this is a global change, may
> have some affects on gdb.threads tests.  I also think about doing
> "taskset -p PID -c 0" in test harness after the inferior is started,
> and do the same to the parent process of inferior (which is either GDB
> or GDBserver), but don't know how to get GDB (in remote host) and
> GDBserver's process id.
>
> The approach in this patch is to have a small c function which sets
> both process affinity and its parent's affinity to core 0 if the target
> is arm linux and the kernel version is known broken having the ptrace
> bug setting VFP registers.  The function set_process_affinity should
> be called in these tests explicitly, but other tests are not affected
> at all.
>
> Note that this kernel bug only exists between commits
> 8130b9d7b9d858aa04ce67805e8951e3cb6e9b2f and e2dfb4b880146bfd4b6aa8e138c0205407cebbaf
> However, a certain commit will be merged to many branches and releases,
> which makes version checks complicated.  I checked all released kernels,
> and get a list of versions that this bug is fixed.  Not all longterm
> kernels on kernel.org have this bug fix, I don't know why, for example,
> some 3.x kernels doesn't have this bug fix.
>
> Secondly, kernels older than 8130b9d7b9d858aa04ce67805e8951e3cb6e9b2f
> are not affected by this bug, so the official kernel releases older
> than 3.0.21 or 3.2.6 are not affected by this bug, but I think the
> distro may backport the commit to their older kernel, so it makes few
> sense to check kernel is older than some versions (3.0.21 and 3.2.6).
>
> gdb/testsuite:
>
> 2016-07-04  Yao Qi  <yao.qi@linaro.org>
>
>         * lib/set_process_affinity.c: New file.
>
>         * gdb.arch/arm-neon.c: Include lib/set_process_affinity.c.
>         (main): Call set_process_affinity.
>         * gdb.base/callfuncs.c: Likewise.
>         * gdb.base/call-rt-st.c: Likewise.
>         * gdb.base/gnu_vector.c: Likewise.
>         * gdb.base/return.c: Likewise.
>         * gdb.base/return2.c: Likewise.
>         * gdb.base/store.c: Likewise.
>         * gdb.base/structs.c: Likewise.
>         * gdb.arch/arm-neon.exp: Set breakpoint and continue to
>         breakpoint.
>         * gdb.base/gnu_vector.exp: Likewise.
>

-- 
Yao (齐尧)

Re: [RFC] Set process affinity in test to work around ARM ptrace bug

[Pedro Alves]

On 07/25/2016 02:22 PM, Yao Qi wrote:
> Ping.

Thanks.

Hmm.  Seeing that the kernel fix was backported to so many
stable releases (positively) surprised me.  In that case, I question
the testsuite workaround a bit harder.  If this was a workaround in
gdb or gdbserver themselves, then it be more clear to me that the workaround
would be going to a broad set of users for whom updating the kernel is not easy.

But since this is only for when running the testsuite alone, I could argue that
this masks the problem and thus makes it look like gdb works better on an
affected system than it really does.  I think if I were working on gdb/gdbserver
on arm, I'd much prefer if gdb told me my system had a broken ptrace, so I
could act on it, rather than masking it off and pretend all is well.
How about we make gdb / gdbserver detect bad kernel version, and output a
warning to the effect?  We already have precedent in nat/linux-ptrace.c.
I think we should probably do that regardless of any testsuite workaround.

How bad would it be to push for people to update their kernels?


From a testsuite workaround angle, instead of sprinkling 
set_process_affinity calls around, what if we we added a new proc
that would be called at the top of the .exp files:

gdb_caching_proc skip_arm_vfp_tests {} {

  if arm && linux && broken linux versions {
     return 1
  }
  
  return 0
}

This would skip tests instead of making them pass, but how bad would
that be?  I assume that people doing gdb development/testing on arm will
be able to update their kernels, and will very much want to do that.

Thanks,
Pedro Alves

Re: [RFC] Set process affinity in test to work around ARM ptrace bug

[Yao Qi]

Pedro Alves <palves@redhat.com> writes:

This work is interrupted by 7.12 release.  Now, I can visit this problem
again.

> But since this is only for when running the testsuite alone, I could argue that
> this masks the problem and thus makes it look like gdb works better on an
> affected system than it really does.  I think if I were working on gdb/gdbserver
> on arm, I'd much prefer if gdb told me my system had a broken ptrace, so I
> could act on it, rather than masking it off and pretend all is well.
> How about we make gdb / gdbserver detect bad kernel version, and output a
> warning to the effect?  We already have precedent in
> nat/linux-ptrace.c.

Do you mean linux_ptrace_test_ret_to_nx_instr?

> I think we should probably do that regardless of any testsuite workaround.
>

Yes, I agree.  I'll write a test in nat/linux-ptrace.c too.

> How bad would it be to push for people to update their kernels?
>

I am pushing people to update kernels.  I've updated the kernels on some
boards I am using, but there are many boxes which is difficult to update
kernel.

>
> From a testsuite workaround angle, instead of sprinkling 
> set_process_affinity calls around, what if we we added a new proc
> that would be called at the top of the .exp files:
>
> gdb_caching_proc skip_arm_vfp_tests {} {
>
>   if arm && linux && broken linux versions {
>      return 1
>   }
>   
>   return 0
> }
>
> This would skip tests instead of making them pass, but how bad would
> that be?  I assume that people doing gdb development/testing on arm will
> be able to update their kernels, and will very much want to do that.

I happen to see there is a proc gdb_skip_float_test, so I write a patch
to detect broken kernel ptrace in it, and use gdb_skip_float_test all
over the test cases.  How about the patch below?

-- 
Yao (齐尧)

From: Yao Qi <yao.qi@linaro.org>
Date: Thu, 1 Sep 2016 14:39:47 +0100
Subject: [PATCH] Detect broken ptrace in gdb_skip_float_test

We recently found a ARM kernel ptrace bug
http://lists.infradead.org/pipermail/linux-arm-kernel/2016-May/431962.html
Details can be found in the comment in gdb_skip_float_test.  We can
skip floating point tests if the kernel bug is detected.

This patch adds more code in gdb_skip_float_test to detect the broken
ptrace on arm-linux.  Such detection should be done at the beginning
of the test, because it starts a fresh GDB, so change the test cases
to invoke gdb_skip_float_test at the beginning of test, and use its
return value afterwards.

Since gdb_skip_float_test becomes a gdb_caching_proc, so it can't
have an argument, this patch also removes argument "msg", which isn't
useful.

gdb/testsuite:

2016-09-01  Yao Qi  <yao.qi@linaro.org>

	* gdb.arch/arm-neon.exp: Skip it if gdb_skip_float_test returns
	true.
	* gdb.base/call-ar-st.exp: Invoke gdb_skip_float_test.
	* gdb.base/call-rt-st.exp: Likewise.
	* gdb.base/call-sc.exp: Invoke gdb_skip_float_test and use its
	return value instead of gdb,skip_float_test.
	* gdb.base/callfuncs.exp: Invoke gdb_skip_float_test.
	(do_function_calls): Use its return value instead of
	gdb,skip_float_test.
	* gdb.base/finish.exp: Likewise.
	* gdb.base/funcargs.exp: Likewise.
	* gdb.base/return.exp: Likewise.
	* gdb.base/return2.exp: Likewise.
	* gdb.base/varargs.exp: Likewise.
	* lib/gdb.exp (gdb_skip_float_test): Change it to
	gdb_caching_proc.  Detect the broken ptrace on arm-linux.

diff --git a/gdb/testsuite/gdb.arch/arm-neon.exp b/gdb/testsuite/gdb.arch/arm-neon.exp
index 053170f..e4612f9 100644
--- a/gdb/testsuite/gdb.arch/arm-neon.exp
+++ b/gdb/testsuite/gdb.arch/arm-neon.exp
@@ -20,6 +20,11 @@ if {![istarget "aarch64*-*-*"] && ![istarget "arm*-*-*"]} {
     return
 }
 
+if { [gdb_skip_float_test] } {
+    verbose "Skipping ${gdb_test_file_name}."
+    return
+}
+
 standard_testfile
 if { [prepare_for_testing ${testfile}.exp ${testfile} ${srcfile} {debug quiet}] } {
     unsupported "ARM NEON is not supported"
diff --git a/gdb/testsuite/gdb.base/call-ar-st.exp b/gdb/testsuite/gdb.base/call-ar-st.exp
index 28436d5..c63935a 100644
--- a/gdb/testsuite/gdb.base/call-ar-st.exp
+++ b/gdb/testsuite/gdb.base/call-ar-st.exp
@@ -33,6 +33,8 @@ if [get_compiler_info] {
     return -1
 }
 
+set skip_float_test [gdb_skip_float_test]
+
 if {[prepare_for_testing $testfile.exp $testfile $srcfile debug]} {
     untested $testfile.exp
     return -1
@@ -65,7 +67,7 @@ gdb_test continue \
 
 
 #call print_double_array(double_array)
-if {![gdb_skip_float_test "print print_double_array(double_array)"] && \
+if {!$skip_float_test && \
     ![gdb_skip_stdio_test "print print_double_array(double_array)"] } {
 
     gdb_test_stdio "print print_double_array(double_array)" \
@@ -126,7 +128,7 @@ gdb_test "tbreak $stop_line" \
 	"Temporary breakpoint.* file .*$srcfile, line $stop_line.*" \
 	"tbreakpoint at tbreak3"
 
-if {![gdb_skip_float_test "continuing to tbreak3"] && \
+if {!$skip_float_test && \
     ![gdb_skip_stdio_test "continuing to tbreak3"] } {
 
     gdb_test_stdio "continue" \
@@ -168,7 +170,7 @@ if ![gdb_skip_stdio_test "next over print_int_array in print_all_arrays"] {
 }
 
 #call print_double_array(array_d)
-if {![gdb_skip_float_test "print print_double_array(array_d)"] && \
+if {!$skip_float_test && \
     ![gdb_skip_stdio_test "print print_double_array(array_d)"] } {
 
     gdb_test_stdio "print print_double_array(array_d)" \
@@ -186,7 +188,7 @@ gdb_test "tbreak $stop_line" \
 "Temporary breakpoint.* file .*$srcfile, line $stop_line.*" \
 "tbreakpoint at tbreak4"
 
-if {![gdb_skip_float_test "continuing to tbreak4"] && \
+if {!$skip_float_test && \
     ![gdb_skip_stdio_test "continuing to tbreak4"] } {
 
     gdb_test_stdio "continue" \
@@ -305,7 +307,7 @@ if ![gdb_skip_stdio_test "continuing to tbreak6"] {
 #                         *flags, *flags_combo, *three_char, *five_char, 
 #                         *int_char_combo, *d1, *d2, *d3, *f1, *f2, *f3)
 
-if {![gdb_skip_float_test "print print_small_structs(...)"] && \
+if {!$skip_float_test && \
     ![gdb_skip_stdio_test "print print_small_structs(...)"] } {
     gdb_test_stdio "print print_small_structs(*struct1, *struct2, *struct3, *struct4, *flags, *flags_combo, *three_char, *five_char, *int_char_combo, *d1, *d2, *d3, *f1, *f2, *f3)" \
 	[multi_line \
@@ -369,7 +371,7 @@ gdb_test "print compute_with_small_structs(20)" \
 #call print_ten_doubles(123.456, 123.456, -0.12, -1.23, 343434.8, 89.098, 
 #                       3.14, -5678.12345, -0.11111111, 216.97065)
 
-if {![gdb_skip_float_test "print print_ten_doubles(...)"] && \
+if {!$skip_float_test && \
     ![gdb_skip_stdio_test "print print_ten_doubles(...)"]} {
     gdb_test_stdio "print print_ten_doubles(123.456, 123.456, -0.12, -1.23, 343434.8, 89.098, 3.14, -5678.12345, -0.11111111, 216.97065)" \
 	[multi_line \
@@ -397,7 +399,7 @@ gdb_test "tbreak print_long_arg_list" \
 # The short match case below handles cases where a buffer
 # overflows or something, and expect can't deal with the full
 # line.  Perhaps a more elegant solution exists... -sts 1999-08-17
-if {![gdb_skip_float_test "step into print_long_arg_list"]} {
+if {!$skip_float_test} {
     gdb_test_multiple "continue" "step into print_long_arg_list" {
 	-re ".*print_long_arg_list \\(a=22.25, b=33.375, c=0, d=-25, e=100, f=2345, struct1=\{value = 6, head = 0\}, struct2=\{value = 10, head = 0\}, struct3=\{value = 12, head = 0\}, struct4=\{value = 14, head = 0\}, flags=\{alpha = 1, beta = 0, gamma = 1, delta = 0, epsilon = 1, omega = 0\}, flags_combo=\{alpha = 1, beta = 0, ch1 = 121 \'y\', gamma = 1, delta = 0, ch2 = 110 \'n\', epsilon = 1, omega = 0\}, three_char=\{ch1 = 97 \'a\', ch2 = 98 \'b\', ch3 = 99 \'c\'\}, five_char=\{ch1 = 108 \'l\', ch2 = 109 \'m\', ch3 = 110 \'n\', ch4 = 111 \'o\', ch5 = 112 \'p\'\}, int_char_combo=\{int1 = 123, ch1 = 122 \'z\'\}, d1=\{double1 = 10.5\}, d2=\{double1 = -3.375\}, d3=\{double1 = 675.09375\}, f1=\{float1 = 45.2340012, float2 = 43.5999985\}, f2=\{float1 = 78.0100021, float2 = 122.099998\}, f3=\{float1 = -1232.34497, float2 = -199.210007\}\\) at .*${srcfile}:$stop_line\[\r\n\]+$stop_line\[ \t\]+printf\\(\"double :.*\", a\\);.*$gdb_prompt $" {
 	    pass "step into print_long_arg_list"
@@ -419,7 +421,7 @@ set ws "\[\n\r\t \]+"
 #                         flags_combo, three_char, five_char, int_char_combo, 
 #                         d1, d2, d3, f1, f2, f3)
 
-if {![gdb_skip_float_test "print_small_structs from print_long_arg_list"] && \
+if {!$skip_float_test && \
     ![gdb_skip_stdio_test "print_small_structs from print_long_arg_list"] } {
 
     # On 32-bit SPARC, some of the args are passed by ref, others by
@@ -520,7 +522,7 @@ gdb_test continue "Continuing\\..*main \\(\\) at .*$srcfile:$stop_line\[\r\n\t \
 
 #call print_long_arg_list(a, b, c, d, e, f, *struct1, *struct2, *struct3, *struct4, *flags, *flags_combo, *three_char, *five_char, *int_char_combo, *d1, *d2, *d3, *f1, *f2, *f3)
 
-if {![gdb_skip_float_test "print print_long_arg_list"] && \
+if {!$skip_float_test && \
     ![gdb_skip_stdio_test "print print_long_arg_list"] } {
 
     gdb_test_stdio "print print_long_arg_list(a, b, c, d, e, f, *struct1, *struct2, *struct3, *struct4, *flags, *flags_combo, *three_char, *five_char, *int_char_combo, *d1, *d2, *d3, *f1, *f2, *f3)" \
diff --git a/gdb/testsuite/gdb.base/call-rt-st.exp b/gdb/testsuite/gdb.base/call-rt-st.exp
index 0f9c5e8..a92ba9e 100644
--- a/gdb/testsuite/gdb.base/call-rt-st.exp
+++ b/gdb/testsuite/gdb.base/call-rt-st.exp
@@ -44,6 +44,8 @@ if [target_info exists gdb,cannot_call_functions] {
     continue
 }
 
+set skip_float_test [gdb_skip_float_test]
+
 # Start with a fresh gdb.
 
 clean_restart ${binfile}
@@ -128,14 +130,14 @@ if ![gdb_skip_stdio_test "print print_one_large_struct(...)"] {
 	".\[0-9\]+ = \\{next_index = \\{1, 2, 3, 4, 5, 6, 7, 8, 9, 10\\}, values = \\{4, 6, 8, 10, 12, 14, 16, 18, 20, 22\\}, head = 0\\}"
 }
 
-if {![gdb_skip_float_test "print print_one_double(*d1)"] && \
+if {!$skip_float_test && \
         ![gdb_skip_stdio_test "print print_one_double(*d1)"] } {
     print_struct_call "print_one_double(*d1)" \
 	".*Contents of one_double_t:\[ \r\n\]+1\\.111110\[ \r\n\]+" \
 	".\[0-9\]+ = \\{double1 = 1\\.111\[0-9\]*\\}"
 }
 
-if {![gdb_skip_float_test "print print_two_floats(*f3)"] && \
+if {!$skip_float_test && \
         ![gdb_skip_stdio_test "print print_two_floats(*f3)"] } {
     print_struct_call "print_two_floats(*f3)" \
 	".*Contents of two_floats_t:\[ \r\n\]+-2\\.345000\[ \t]+1\\.000000\[ \r\n\]+" \
diff --git a/gdb/testsuite/gdb.base/call-sc.exp b/gdb/testsuite/gdb.base/call-sc.exp
index 7592d65..89065e1 100644
--- a/gdb/testsuite/gdb.base/call-sc.exp
+++ b/gdb/testsuite/gdb.base/call-sc.exp
@@ -35,6 +35,7 @@ standard_testfile .c
 if [get_compiler_info] {
     return -1
 }
+set skip_float_test [gdb_skip_float_test]
 
 # Compile a variant of scalars.c using TYPE to specify the type of the
 # parameter and return-type.  Run the compiled program up to "main".
@@ -427,7 +428,7 @@ start_scalars_test tll
 test_scalar_calls
 test_scalar_returns
 
-if ![target_info exists gdb,skip_float_tests] {
+if {!$skip_float_test} {
     # Approx size: 4, 8, ...
     start_scalars_test tf
     test_scalar_calls
diff --git a/gdb/testsuite/gdb.base/callfuncs.exp b/gdb/testsuite/gdb.base/callfuncs.exp
index 1ec33d8..b108952 100644
--- a/gdb/testsuite/gdb.base/callfuncs.exp
+++ b/gdb/testsuite/gdb.base/callfuncs.exp
@@ -30,6 +30,8 @@ if [target_info exists gdb,cannot_call_functions] {
     continue
 }
 
+set skip_float_test [gdb_skip_float_test]
+
 # FIXME:  Before calling this proc, we should probably verify that
 # we can call inferior functions and get a valid integral value
 # returned.
@@ -38,7 +40,7 @@ if [target_info exists gdb,cannot_call_functions] {
 # (computed in the inferior) is 1 for true and 0 for false.
 
 proc do_function_calls {} {
-    global gdb_prompt
+    global gdb_prompt skip_float_test
 
     # We need to up this because this can be really slow on some boards.
     set timeout 60
@@ -71,7 +73,7 @@ proc do_function_calls {} {
     gdb_test "p t_long_values(789,long_val2)" " = 1"
     gdb_test "p t_long_values(long_val1,-321)" " = 1"
 
-    if ![target_info exists gdb,skip_float_tests] {
+    if {!$skip_float_test} {
 	gdb_test "p t_float_values(0.0,0.0)" " = 0"
 
 	# These next four tests fail on the mn10300.
@@ -199,7 +201,7 @@ proc do_function_calls {} {
     gdb_test "p t_structs_l(struct_val1)" "= 51" \
 	"call inferior func with struct - returns long"
 
-    if ![target_info exists gdb,skip_float_tests] {
+    if {!$skip_float_test} {
 	gdb_test "p t_structs_f(struct_val1)" "= 2.12.*" \
 	    "call inferior func with struct - returns float"
 	gdb_test "p t_structs_d(struct_val1)" "= 9.87.*" \
diff --git a/gdb/testsuite/gdb.base/finish.exp b/gdb/testsuite/gdb.base/finish.exp
index 47bf6f8..c5387bb 100644
--- a/gdb/testsuite/gdb.base/finish.exp
+++ b/gdb/testsuite/gdb.base/finish.exp
@@ -15,6 +15,7 @@
 
 # This file was written by Michael Snyder (msnyder@redhat.com)
 
+set skip_float_test [gdb_skip_float_test]
 
 # re-use the program from the "return2" test.
 if { [prepare_for_testing finish.exp finish return2.c] } {
@@ -86,7 +87,7 @@ proc finish_abbreviation { abbrev } {
 }
 
 proc finish_tests { } {
-    global gdb_prompt
+    global gdb_prompt skip_float_test
 
     if { ! [ runto_main ] } then {
 	untested finish.exp
@@ -99,7 +100,7 @@ proc finish_tests { } {
     finish_1 "int"
     finish_1 "long"
     finish_1 "long_long"
-    if ![target_info exists gdb,skip_float_tests] {
+    if {!$skip_float_test} {
 	finish_1 "float"
 	finish_1 "double"
     }
diff --git a/gdb/testsuite/gdb.base/funcargs.exp b/gdb/testsuite/gdb.base/funcargs.exp
index 792ca9e..fe04b2f 100644
--- a/gdb/testsuite/gdb.base/funcargs.exp
+++ b/gdb/testsuite/gdb.base/funcargs.exp
@@ -29,6 +29,8 @@ if [get_compiler_info] {
     return -1
 }
 
+set skip_float_test [gdb_skip_float_test]
+
 if {[prepare_for_testing $testfile.exp $testfile $srcfile $compile_flags]} {
     untested $testfile.exp
     return -1
@@ -1155,7 +1157,7 @@ gdb_test_no_output "set print frame-arguments all"
 
 integral_args
 unsigned_integral_args
-if {![target_info exists gdb,skip_float_tests]} {
+if {!$skip_float_test} {
   float_and_integral_args
 }
 
@@ -1165,7 +1167,7 @@ if [support_complex_tests] {
 
     complex_integral_args
 
-    if {![target_info exists gdb,skip_float_tests]} {
+    if {!$skip_float_test} {
 	complex_float_integral_args
     }
 }
diff --git a/gdb/testsuite/gdb.base/return.exp b/gdb/testsuite/gdb.base/return.exp
index 95748eb..63cccf2 100644
--- a/gdb/testsuite/gdb.base/return.exp
+++ b/gdb/testsuite/gdb.base/return.exp
@@ -19,8 +19,10 @@ if { [prepare_for_testing return.exp "return"] } {
     return -1
 }
 
+set skip_float_test [gdb_skip_float_test]
+
 proc return_tests { } {
-    global gdb_prompt
+    global gdb_prompt skip_float_test
 
 
     if { ! [ runto func1 ] } then { return 0 }
@@ -88,7 +90,7 @@ proc return_tests { } {
     # is not xfailed.
 
     setup_xfail "sparc-*-solaris2.3*" "sparc-*-solaris2.4*" "m6811-*-*"
-    if ![target_info exists gdb,skip_float_tests] {
+    if {!$skip_float_test} {
 	gdb_test "p tmp3" ".* = 5.*" \
 	    "correct value returned double test (known problem with sparc solaris)"
     }
diff --git a/gdb/testsuite/gdb.base/return2.exp b/gdb/testsuite/gdb.base/return2.exp
index d6ff283..77be75e 100644
--- a/gdb/testsuite/gdb.base/return2.exp
+++ b/gdb/testsuite/gdb.base/return2.exp
@@ -23,6 +23,8 @@ if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {deb
      return -1
 }
 
+set skip_float_test [gdb_skip_float_test]
+
 proc return_1 { type } {
     global gdb_prompt
 
@@ -77,7 +79,7 @@ proc return_void { } {
 }
 
 proc return2_tests { } {
-    global gdb_prompt
+    global gdb_prompt skip_float_test
 
     if { ! [ runto_main ] } then {
 	untested return2.exp
@@ -92,7 +94,7 @@ proc return2_tests { } {
     if { ! [istarget "m6811-*-*"] && ![istarget "h8300*-*"] } then {
         return_1 "long_long"
     }
-    if ![target_info exists gdb,skip_float_tests] {
+    if {!$skip_float_test} {
 	return_1 "float"
 	if { ! [istarget "m6811-*-*"] } then {
 	    return_1 "double"
diff --git a/gdb/testsuite/gdb.base/varargs.exp b/gdb/testsuite/gdb.base/varargs.exp
index f400541..36b0107 100644
--- a/gdb/testsuite/gdb.base/varargs.exp
+++ b/gdb/testsuite/gdb.base/varargs.exp
@@ -37,6 +37,8 @@ if [get_compiler_info] {
     return -1
 }
 
+set skip_float_test [gdb_skip_float_test]
+
 set additional_flags {debug}
 if [support_complex_tests] {
     lappend additional_flags "additional_flags=-DTEST_COMPLEX"
@@ -87,7 +89,7 @@ gdb_test_stdio "print find_max2(3,1,2,3)" \
     ".\[0-9\]+ = 3" \
     "print find_max2(3,1,2,3)"
 
-if {![target_info exists gdb,skip_float_tests]} {
+if {!$skip_float_test} {
     gdb_test_stdio "print find_max_double(5,1.0,17.0,2.0,3.0,4.0)" \
 	"find_max\\(.*\\) returns 17\\.000000\[ \r\n\]+" \
 	".\[0-9\]+ = 17" \
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index b7b8fad..5cab774 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -4882,14 +4882,102 @@ proc rerun_to_main {} {
   }
 }
 
-# Print a message and return true if a test should be skipped
-# due to lack of floating point suport.
+# Return true if a test should be skipped due to lack of floating
+# point support or GDB can't fetch the contents from floating point
+# registers.
 
-proc gdb_skip_float_test { msg } {
+gdb_caching_proc gdb_skip_float_test {
     if [target_info exists gdb,skip_float_tests] {
-	verbose "Skipping test '$msg': no float tests."
 	return 1
     }
+
+    # There is an ARM kernel ptrace bug that hardware VFP registers
+    # are not updated after GDB ptrace set VFP registers.  The bug
+    # was introduced by kernel commit 8130b9d7b9d858aa04ce67805e8951e3cb6e9b2f
+    # in 2012 and is fixed in e2dfb4b880146bfd4b6aa8e138c0205407cebbaf
+    # in May 2016.  In other words, kernels older than 4.6.3, 4.4.14,
+    # 4.1.27, 3.18.36, and 3.14.73 have this bug.
+    # This kernel bug is detected by check how does GDB change the
+    # program result by changing one VFP register.
+    if { [istarget "arm*-*-linux*"] } {
+
+	set compile_flags {debug nowarnings }
+
+	# Set up, compile, and execute a test program having VFP
+	# operations.
+	set src [standard_temp_file arm_vfp[pid].c]
+	set exe [standard_temp_file arm_vfp[pid].x]
+
+	gdb_produce_source $src {
+	    int main() {
+		double d = 4.0;
+		int ret;
+
+		asm ("vldr d0, [%0]" : : "r" (&d));
+		asm ("vldr d1, [%0]" : : "r" (&d));
+		asm (".global break_here\n"
+		     "break_here:");
+		asm ("vcmp.f64 d0, d1\n"
+		     "vmrs APSR_nzcv, fpscr\n"
+		     "bne L_value_different\n"
+		     "movs %0, #0\n"
+		     "b L_end\n"
+		     "L_value_different:\n"
+		     "movs %0, #1\n"
+		     "L_end:\n" : "=r" (ret) :);
+
+		/* Return $d0 != $d1.  */
+		return ret;
+	    }
+	}
+
+	verbose "compiling testfile $src" 2
+	set lines [gdb_compile $src $exe executable $compile_flags]
+	file delete $src
+
+	if ![string match "" $lines] then {
+	    verbose "testfile compilation failed, returning 1" 2
+	    return 0
+	}
+
+	# No error message, compilation succeeded so now run it via gdb.
+	# Run the test up to 5 times to detect whether ptrace can
+	# correctly update VFP registers or not.
+	set skip_vfp_test 0
+	for {set i 0} {$i < 5} {incr i} {
+	    global gdb_prompt srcdir subdir
+
+	    gdb_exit
+	    gdb_start
+	    gdb_reinitialize_dir $srcdir/$subdir
+	    gdb_load "$exe"
+
+	    runto_main
+	    gdb_test "break *break_here"
+	    gdb_continue_to_breakpoint "break_here"
+
+	    # Modify $d0 to a different value, so the exit code should
+	    # be 1.
+	    gdb_test "set \$d0 = 5.0"
+
+	    set test "continue to exit"
+	    gdb_test_multiple "continue" "$test" {
+		-re "exited with code 01.*$gdb_prompt $" {
+		}
+		-re "exited normally.*$gdb_prompt $" {
+		    # However, the exit code is 0.  That means something
+		    # wrong in setting VFP registers.
+		    set skip_vfp_test 1
+		    break
+		}
+	    }
+	}
+
+	gdb_exit
+	remote_file build delete $exe
+
+	return $skip_vfp_test
+    }
     return 0
 }
 

Re: [RFC] Set process affinity in test to work around ARM ptrace bug

[Pedro Alves]

On 09/01/2016 03:48 PM, Yao Qi wrote:
> Pedro Alves <palves@redhat.com> writes:
> 
>> But since this is only for when running the testsuite alone, I could argue that
>> this masks the problem and thus makes it look like gdb works better on an
>> affected system than it really does.  I think if I were working on gdb/gdbserver
>> on arm, I'd much prefer if gdb told me my system had a broken ptrace, so I
>> could act on it, rather than masking it off and pretend all is well.
>> How about we make gdb / gdbserver detect bad kernel version, and output a
>> warning to the effect?  We already have precedent in
>> nat/linux-ptrace.c.
> 
> Do you mean linux_ptrace_test_ret_to_nx_instr?

Yes.

> 
>> I think we should probably do that regardless of any testsuite workaround.
>>
> 
> Yes, I agree.  I'll write a test in nat/linux-ptrace.c too.
> 

> I happen to see there is a proc gdb_skip_float_test, so I write a patch
> to detect broken kernel ptrace in it, and use gdb_skip_float_test all
> over the test cases.  How about the patch below?

I like it.  This version LGTM.  Thanks for adjusting.

-- 
Pedro Alves

Re: [RFC] Set process affinity in test to work around ARM ptrace bug

[Yao Qi]

On Fri, Sep 2, 2016 at 2:00 AM, Pedro Alves <palves@redhat.com> wrote:
>
> I like it.  This version LGTM.  Thanks for adjusting.
>

Patch is pushed in to master.

-- 
Yao (齐尧)