Patchwork [gdb/testsuite] Fix target_supports_scheduler_locking raciness

login
register
mail settings
Submitter Tom de Vries
Date Oct. 4, 2018, 5:40 p.m.
Message ID <20181004174015.GA20307@delia>
Download mbox | patch
Permalink /patch/29649/
State New
Headers show

Comments

Tom de Vries - Oct. 4, 2018, 5:40 p.m.
Hi,

When calling gdb_start_cmd, it's the caller's responsibility to wait for gdb
to return to the prompt.  In target_supports_scheduler_locking, that's not the
case, and consequently, target_supports_scheduler_locking fails spuriously.

Fix by using runto_main instead.

Build and reg-tested on x86_64-linux.

OK for trunk?

Thanks,
- Tom

[gdb/testsuite] Fix target_supports_scheduler_locking raciness

2018-10-04  Tom de Vries  <tdevries@suse.de>

	* gdb.base/gdb-caching-proc.exp: New file.
	* lib/gdb.exp (target_supports_scheduler_locking): Replace gdb_start_cmd
	with runto_main.

---
 gdb/testsuite/gdb.base/gdb-caching-proc.exp | 111 ++++++++++++++++++++++++++++
 gdb/testsuite/lib/gdb.exp                   |   4 +-
 2 files changed, 114 insertions(+), 1 deletion(-)
Pedro Alves - Oct. 9, 2018, 12:58 p.m.
On 10/04/2018 06:40 PM, Tom de Vries wrote:
> Hi,
> 
> When calling gdb_start_cmd, it's the caller's responsibility to wait for gdb
> to return to the prompt.  In target_supports_scheduler_locking, that's not the
> case, and consequently, target_supports_scheduler_locking fails spuriously.
> 
> Fix by using runto_main instead.
> 
> Build and reg-tested on x86_64-linux.
> 
> OK for trunk?

The fix itself it OK.  Thanks for doing that.  Can you please
submit the gdb-caching-proc.exp separately, with its own rationale?

Thanks,
Pedro Alves

Patch

diff --git a/gdb/testsuite/gdb.base/gdb-caching-proc.exp b/gdb/testsuite/gdb.base/gdb-caching-proc.exp
new file mode 100644
index 0000000000..87c20fc574
--- /dev/null
+++ b/gdb/testsuite/gdb.base/gdb-caching-proc.exp
@@ -0,0 +1,111 @@ 
+#   Copyright 2018 Free Software Foundation, Inc.
+
+# 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/>.
+
+# When caching a proc using gdb_caching_proc, it will become less likely to
+# be executed, and consequently it's going to be harder to detect that the
+# proc is racy.  OTOH, in general the proc is easy to rerun.  So, run all
+# uncached gdb_caching_procs a number of times and detect inconsistent results.
+# The purpose of caching is to reduce runtime, so rerunning is somewhat
+# counter-productive in that aspect, but it's better than uncached, because the
+# number of reruns is constant-bounded, and the increase in runtime is bound to
+# this test-case, and could be disabled on slow targets.
+
+# Test gdb_caching_proc NAME
+proc test_proc { name } {
+    set real_name gdb_real__$name
+
+    set first [$real_name]
+    lappend resultlist $first
+
+    # Ten repetitions was enough to trigger target_supports_scheduler_locking,
+    # and costs about 20 seconds on an i7-6600U.
+    set repeat 10
+
+    set resultlist [list]
+    set racy 0
+    for {set i 0} {$i < $repeat} {incr i} {
+	set rerun [$real_name]
+	lappend resultlist $rerun
+	if { $rerun != $first } {
+	    set racy 1
+	}
+    }
+
+    if { $racy  == 0 } {
+	pass "$name consistency"
+    } else {
+	fail "$name consistency"
+	verbose -log "$name: $resultlist"
+    }
+}
+
+# Test gdb_caching_procs in FILE
+proc test_file { file } {
+    upvar obj obj
+    set procnames [list]
+
+    set fp [open $file]
+    while { [gets $fp line] >= 0 } {
+	if [regexp -- "^gdb_caching_proc \[ \t\]*(\[^ \t\]*)" $line \
+		match procname] {
+	    lappend procnames $procname
+	}
+    }
+    close $fp
+
+    if { [llength $procnames] == 0 } {
+	return
+    }
+
+    if { [file tail $file] == "gdb.exp" } {
+	# Already loaded
+    } else {
+	load_lib [file tail $file]
+    }
+
+    foreach procname $procnames {
+	switch $procname {
+	    "is_address_zero_readable" { set setup_gdb 1 }
+	    "target_is_gdbserver" { set setup_gdb 1 }
+	    default {set setup_gdb 0 }
+	}
+
+	if { $setup_gdb } {
+	    clean_restart $obj
+	}
+
+	test_proc $procname
+
+	if { $setup_gdb } {
+	    gdb_exit
+	}
+    }
+}
+
+# Init
+set me "gdb_caching_proc"
+set src { int main() { return 0; } }
+if { ![gdb_simple_compile $me $src executable] } {
+    return 0
+}
+
+# Test gdb_caching_procs in gdb/testsuite/lib/*.exp
+set files [eval glob -types f $srcdir/lib/*.exp]
+foreach file $files {
+    test_file $file
+}
+
+# Cleanup
+remote_file build delete $obj
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index fbdb436e33..83b83eb23d 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -5971,7 +5971,9 @@  gdb_caching_proc target_supports_scheduler_locking {
     }
 
     clean_restart $obj
-    gdb_start_cmd
+    if ![runto_main] {
+        return 0
+    }
 
     set supports_schedule_locking -1
     set current_schedule_locking_mode ""