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

Yao Qi
2016-09-01T14:48:42Z

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
}