public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] Improve and fix catch-syscall.exp
@ 2013-12-13 23:06 Sergio Durigan Junior
  2013-12-15 18:31 ` Doug Evans
  2013-12-16 17:50 ` Pedro Alves
  0 siblings, 2 replies; 12+ messages in thread
From: Sergio Durigan Junior @ 2013-12-13 23:06 UTC (permalink / raw)
  To: GDB Patches

Hi,

While fixing another bug, I found that the current
gdb.base/catch-syscall.exp is kind of messy, could use some
improvements, and is not correctly testing some things.

I've made the following patch to address all the issues I found.  On the
organization side, it does a cleanup and removes unecessary imports of
gdb_prompt, uses prepare_for_testing and clean_restart where needed, and
fixes some comments.  The testcase was also not correctly testing
catching syscalls using only numbers, or catching many syscalls at
once.  I fixed that.  This is good because I will soon submit another
patch to fix a bug on catch syscall which will make use of the new
things I've added.

I tested this on x86_64 Fedora 18, and I'm waiting for machines to test
on PPC and ARM at least, but I checked the syscalls numbers on every
architecture supported by the patch to make sure everything was OK.

OK to apply?

-- 
Sergio

2013-12-13  Sergio Durigan Junior  <sergiodj@redhat.com>

	* gdb.base/catch-syscall.exp: Remove check for "hppa*-hp-hpux*"
	target.  Replace gdb_compile by prepare_for_testing.  Call
	fill_all_syscalls_numbers before starting.  Replace gdb_exit,
	gdb_start, gdb_reinitialize_dir and gdb_load by clean_restart.
	(check_info_bp_any_syscall, check_info_bp_specific_syscall)
	(check_info_bp_many_syscalls, check_call_to_syscall)
	(check_return_from_syscall, check_continue)
	(insert_catch_syscall_with_arg): Remove global gdb_prompt.
	(insert_catch_syscall_with_many_args): Likewise.  Fix $filter_str.
	(check_for_program_end, test_catch_syscall_without_args)
	(test_catch_syscall_with_args, test_catch_syscall_with_many_args)
	(test_catch_syscall_with_wrong_args)
	(test_catch_syscall_restarting_inferior)
	(test_catch_syscall_fail_nodatadir, do_syscall_tests): Remove
	global gdb_prompt.
	(test_catch_syscall_without_args_noxml): Likewise.  Add global
	last_syscall_number.  Test for the exact syscall number to be
	caught.
	(test_catch_syscall_with_args_noxml): Remove global gdb_prompt.
	Add global all_syscalls_numbers.  Test each syscall number to be
	caught, instead of only testing "close".
	(test_catch_syscall_with_wrong_args_noxml): Remove global gdb_prompt.
	(do_syscall_tests_without_xml): Likewise.  Remove stale comment.
	(fill_all_syscalls_numbers): Add global last_syscall_number.
	Fill the correct syscall numbers according to each architecture.

diff --git a/gdb/testsuite/gdb.base/catch-syscall.exp b/gdb/testsuite/gdb.base/catch-syscall.exp
index 7f1bd29..172890c 100644
--- a/gdb/testsuite/gdb.base/catch-syscall.exp
+++ b/gdb/testsuite/gdb.base/catch-syscall.exp
@@ -24,7 +24,7 @@ if { [is_remote target] || ![isnative] } then {
 }
 
 # Until "catch syscall" is implemented on other targets...
-if {![istarget "hppa*-hp-hpux*"] && ![istarget "*-linux*"]} then {
+if { ![istarget "*-linux*"] } {
     continue
 }
 
@@ -40,26 +40,26 @@ if { ![istarget "x86_64-*-linux*"] && ![istarget "i\[34567\]86-*-linux*"]
 
 standard_testfile
 
+if  { [prepare_for_testing ${testfile}.exp $testfile ${testfile}.c] } {
+     untested catch-syscall.exp
+     return -1
+}
+
 # All (but the last) syscalls from the example code
 # They are ordered according to the file, so do not change this.
 set all_syscalls { "close" "chroot" }
 set all_syscalls_numbers { }
+
 # The last syscall (exit()) does not return, so
 # we cannot expect the catchpoint to be triggered
 # twice.  It is a special case.
 set last_syscall "exit_group"
-
-if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug}] != "" } {
-     untested catch-syscall.exp
-     return -1
-}
+set last_syscall_number { }
 
 # Internal procedure used to check if, after issuing a 'catch syscall'
 # command (without arguments), the 'info breakpoints' command displays
 # that '"any syscall"' is to be caught.
 proc check_info_bp_any_syscall {} {
-    global gdb_prompt
-
     # Verifying that the catchpoint appears in the 'info breakpoints'
     # command, but with "<any syscall>".
     set thistest "catch syscall appears in 'info breakpoints'"
@@ -70,8 +70,6 @@ proc check_info_bp_any_syscall {} {
 # command (with arguments), the 'info breakpoints' command displays
 # that the syscall 'X' is to be caught.
 proc check_info_bp_specific_syscall { syscall } {
-    global gdb_prompt
-
     set thistest "syscall(s) $syscall appears in 'info breakpoints'"
     gdb_test "info breakpoints" ".*catchpoint.*keep y.*syscall(\[(\]s\[)\])? (.)?${syscall}(.)?.*" $thistest
 }
@@ -80,7 +78,6 @@ proc check_info_bp_specific_syscall { syscall } {
 # command (with many arguments), the 'info breakpoints' command displays
 # that the syscalls 'X' are to be caught.
 proc check_info_bp_many_syscalls { syscalls } {
-    global gdb_prompt
     set filter_str ""
 
     foreach name $syscalls {
@@ -95,16 +92,12 @@ proc check_info_bp_many_syscalls { syscalls } {
 
 # This procedure checks if there was a call to a syscall.
 proc check_call_to_syscall { syscall } {
-    global gdb_prompt
-
     set thistest "program has called $syscall"
     gdb_test "continue" "Catchpoint .*(call to syscall .?${syscall}.?).*" $thistest
 }
 
 # This procedure checks if the syscall returned.
 proc check_return_from_syscall { syscall } {
-    global gdb_prompt
-
     set thistest "syscall $syscall has returned"
     gdb_test "continue" "Catchpoint .*(returned from syscall (.)?${syscall}(.)?).*" $thistest
 }
@@ -112,8 +105,6 @@ proc check_return_from_syscall { syscall } {
 # Internal procedure that performs two 'continue' commands and checks if
 # a syscall call AND return occur.
 proc check_continue { syscall } {
-    global gdb_prompt
-
     # Testing if the 'continue' stops at the
     # specified syscall_name.  If it does, then it should
     # first print that the infeior has called the syscall,
@@ -127,8 +118,6 @@ proc check_continue { syscall } {
 
 # Inserts a syscall catchpoint with an argument.
 proc insert_catch_syscall_with_arg { syscall } {
-    global gdb_prompt
-
     # Trying to set the catchpoint
     set thistest "catch syscall with arguments ($syscall)"
     gdb_test "catch syscall $syscall" "Catchpoint .*(syscall (.)?${syscall}(.)?( \[\[0-9\]+\])?).*" $thistest
@@ -138,12 +127,11 @@ proc insert_catch_syscall_with_arg { syscall } {
 
 # Inserts a syscall catchpoint with many arguments.
 proc insert_catch_syscall_with_many_args { syscalls numbers } {
-    global gdb_prompt
     set catch [ join $syscalls " " ]
     set filter_str ""
 
     foreach name $syscalls number $numbers {
-      set filter_str "${filter_str}'${name}' \[${number}\] "
+      set filter_str "${filter_str}'${name}' \\\[${number}\\\] "
     }
 
     set filter_str [ string trimright $filter_str " " ]
@@ -156,8 +144,6 @@ proc insert_catch_syscall_with_many_args { syscalls numbers } {
 }
 
 proc check_for_program_end {} {
-    global gdb_prompt
-
     # Deleting the catchpoints
     delete_breakpoints
 
@@ -166,7 +152,7 @@ proc check_for_program_end {} {
 }
 
 proc test_catch_syscall_without_args {} {
-    global gdb_prompt all_syscalls last_syscall
+    global all_syscalls last_syscall
 
     with_test_prefix "without arguments" {
 	# Trying to set the syscall.
@@ -190,8 +176,6 @@ proc test_catch_syscall_without_args {} {
 
 proc test_catch_syscall_with_args {} {
     with_test_prefix "with arguments" {
-	global gdb_prompt
-
 	set syscall_name "close"
 	insert_catch_syscall_with_arg $syscall_name
 
@@ -205,7 +189,7 @@ proc test_catch_syscall_with_args {} {
 
 proc test_catch_syscall_with_many_args {} {
     with_test_prefix "with many arguments" {
-	global gdb_prompt all_syscalls all_syscalls_numbers
+	global all_syscalls all_syscalls_numbers
 
 	insert_catch_syscall_with_many_args $all_syscalls $all_syscalls_numbers
 
@@ -221,8 +205,6 @@ proc test_catch_syscall_with_many_args {} {
 
 proc test_catch_syscall_with_wrong_args {} {
     with_test_prefix "wrong args" {
-	global gdb_prompt
-
 	# mlock is not called from the source
 	set syscall_name "mlock"
 	insert_catch_syscall_with_arg $syscall_name
@@ -237,8 +219,6 @@ proc test_catch_syscall_with_wrong_args {} {
 
 proc test_catch_syscall_restarting_inferior {} {
     with_test_prefix "restarting inferior" {
-	global gdb_prompt
-
 	set syscall_name "chroot"
 
 	with_test_prefix "entry" {
@@ -263,8 +243,6 @@ proc test_catch_syscall_restarting_inferior {} {
 
 proc test_catch_syscall_fail_nodatadir {} {
     with_test_prefix "fail no datadir" {
-	global gdb_prompt
-
 	# Sanitizing.
 	delete_breakpoints
 
@@ -289,8 +267,6 @@ proc test_catch_syscall_fail_nodatadir {} {
 }
 
 proc do_syscall_tests {} {
-    global gdb_prompt srcdir
-
     # NOTE: We don't have to point gdb at the correct data-directory.
     # For the build tree that is handled by INTERNAL_GDBFLAGS.
 
@@ -332,7 +308,7 @@ proc test_catch_syscall_without_args_noxml {} {
     with_test_prefix "without args noxml" {
 	# We will need the syscall names even not using it because we
 	# need to know know many syscalls are in the example file.
-	global gdb_prompt all_syscalls last_syscall
+	global all_syscalls last_syscall_number all_syscalls_numbers
 
 	delete_breakpoints
 
@@ -340,19 +316,15 @@ proc test_catch_syscall_without_args_noxml {} {
 
 	# Now, we should be able to set a catchpoint, and GDB shall
 	# not display the warning anymore.
-	foreach name $all_syscalls {
-	    # Unfortunately, we don't know the syscall number that
-	    # will be caught because this information is
-	    # arch-dependent.  Thus, we try to catch anything similar
-	    # to a number.
+	foreach name $all_syscalls number $all_syscalls_numbers {
 	    with_test_prefix "$name" {
-		check_continue "\[0-9\]*"
+		check_continue $number
 	    }
 	}
 
 	# At last but not least, we check if the inferior has called
 	# the last (exit) syscall.
-	check_call_to_syscall "\[0-9\]*"
+	check_call_to_syscall $last_syscall_number
 
 	delete_breakpoints
     }
@@ -360,25 +332,19 @@ proc test_catch_syscall_without_args_noxml {} {
 
 proc test_catch_syscall_with_args_noxml {} {
     with_test_prefix "with args noxml" {
-	global gdb_prompt
-
-	# The number of the "close" syscall.  This is our option for a
-	# "long-estabilished" syscall in all Linux architectures, but
-	# unfortunately x86_64 and a few other platforms don't "follow
-	# the convention".  Because of this, we need this ugly check
-	# :-(.
-	set close_number ""
-	if { [istarget "x86_64-*-linux*"] } {
-	    set close_number "3"
-	} else {
-	    set close_number "6"
-	}
+	global all_syscalls_numbers
 
 	delete_breakpoints
 
-	insert_catch_syscall_with_arg $close_number
+	# Inserting all syscalls numbers to be caught
+	foreach syscall_number $all_syscalls_numbers {
+	    insert_catch_syscall_with_arg $syscall_number
+	}
 
-	check_continue $close_number
+	# Checking that all syscalls are caught.
+	foreach syscall_number $all_syscalls_numbers {
+	    check_continue $syscall_number
+	}
 
 	delete_breakpoints
     }
@@ -386,8 +352,6 @@ proc test_catch_syscall_with_args_noxml {} {
 
 proc test_catch_syscall_with_wrong_args_noxml {} {
     with_test_prefix "with wrong args noxml" {
-	global gdb_prompt
-
 	delete_breakpoints
 
 	# Even without XML support, GDB should not accept unknown
@@ -400,8 +364,6 @@ proc test_catch_syscall_with_wrong_args_noxml {} {
 }
 
 proc do_syscall_tests_without_xml {} {
-    global gdb_prompt srcdir
-
     # Make sure GDB doesn't load the syscalls xml from the system data
     # directory.
     gdb_test_no_output "set data-directory /the/path/to/nowhere"
@@ -413,12 +375,6 @@ proc do_syscall_tests_without_xml {} {
     # The only valid argument "catch syscall" should accept is the
     # syscall number, and not the name (since it can't translate a
     # name to a number).
-    #
-    # It's worth mentioning that we only try to catch the syscall
-    # close().  This is because the syscall number is an arch-dependent
-    # information, so we can't assume that we know every syscall number
-    # in this system.  Therefore, we have decided to use a "long-estabilished"
-    # system call, and close() just sounded the right choice :-).
     if [runto_main] then { test_catch_syscall_with_args_noxml }
 
     # Now, we'll try to provide a syscall name (valid or not) to the command,
@@ -428,47 +384,58 @@ proc do_syscall_tests_without_xml {} {
 
 # This procedure fills the vector "all_syscalls_numbers" with the proper
 # numbers for the used syscalls according to the architecture.
+#
+# These numbers were taken from the respective <asm/unistd.h> files
+# from each architecture.
 proc fill_all_syscalls_numbers {} {
-    global all_syscalls_numbers
+    global all_syscalls_numbers last_syscall_number
 
     # For Linux on x86, PPC, PPC64, SPARC, SPARC64 and ARM,
     # the numbers for the syscalls "close" and "chroot" are the same.
-    if { [istarget "i\[34567\]86-*-linux*"]
-         || [istarget "powerpc-*-linux*"] || [istarget "powerpc64-*-linux*"]
-         || [istarget "sparc-*-linux*"] || [istarget "sparc64-*-linux*"]
-         || [istarget "arm*-linux*"] } {
-         set all_syscalls_numbers { "6" "61" }
+    # For x86_64, the numbers differ.
+    if { [istarget "x86_64-*-linux*"] } {
+	set all_syscalls_numbers { "3" "161" }
+	set last_syscall_number "231"
+    } elseif { [istarget "i\[34567\]86-*-linux*"]
+	       || [istarget "powerpc-*-linux*"]
+	       || [istarget "powerpc64-*-linux*"]
+	       || [istarget "sparc-*-linux*"]
+	       || [istarget "sparc64-*-linux*"]
+	       || [istarget "arm*-linux*"] } {
+	set all_syscalls_numbers { "6" "61" }
     }
-}
 
-# Start with a fresh gdb
+    #Now, filling the last_syscall_number.  It is the "exit_group" syscall.
+    if { [istarget "arm*-linux*"] } {
+	set last_syscall_number "248"
+    } elseif { [istarget "powerpc64-*-linux*"]
+	       || [istarget "powerpc64-*-linux*"] } {
+	set last_syscall_number "234"
+    } elseif { [istarget "sparc-*-linux*"]
+	       || [istarget "sparc64-*-linux*"] } {
+	set last_syscall_number "188"
+    } elseif { [istarget "i\[34567\]86-*-linux*"] } {
+	set last_syscall_number "252"
+    }
+}
 
-gdb_exit
-set do_xml_test ![gdb_skip_xml_test]
-gdb_start
-gdb_reinitialize_dir $srcdir/$subdir
-gdb_load ${binfile}
+# Filling all the syscalls numbers before starting anything.
+fill_all_syscalls_numbers
 
 # Execute the tests, using XML support
-if $do_xml_test {
+if { ![gdb_skip_xml_test] } {
+  clean_restart $binfile
   do_syscall_tests
 
   # Now, we have to see if GDB displays a warning when we
   # don't set the data-directory but try to use catch syscall
   # anyway.  For that, we must restart GDB first.
-  gdb_exit
-  gdb_start
-  gdb_reinitialize_dir $srcdir/$subdir
-  gdb_load ${binfile}
+  clean_restart $binfile
   test_catch_syscall_fail_nodatadir
 }
 
 # Restart gdb
-
-gdb_exit
-gdb_start
-gdb_reinitialize_dir $srcdir/$subdir
-gdb_load ${binfile}
+clean_restart $binfile
 
 # Execute the tests, without XML support.  In this case, GDB will
 # only display syscall numbers, and not syscall names.

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

* Re: [PATCH] Improve and fix catch-syscall.exp
  2013-12-13 23:06 [PATCH] Improve and fix catch-syscall.exp Sergio Durigan Junior
@ 2013-12-15 18:31 ` Doug Evans
  2013-12-15 18:44   ` Doug Evans
                     ` (2 more replies)
  2013-12-16 17:50 ` Pedro Alves
  1 sibling, 3 replies; 12+ messages in thread
From: Doug Evans @ 2013-12-15 18:31 UTC (permalink / raw)
  To: Sergio Durigan Junior; +Cc: GDB Patches

Sergio Durigan Junior <sergiodj@redhat.com> writes:

> Hi,
>
> While fixing another bug, I found that the current
> gdb.base/catch-syscall.exp is kind of messy, could use some
> improvements, and is not correctly testing some things.
>
> I've made the following patch to address all the issues I found.  On the
> organization side, it does a cleanup and removes unecessary imports of
> gdb_prompt, uses prepare_for_testing and clean_restart where needed, and
> fixes some comments.  The testcase was also not correctly testing
> catching syscalls using only numbers, or catching many syscalls at
> once.  I fixed that.  This is good because I will soon submit another
> patch to fix a bug on catch syscall which will make use of the new
> things I've added.
>
> I tested this on x86_64 Fedora 18, and I'm waiting for machines to test
> on PPC and ARM at least, but I checked the syscalls numbers on every
> architecture supported by the patch to make sure everything was OK.
>
> OK to apply?

Hi.

I was wondering, what if the magic numbers that are the syscall
numbers were recorded in the test .c file like:

int close_syscall_number = foo;

and then have the .exp fetch these values after running-to-main.
That would save having to record syscall numbers in the .exp,
and all the conditionals to test for the architecture.

Not sure there isn't a flaw in this plan,
and I guess it's debatable whether it's better to just record
the numbers in the .exp or reference the __NR_* numbers from asm/unistd*.h
in the .c, but it sounds promising.

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

* Re: [PATCH] Improve and fix catch-syscall.exp
  2013-12-15 18:31 ` Doug Evans
@ 2013-12-15 18:44   ` Doug Evans
  2013-12-16  4:24     ` Sergio Durigan Junior
  2013-12-16  4:20   ` Sergio Durigan Junior
  2013-12-16 23:09   ` Sergio Durigan Junior
  2 siblings, 1 reply; 12+ messages in thread
From: Doug Evans @ 2013-12-15 18:44 UTC (permalink / raw)
  To: Sergio Durigan Junior; +Cc: GDB Patches

On Sun, Dec 15, 2013 at 10:30 AM, Doug Evans <xdje42@gmail.com> wrote:
> Sergio Durigan Junior <sergiodj@redhat.com> writes:
>
>> Hi,
>>
>> While fixing another bug, I found that the current
>> gdb.base/catch-syscall.exp is kind of messy, could use some
>> improvements, and is not correctly testing some things.
>>
>> I've made the following patch to address all the issues I found.  On the
>> organization side, it does a cleanup and removes unecessary imports of
>> gdb_prompt, uses prepare_for_testing and clean_restart where needed, and
>> fixes some comments.  The testcase was also not correctly testing
>> catching syscalls using only numbers, or catching many syscalls at
>> once.  I fixed that.  This is good because I will soon submit another
>> patch to fix a bug on catch syscall which will make use of the new
>> things I've added.
>>
>> I tested this on x86_64 Fedora 18, and I'm waiting for machines to test
>> on PPC and ARM at least, but I checked the syscalls numbers on every
>> architecture supported by the patch to make sure everything was OK.
>>
>> OK to apply?
>
> Hi.
>
> I was wondering, what if the magic numbers that are the syscall
> numbers were recorded in the test .c file like:
>
> int close_syscall_number = foo;
>
> and then have the .exp fetch these values after running-to-main.
> That would save having to record syscall numbers in the .exp,
> and all the conditionals to test for the architecture.
>
> Not sure there isn't a flaw in this plan,
> and I guess it's debatable whether it's better to just record
> the numbers in the .exp or reference the __NR_* numbers from asm/unistd*.h
> in the .c, but it sounds promising.

Alternatively, gdb knows the numbers.  IWBN to provide a way to print
them, and then the .exp file could get the numbers from that.
OTOH, one could just have the .exp do the catch and process the output
to get the number.

(gdb) catch syscall 6
Catchpoint 2 (syscall 'close' [6])
(gdb) catch syscall close
Catchpoint 1 (syscall 'close' [6])

One might even check the .c __NR_ value with gdb's value computed from
the .exp for extra paranoid testing, but I'm guessing there's
insufficient benefit.

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

* Re: [PATCH] Improve and fix catch-syscall.exp
  2013-12-15 18:31 ` Doug Evans
  2013-12-15 18:44   ` Doug Evans
@ 2013-12-16  4:20   ` Sergio Durigan Junior
  2013-12-16 23:09   ` Sergio Durigan Junior
  2 siblings, 0 replies; 12+ messages in thread
From: Sergio Durigan Junior @ 2013-12-16  4:20 UTC (permalink / raw)
  To: Doug Evans; +Cc: GDB Patches

On Sunday, December 15 2013, Doug Evans wrote:

> Hi.
>
> I was wondering, what if the magic numbers that are the syscall
> numbers were recorded in the test .c file like:
>
> int close_syscall_number = foo;
>
> and then have the .exp fetch these values after running-to-main.
> That would save having to record syscall numbers in the .exp,
> and all the conditionals to test for the architecture.

But then we'd have to make the conditionals on the .c file instead,
right?  I mean, we'd just be switching the place of the problem...

> Not sure there isn't a flaw in this plan,
> and I guess it's debatable whether it's better to just record
> the numbers in the .exp or reference the __NR_* numbers from asm/unistd*.h
> in the .c, but it sounds promising.

The .exp file needs to know the syscall numbers (not only the names) in
order to compare them with the output of "catch syscall".  Therefore,
just referencing the syscalls as __NR_* won't help with that...

Unless I'm missing something in your proposal, I don't see how it could
improve the current situation.

-- 
Sergio

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

* Re: [PATCH] Improve and fix catch-syscall.exp
  2013-12-15 18:44   ` Doug Evans
@ 2013-12-16  4:24     ` Sergio Durigan Junior
  0 siblings, 0 replies; 12+ messages in thread
From: Sergio Durigan Junior @ 2013-12-16  4:24 UTC (permalink / raw)
  To: Doug Evans; +Cc: GDB Patches

On Sunday, December 15 2013, Doug Evans wrote:

> Alternatively, gdb knows the numbers.  IWBN to provide a way to print
> them, and then the .exp file could get the numbers from that.
> OTOH, one could just have the .exp do the catch and process the output
> to get the number.
>
> (gdb) catch syscall 6
> Catchpoint 2 (syscall 'close' [6])
> (gdb) catch syscall close
> Catchpoint 1 (syscall 'close' [6])

Yes, it can be done.  But testing for the correctness of this number is
also part of the test, IMO...

> One might even check the .c __NR_ value with gdb's value computed from
> the .exp for extra paranoid testing, but I'm guessing there's
> insufficient benefit.

... and maybe it isn't an insufficient benefit.  I mean, currently
"catch syscall" works on the host only, but it's being implemented in
gdbserver and eventually I guess it'll be a good idea to really check
those numbers against the hard-coded ones.  Currently, they are
hard-coded in the .exp file, but as you said, one could check the __NR_*
value too...

Anyway, just my opinion of why we should keep those numbers hard-coded
somewhere.

-- 
Sergio

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

* Re: [PATCH] Improve and fix catch-syscall.exp
  2013-12-13 23:06 [PATCH] Improve and fix catch-syscall.exp Sergio Durigan Junior
  2013-12-15 18:31 ` Doug Evans
@ 2013-12-16 17:50 ` Pedro Alves
  2013-12-16 17:59   ` Sergio Durigan Junior
  1 sibling, 1 reply; 12+ messages in thread
From: Pedro Alves @ 2013-12-16 17:50 UTC (permalink / raw)
  To: Sergio Durigan Junior; +Cc: GDB Patches

On 12/13/2013 11:05 PM, Sergio Durigan Junior wrote:> diff --git a/gdb/testsuite/gdb.base/catch-syscall.exp b/gdb/testsuite/gdb.base/catch-syscall.exp
> index 7f1bd29..172890c 100644
> --- a/gdb/testsuite/gdb.base/catch-syscall.exp
> +++ b/gdb/testsuite/gdb.base/catch-syscall.exp
> @@ -24,7 +24,7 @@ if { [is_remote target] || ![isnative] } then {
>  }
>  
>  # Until "catch syscall" is implemented on other targets...
> -if {![istarget "hppa*-hp-hpux*"] && ![istarget "*-linux*"]} then {
> +if { ![istarget "*-linux*"] } {
>      continue
>  }

Why's that?  AFAICS, hpux does support catching syscalls, at
least by number I assume should work.  See
TARGET_WAITKIND_SYSCALL_ENTRY, etc. being handled in
inf-ttrace.c.  It might be better to leave the testing exposed
there, even if it might be failing miserably.

Otherwise, looks like good forward progress to me,
irrespective of where the discussion about syscall numbers
leads (seems like even if we got the numbers from the
program, we'd just tweak fill_all_syscalls_numbers), so
other than the above, it looks OK to me.

-- 
Pedro Alves

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

* Re: [PATCH] Improve and fix catch-syscall.exp
  2013-12-16 17:50 ` Pedro Alves
@ 2013-12-16 17:59   ` Sergio Durigan Junior
  0 siblings, 0 replies; 12+ messages in thread
From: Sergio Durigan Junior @ 2013-12-16 17:59 UTC (permalink / raw)
  To: Pedro Alves; +Cc: GDB Patches

On Monday, December 16 2013, Pedro Alves wrote:

> On 12/13/2013 11:05 PM, Sergio Durigan Junior wrote:> diff --git a/gdb/testsuite/gdb.base/catch-syscall.exp b/gdb/testsuite/gdb.base/catch-syscall.exp
>> index 7f1bd29..172890c 100644
>> --- a/gdb/testsuite/gdb.base/catch-syscall.exp
>> +++ b/gdb/testsuite/gdb.base/catch-syscall.exp
>> @@ -24,7 +24,7 @@ if { [is_remote target] || ![isnative] } then {
>>  }
>>  
>>  # Until "catch syscall" is implemented on other targets...
>> -if {![istarget "hppa*-hp-hpux*"] && ![istarget "*-linux*"]} then {
>> +if { ![istarget "*-linux*"] } {
>>      continue
>>  }
>
> Why's that?  AFAICS, hpux does support catching syscalls, at
> least by number I assume should work.  See
> TARGET_WAITKIND_SYSCALL_ENTRY, etc. being handled in
> inf-ttrace.c.  It might be better to leave the testing exposed
> there, even if it might be failing miserably.

Aha, right, I thought it was a "pasto" from somewhere else, but I see
your point.  Thanks for correcting.

> Otherwise, looks like good forward progress to me,
> irrespective of where the discussion about syscall numbers
> leads (seems like even if we got the numbers from the
> program, we'd just tweak fill_all_syscalls_numbers), so
> other than the above, it looks OK to me.

Yeah.  OK then, I will wait for Doug's reply and then push the patch if
he's OK with it.

Thanks,

-- 
Sergio

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

* Re: [PATCH] Improve and fix catch-syscall.exp
  2013-12-15 18:31 ` Doug Evans
  2013-12-15 18:44   ` Doug Evans
  2013-12-16  4:20   ` Sergio Durigan Junior
@ 2013-12-16 23:09   ` Sergio Durigan Junior
  2013-12-17 10:24     ` Pedro Alves
  2 siblings, 1 reply; 12+ messages in thread
From: Sergio Durigan Junior @ 2013-12-16 23:09 UTC (permalink / raw)
  To: Doug Evans; +Cc: GDB Patches

On Sunday, December 15 2013, Doug Evans wrote:

> I was wondering, what if the magic numbers that are the syscall
> numbers were recorded in the test .c file like:
>
> int close_syscall_number = foo;

Can you tell me what you think about this patch?

Thanks.

-- 
Sergio

2013-12-17  Sergio Durigan Junior  <sergiodj@redhat.com>

	* gdb.base/catch-syscall.c: Include <asm/unistd.h>.
	(close_syscall, chroot_syscall, exit_group_syscall): New
	variables.
	* gdb.base/catch-syscall.exp: Replace gdb_compile by
	prepare_for_testing.  Call fill_all_syscalls_numbers before
	starting.  Replace gdb_exit, gdb_start, gdb_reinitialize_dir and
	gdb_load by clean_restart.
	(check_info_bp_any_syscall, check_info_bp_specific_syscall)
	(check_info_bp_many_syscalls): Remove global gdb_prompt.
	(check_call_to_syscall): Likewise.  Add global decimal.  Improve
	testing regex.
	(check_return_from_syscall): Likewise.
	(check_continue, insert_catch_syscall_with_arg): Remove global
	gdb_prompt.
	(insert_catch_syscall_with_many_args): Likewise.  Add global
	decimal.  Fix $filter_str.  Improve testing regex.
	(check_for_program_end): Remove global gdb_prompt.
	(test_catch_syscall_without_args): Likewise.  Add global decimal.
	Improve testing regex.
	(test_catch_syscall_with_args, test_catch_syscall_with_many_args)
	(test_catch_syscall_with_wrong_args)
	(test_catch_syscall_restarting_inferior)
	(test_catch_syscall_fail_nodatadir): Remove global gdb_prompt.
	(do_syscall_tests): Likewise.  Remove global srcdir.
	(test_catch_syscall_without_args_noxml): Remove global gdb_prompt.
	Add global last_syscall_number.  Test for the exact syscall number
	to be caught.
	(test_catch_syscall_with_args_noxml): Remove global gdb_prompt.
	Add global all_syscalls_numbers.  Test each syscall number to be
	caught, instead of only testing "close".
	(test_catch_syscall_with_wrong_args_noxml): Remove global gdb_prompt.
	(do_syscall_tests_without_xml): Likewise.  Remove global srcdir.
	Remove stale comment.
	(fill_all_syscalls_numbers): Add global last_syscall_number.  Fill
	the correct syscall numbers using information from the inferior.

diff --git a/gdb/testsuite/gdb.base/catch-syscall.c b/gdb/testsuite/gdb.base/catch-syscall.c
index 64850de..a4dc10c 100644
--- a/gdb/testsuite/gdb.base/catch-syscall.c
+++ b/gdb/testsuite/gdb.base/catch-syscall.c
@@ -8,9 +8,16 @@
    September, 2008 */
 
 #include <unistd.h>
+#include <asm/unistd.h>
 #include <fcntl.h>
 #include <sys/stat.h>
 
+/* These are the syscalls numbers used by the test.  */
+
+static int close_syscall = __NR_close;
+static int chroot_syscall = __NR_chroot;
+static int exit_group_syscall = __NR_exit_group;
+
 int
 main (void)
 {
diff --git a/gdb/testsuite/gdb.base/catch-syscall.exp b/gdb/testsuite/gdb.base/catch-syscall.exp
index 7f1bd29..c18e878 100644
--- a/gdb/testsuite/gdb.base/catch-syscall.exp
+++ b/gdb/testsuite/gdb.base/catch-syscall.exp
@@ -24,7 +24,7 @@ if { [is_remote target] || ![isnative] } then {
 }
 
 # Until "catch syscall" is implemented on other targets...
-if {![istarget "hppa*-hp-hpux*"] && ![istarget "*-linux*"]} then {
+if { ![istarget "hppa*-hp-hpux*"] && ![istarget "*-linux*"] } {
     continue
 }
 
@@ -40,26 +40,26 @@ if { ![istarget "x86_64-*-linux*"] && ![istarget "i\[34567\]86-*-linux*"]
 
 standard_testfile
 
+if  { [prepare_for_testing ${testfile}.exp $testfile ${testfile}.c] } {
+     untested catch-syscall.exp
+     return -1
+}
+
 # All (but the last) syscalls from the example code
 # They are ordered according to the file, so do not change this.
 set all_syscalls { "close" "chroot" }
 set all_syscalls_numbers { }
+
 # The last syscall (exit()) does not return, so
 # we cannot expect the catchpoint to be triggered
 # twice.  It is a special case.
 set last_syscall "exit_group"
-
-if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug}] != "" } {
-     untested catch-syscall.exp
-     return -1
-}
+set last_syscall_number { }
 
 # Internal procedure used to check if, after issuing a 'catch syscall'
 # command (without arguments), the 'info breakpoints' command displays
 # that '"any syscall"' is to be caught.
 proc check_info_bp_any_syscall {} {
-    global gdb_prompt
-
     # Verifying that the catchpoint appears in the 'info breakpoints'
     # command, but with "<any syscall>".
     set thistest "catch syscall appears in 'info breakpoints'"
@@ -70,8 +70,6 @@ proc check_info_bp_any_syscall {} {
 # command (with arguments), the 'info breakpoints' command displays
 # that the syscall 'X' is to be caught.
 proc check_info_bp_specific_syscall { syscall } {
-    global gdb_prompt
-
     set thistest "syscall(s) $syscall appears in 'info breakpoints'"
     gdb_test "info breakpoints" ".*catchpoint.*keep y.*syscall(\[(\]s\[)\])? (.)?${syscall}(.)?.*" $thistest
 }
@@ -80,7 +78,6 @@ proc check_info_bp_specific_syscall { syscall } {
 # command (with many arguments), the 'info breakpoints' command displays
 # that the syscalls 'X' are to be caught.
 proc check_info_bp_many_syscalls { syscalls } {
-    global gdb_prompt
     set filter_str ""
 
     foreach name $syscalls {
@@ -95,25 +92,23 @@ proc check_info_bp_many_syscalls { syscalls } {
 
 # This procedure checks if there was a call to a syscall.
 proc check_call_to_syscall { syscall } {
-    global gdb_prompt
+    global decimal
 
     set thistest "program has called $syscall"
-    gdb_test "continue" "Catchpoint .*(call to syscall .?${syscall}.?).*" $thistest
+    gdb_test "continue" "Catchpoint $decimal \\(call to syscall .?${syscall}.?\\).*" $thistest
 }
 
 # This procedure checks if the syscall returned.
 proc check_return_from_syscall { syscall } {
-    global gdb_prompt
+    global decimal
 
     set thistest "syscall $syscall has returned"
-    gdb_test "continue" "Catchpoint .*(returned from syscall (.)?${syscall}(.)?).*" $thistest
+    gdb_test "continue" "Catchpoint $decimal \\(returned from syscall ${syscall}\\).*" $thistest
 }
 
 # Internal procedure that performs two 'continue' commands and checks if
 # a syscall call AND return occur.
 proc check_continue { syscall } {
-    global gdb_prompt
-
     # Testing if the 'continue' stops at the
     # specified syscall_name.  If it does, then it should
     # first print that the infeior has called the syscall,
@@ -127,50 +122,48 @@ proc check_continue { syscall } {
 
 # Inserts a syscall catchpoint with an argument.
 proc insert_catch_syscall_with_arg { syscall } {
-    global gdb_prompt
+    global decimal
 
     # Trying to set the catchpoint
     set thistest "catch syscall with arguments ($syscall)"
-    gdb_test "catch syscall $syscall" "Catchpoint .*(syscall (.)?${syscall}(.)?( \[\[0-9\]+\])?).*" $thistest
+    gdb_test "catch syscall $syscall" "Catchpoint $decimal \\(syscall \'?${syscall}\'?( \[${decimal}\])?\\)" $thistest
 
     check_info_bp_specific_syscall $syscall
 }
 
 # Inserts a syscall catchpoint with many arguments.
 proc insert_catch_syscall_with_many_args { syscalls numbers } {
-    global gdb_prompt
+    global decimal
+
     set catch [ join $syscalls " " ]
     set filter_str ""
 
     foreach name $syscalls number $numbers {
-      set filter_str "${filter_str}'${name}' \[${number}\] "
+      set filter_str "${filter_str}'${name}' \\\[${number}\\\] "
     }
 
     set filter_str [ string trimright $filter_str " " ]
 
     # Trying to set the catchpoint
     set thistest "catch syscall with arguments ($filter_str)"
-    gdb_test "catch syscall $catch" "Catchpoint .*(syscalls (.)?${filter_str}(.)?).*" $thistest
+    gdb_test "catch syscall $catch" "Catchpoint $decimal \\(syscalls ${filter_str}\\).*" $thistest
 
     check_info_bp_many_syscalls $syscalls
 }
 
 proc check_for_program_end {} {
-    global gdb_prompt
-
     # Deleting the catchpoints
     delete_breakpoints
 
     gdb_continue_to_end
-
 }
 
 proc test_catch_syscall_without_args {} {
-    global gdb_prompt all_syscalls last_syscall
+    global all_syscalls last_syscall decimal
 
     with_test_prefix "without arguments" {
 	# Trying to set the syscall.
-	gdb_test "catch syscall" "Catchpoint .*(syscall).*"
+	gdb_test "catch syscall" "Catchpoint $decimal \\(any syscall\\)"
 
 	check_info_bp_any_syscall
 
@@ -190,8 +183,6 @@ proc test_catch_syscall_without_args {} {
 
 proc test_catch_syscall_with_args {} {
     with_test_prefix "with arguments" {
-	global gdb_prompt
-
 	set syscall_name "close"
 	insert_catch_syscall_with_arg $syscall_name
 
@@ -205,7 +196,7 @@ proc test_catch_syscall_with_args {} {
 
 proc test_catch_syscall_with_many_args {} {
     with_test_prefix "with many arguments" {
-	global gdb_prompt all_syscalls all_syscalls_numbers
+	global all_syscalls all_syscalls_numbers
 
 	insert_catch_syscall_with_many_args $all_syscalls $all_syscalls_numbers
 
@@ -221,8 +212,6 @@ proc test_catch_syscall_with_many_args {} {
 
 proc test_catch_syscall_with_wrong_args {} {
     with_test_prefix "wrong args" {
-	global gdb_prompt
-
 	# mlock is not called from the source
 	set syscall_name "mlock"
 	insert_catch_syscall_with_arg $syscall_name
@@ -237,8 +226,6 @@ proc test_catch_syscall_with_wrong_args {} {
 
 proc test_catch_syscall_restarting_inferior {} {
     with_test_prefix "restarting inferior" {
-	global gdb_prompt
-
 	set syscall_name "chroot"
 
 	with_test_prefix "entry" {
@@ -263,8 +250,6 @@ proc test_catch_syscall_restarting_inferior {} {
 
 proc test_catch_syscall_fail_nodatadir {} {
     with_test_prefix "fail no datadir" {
-	global gdb_prompt
-
 	# Sanitizing.
 	delete_breakpoints
 
@@ -289,8 +274,6 @@ proc test_catch_syscall_fail_nodatadir {} {
 }
 
 proc do_syscall_tests {} {
-    global gdb_prompt srcdir
-
     # NOTE: We don't have to point gdb at the correct data-directory.
     # For the build tree that is handled by INTERNAL_GDBFLAGS.
 
@@ -332,7 +315,7 @@ proc test_catch_syscall_without_args_noxml {} {
     with_test_prefix "without args noxml" {
 	# We will need the syscall names even not using it because we
 	# need to know know many syscalls are in the example file.
-	global gdb_prompt all_syscalls last_syscall
+	global all_syscalls last_syscall_number all_syscalls_numbers
 
 	delete_breakpoints
 
@@ -340,19 +323,15 @@ proc test_catch_syscall_without_args_noxml {} {
 
 	# Now, we should be able to set a catchpoint, and GDB shall
 	# not display the warning anymore.
-	foreach name $all_syscalls {
-	    # Unfortunately, we don't know the syscall number that
-	    # will be caught because this information is
-	    # arch-dependent.  Thus, we try to catch anything similar
-	    # to a number.
+	foreach name $all_syscalls number $all_syscalls_numbers {
 	    with_test_prefix "$name" {
-		check_continue "\[0-9\]*"
+		check_continue $number
 	    }
 	}
 
 	# At last but not least, we check if the inferior has called
 	# the last (exit) syscall.
-	check_call_to_syscall "\[0-9\]*"
+	check_call_to_syscall $last_syscall_number
 
 	delete_breakpoints
     }
@@ -360,25 +339,19 @@ proc test_catch_syscall_without_args_noxml {} {
 
 proc test_catch_syscall_with_args_noxml {} {
     with_test_prefix "with args noxml" {
-	global gdb_prompt
-
-	# The number of the "close" syscall.  This is our option for a
-	# "long-estabilished" syscall in all Linux architectures, but
-	# unfortunately x86_64 and a few other platforms don't "follow
-	# the convention".  Because of this, we need this ugly check
-	# :-(.
-	set close_number ""
-	if { [istarget "x86_64-*-linux*"] } {
-	    set close_number "3"
-	} else {
-	    set close_number "6"
-	}
+	global all_syscalls_numbers
 
 	delete_breakpoints
 
-	insert_catch_syscall_with_arg $close_number
+	# Inserting all syscalls numbers to be caught
+	foreach syscall_number $all_syscalls_numbers {
+	    insert_catch_syscall_with_arg $syscall_number
+	}
 
-	check_continue $close_number
+	# Checking that all syscalls are caught.
+	foreach syscall_number $all_syscalls_numbers {
+	    check_continue $syscall_number
+	}
 
 	delete_breakpoints
     }
@@ -386,8 +359,6 @@ proc test_catch_syscall_with_args_noxml {} {
 
 proc test_catch_syscall_with_wrong_args_noxml {} {
     with_test_prefix "with wrong args noxml" {
-	global gdb_prompt
-
 	delete_breakpoints
 
 	# Even without XML support, GDB should not accept unknown
@@ -400,8 +371,6 @@ proc test_catch_syscall_with_wrong_args_noxml {} {
 }
 
 proc do_syscall_tests_without_xml {} {
-    global gdb_prompt srcdir
-
     # Make sure GDB doesn't load the syscalls xml from the system data
     # directory.
     gdb_test_no_output "set data-directory /the/path/to/nowhere"
@@ -413,12 +382,6 @@ proc do_syscall_tests_without_xml {} {
     # The only valid argument "catch syscall" should accept is the
     # syscall number, and not the name (since it can't translate a
     # name to a number).
-    #
-    # It's worth mentioning that we only try to catch the syscall
-    # close().  This is because the syscall number is an arch-dependent
-    # information, so we can't assume that we know every syscall number
-    # in this system.  Therefore, we have decided to use a "long-estabilished"
-    # system call, and close() just sounded the right choice :-).
     if [runto_main] then { test_catch_syscall_with_args_noxml }
 
     # Now, we'll try to provide a syscall name (valid or not) to the command,
@@ -428,47 +391,35 @@ proc do_syscall_tests_without_xml {} {
 
 # This procedure fills the vector "all_syscalls_numbers" with the proper
 # numbers for the used syscalls according to the architecture.
+#
+# These numbers were taken from the respective <asm/unistd.h> files
+# from each architecture.
 proc fill_all_syscalls_numbers {} {
-    global all_syscalls_numbers
-
-    # For Linux on x86, PPC, PPC64, SPARC, SPARC64 and ARM,
-    # the numbers for the syscalls "close" and "chroot" are the same.
-    if { [istarget "i\[34567\]86-*-linux*"]
-         || [istarget "powerpc-*-linux*"] || [istarget "powerpc64-*-linux*"]
-         || [istarget "sparc-*-linux*"] || [istarget "sparc64-*-linux*"]
-         || [istarget "arm*-linux*"] } {
-         set all_syscalls_numbers { "6" "61" }
-    }
-}
+    global all_syscalls_numbers last_syscall_number
 
-# Start with a fresh gdb
+    set close_syscall [get_integer_valueof "close_syscall" 0]
+    set chroot_syscall [get_integer_valueof "chroot_syscall" 0]
+    set all_syscalls_numbers [list $close_syscall $chroot_syscall]
+    set last_syscall_number [get_integer_valueof "exit_group_syscall" 0]
+}
 
-gdb_exit
-set do_xml_test ![gdb_skip_xml_test]
-gdb_start
-gdb_reinitialize_dir $srcdir/$subdir
-gdb_load ${binfile}
+# Filling all the syscalls numbers before starting anything.
+fill_all_syscalls_numbers
 
 # Execute the tests, using XML support
-if $do_xml_test {
+if { ![gdb_skip_xml_test] } {
+  clean_restart $binfile
   do_syscall_tests
 
   # Now, we have to see if GDB displays a warning when we
   # don't set the data-directory but try to use catch syscall
   # anyway.  For that, we must restart GDB first.
-  gdb_exit
-  gdb_start
-  gdb_reinitialize_dir $srcdir/$subdir
-  gdb_load ${binfile}
+  clean_restart $binfile
   test_catch_syscall_fail_nodatadir
 }
 
 # Restart gdb
-
-gdb_exit
-gdb_start
-gdb_reinitialize_dir $srcdir/$subdir
-gdb_load ${binfile}
+clean_restart $binfile
 
 # Execute the tests, without XML support.  In this case, GDB will
 # only display syscall numbers, and not syscall names.

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

* Re: [PATCH] Improve and fix catch-syscall.exp
  2013-12-16 23:09   ` Sergio Durigan Junior
@ 2013-12-17 10:24     ` Pedro Alves
  2013-12-17 17:35       ` Sergio Durigan Junior
  0 siblings, 1 reply; 12+ messages in thread
From: Pedro Alves @ 2013-12-17 10:24 UTC (permalink / raw)
  To: Sergio Durigan Junior; +Cc: Doug Evans, GDB Patches

On 12/16/2013 11:09 PM, Sergio Durigan Junior wrote:
> +    set close_syscall [get_integer_valueof "close_syscall" 0]
> +    set chroot_syscall [get_integer_valueof "chroot_syscall" 0]
> +    set all_syscalls_numbers [list $close_syscall $chroot_syscall]
> +    set last_syscall_number [get_integer_valueof "exit_group_syscall" 0]

Minor nit:

As we saw in the other patch, using 0 for invalid syscall
number isn't a good idea.  It shouldn't really matter much in
practice but for code clarity I suggest defaulting to -1 instead.

-- 
Pedro Alves

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

* Re: [PATCH] Improve and fix catch-syscall.exp
  2013-12-17 10:24     ` Pedro Alves
@ 2013-12-17 17:35       ` Sergio Durigan Junior
  2013-12-18 21:10         ` Doug Evans
  0 siblings, 1 reply; 12+ messages in thread
From: Sergio Durigan Junior @ 2013-12-17 17:35 UTC (permalink / raw)
  To: Pedro Alves; +Cc: Doug Evans, GDB Patches

On Tuesday, December 17 2013, Pedro Alves wrote:

> On 12/16/2013 11:09 PM, Sergio Durigan Junior wrote:
>> +    set close_syscall [get_integer_valueof "close_syscall" 0]
>> +    set chroot_syscall [get_integer_valueof "chroot_syscall" 0]
>> +    set all_syscalls_numbers [list $close_syscall $chroot_syscall]
>> +    set last_syscall_number [get_integer_valueof "exit_group_syscall" 0]
>
> Minor nit:
>
> As we saw in the other patch, using 0 for invalid syscall
> number isn't a good idea.  It shouldn't really matter much in
> practice but for code clarity I suggest defaulting to -1 instead.

Thanks, fixed.

I also modified catch-syscall.c to use SYS_bla instead of __NR_bla,
according to Doug's suggestion on IRC.

I believe Pedro already gave an approval.  Doug, are you OK with this
version?

-- 
Sergio

2013-12-17  Sergio Durigan Junior  <sergiodj@redhat.com>

	* gdb.base/catch-syscall.c: Include <sys/syscall.h>.
	(close_syscall, chroot_syscall, exit_group_syscall): New
	variables.
	* gdb.base/catch-syscall.exp: Replace gdb_compile by
	prepare_for_testing.  Call fill_all_syscalls_numbers before
	starting.  Replace gdb_exit, gdb_start, gdb_reinitialize_dir and
	gdb_load by clean_restart.
	(check_info_bp_any_syscall, check_info_bp_specific_syscall)
	(check_info_bp_many_syscalls): Remove global gdb_prompt.
	(check_call_to_syscall): Likewise.  Add global decimal.  Improve
	testing regex.
	(check_return_from_syscall): Likewise.
	(check_continue, insert_catch_syscall_with_arg): Remove global
	gdb_prompt.
	(insert_catch_syscall_with_many_args): Likewise.  Add global
	decimal.  Fix $filter_str.  Improve testing regex.
	(check_for_program_end): Remove global gdb_prompt.
	(test_catch_syscall_without_args): Likewise.  Add global decimal.
	Improve testing regex.
	(test_catch_syscall_with_args, test_catch_syscall_with_many_args)
	(test_catch_syscall_with_wrong_args)
	(test_catch_syscall_restarting_inferior)
	(test_catch_syscall_fail_nodatadir): Remove global gdb_prompt.
	(do_syscall_tests): Likewise.  Remove global srcdir.
	(test_catch_syscall_without_args_noxml): Remove global gdb_prompt.
	Add global last_syscall_number.  Test for the exact syscall number
	to be caught.
	(test_catch_syscall_with_args_noxml): Remove global gdb_prompt.
	Add global all_syscalls_numbers.  Test each syscall number to be
	caught, instead of only testing "close".
	(test_catch_syscall_with_wrong_args_noxml): Remove global gdb_prompt.
	(do_syscall_tests_without_xml): Likewise.  Remove global srcdir.
	Remove stale comment.
	(fill_all_syscalls_numbers): Add global last_syscall_number.  Fill
	the correct syscall numbers using information from the inferior.

diff --git a/gdb/testsuite/gdb.base/catch-syscall.c b/gdb/testsuite/gdb.base/catch-syscall.c
index 64850de..8f94191 100644
--- a/gdb/testsuite/gdb.base/catch-syscall.c
+++ b/gdb/testsuite/gdb.base/catch-syscall.c
@@ -8,9 +8,16 @@
    September, 2008 */
 
 #include <unistd.h>
+#include <sys/syscall.h>
 #include <fcntl.h>
 #include <sys/stat.h>
 
+/* These are the syscalls numbers used by the test.  */
+
+static int close_syscall = SYS_close;
+static int chroot_syscall = SYS_chroot;
+static int exit_group_syscall = SYS_exit_group;
+
 int
 main (void)
 {
diff --git a/gdb/testsuite/gdb.base/catch-syscall.exp b/gdb/testsuite/gdb.base/catch-syscall.exp
index 7f1bd29..087fbe2 100644
--- a/gdb/testsuite/gdb.base/catch-syscall.exp
+++ b/gdb/testsuite/gdb.base/catch-syscall.exp
@@ -24,7 +24,7 @@ if { [is_remote target] || ![isnative] } then {
 }
 
 # Until "catch syscall" is implemented on other targets...
-if {![istarget "hppa*-hp-hpux*"] && ![istarget "*-linux*"]} then {
+if { ![istarget "hppa*-hp-hpux*"] && ![istarget "*-linux*"] } {
     continue
 }
 
@@ -40,26 +40,26 @@ if { ![istarget "x86_64-*-linux*"] && ![istarget "i\[34567\]86-*-linux*"]
 
 standard_testfile
 
+if  { [prepare_for_testing ${testfile}.exp $testfile ${testfile}.c] } {
+     untested catch-syscall.exp
+     return -1
+}
+
 # All (but the last) syscalls from the example code
 # They are ordered according to the file, so do not change this.
 set all_syscalls { "close" "chroot" }
 set all_syscalls_numbers { }
+
 # The last syscall (exit()) does not return, so
 # we cannot expect the catchpoint to be triggered
 # twice.  It is a special case.
 set last_syscall "exit_group"
-
-if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug}] != "" } {
-     untested catch-syscall.exp
-     return -1
-}
+set last_syscall_number { }
 
 # Internal procedure used to check if, after issuing a 'catch syscall'
 # command (without arguments), the 'info breakpoints' command displays
 # that '"any syscall"' is to be caught.
 proc check_info_bp_any_syscall {} {
-    global gdb_prompt
-
     # Verifying that the catchpoint appears in the 'info breakpoints'
     # command, but with "<any syscall>".
     set thistest "catch syscall appears in 'info breakpoints'"
@@ -70,8 +70,6 @@ proc check_info_bp_any_syscall {} {
 # command (with arguments), the 'info breakpoints' command displays
 # that the syscall 'X' is to be caught.
 proc check_info_bp_specific_syscall { syscall } {
-    global gdb_prompt
-
     set thistest "syscall(s) $syscall appears in 'info breakpoints'"
     gdb_test "info breakpoints" ".*catchpoint.*keep y.*syscall(\[(\]s\[)\])? (.)?${syscall}(.)?.*" $thistest
 }
@@ -80,7 +78,6 @@ proc check_info_bp_specific_syscall { syscall } {
 # command (with many arguments), the 'info breakpoints' command displays
 # that the syscalls 'X' are to be caught.
 proc check_info_bp_many_syscalls { syscalls } {
-    global gdb_prompt
     set filter_str ""
 
     foreach name $syscalls {
@@ -95,25 +92,23 @@ proc check_info_bp_many_syscalls { syscalls } {
 
 # This procedure checks if there was a call to a syscall.
 proc check_call_to_syscall { syscall } {
-    global gdb_prompt
+    global decimal
 
     set thistest "program has called $syscall"
-    gdb_test "continue" "Catchpoint .*(call to syscall .?${syscall}.?).*" $thistest
+    gdb_test "continue" "Catchpoint $decimal \\(call to syscall .?${syscall}.?\\).*" $thistest
 }
 
 # This procedure checks if the syscall returned.
 proc check_return_from_syscall { syscall } {
-    global gdb_prompt
+    global decimal
 
     set thistest "syscall $syscall has returned"
-    gdb_test "continue" "Catchpoint .*(returned from syscall (.)?${syscall}(.)?).*" $thistest
+    gdb_test "continue" "Catchpoint $decimal \\(returned from syscall ${syscall}\\).*" $thistest
 }
 
 # Internal procedure that performs two 'continue' commands and checks if
 # a syscall call AND return occur.
 proc check_continue { syscall } {
-    global gdb_prompt
-
     # Testing if the 'continue' stops at the
     # specified syscall_name.  If it does, then it should
     # first print that the infeior has called the syscall,
@@ -127,50 +122,48 @@ proc check_continue { syscall } {
 
 # Inserts a syscall catchpoint with an argument.
 proc insert_catch_syscall_with_arg { syscall } {
-    global gdb_prompt
+    global decimal
 
     # Trying to set the catchpoint
     set thistest "catch syscall with arguments ($syscall)"
-    gdb_test "catch syscall $syscall" "Catchpoint .*(syscall (.)?${syscall}(.)?( \[\[0-9\]+\])?).*" $thistest
+    gdb_test "catch syscall $syscall" "Catchpoint $decimal \\(syscall \'?${syscall}\'?( \[${decimal}\])?\\)" $thistest
 
     check_info_bp_specific_syscall $syscall
 }
 
 # Inserts a syscall catchpoint with many arguments.
 proc insert_catch_syscall_with_many_args { syscalls numbers } {
-    global gdb_prompt
+    global decimal
+
     set catch [ join $syscalls " " ]
     set filter_str ""
 
     foreach name $syscalls number $numbers {
-      set filter_str "${filter_str}'${name}' \[${number}\] "
+      set filter_str "${filter_str}'${name}' \\\[${number}\\\] "
     }
 
     set filter_str [ string trimright $filter_str " " ]
 
     # Trying to set the catchpoint
     set thistest "catch syscall with arguments ($filter_str)"
-    gdb_test "catch syscall $catch" "Catchpoint .*(syscalls (.)?${filter_str}(.)?).*" $thistest
+    gdb_test "catch syscall $catch" "Catchpoint $decimal \\(syscalls ${filter_str}\\).*" $thistest
 
     check_info_bp_many_syscalls $syscalls
 }
 
 proc check_for_program_end {} {
-    global gdb_prompt
-
     # Deleting the catchpoints
     delete_breakpoints
 
     gdb_continue_to_end
-
 }
 
 proc test_catch_syscall_without_args {} {
-    global gdb_prompt all_syscalls last_syscall
+    global all_syscalls last_syscall decimal
 
     with_test_prefix "without arguments" {
 	# Trying to set the syscall.
-	gdb_test "catch syscall" "Catchpoint .*(syscall).*"
+	gdb_test "catch syscall" "Catchpoint $decimal \\(any syscall\\)"
 
 	check_info_bp_any_syscall
 
@@ -190,8 +183,6 @@ proc test_catch_syscall_without_args {} {
 
 proc test_catch_syscall_with_args {} {
     with_test_prefix "with arguments" {
-	global gdb_prompt
-
 	set syscall_name "close"
 	insert_catch_syscall_with_arg $syscall_name
 
@@ -205,7 +196,7 @@ proc test_catch_syscall_with_args {} {
 
 proc test_catch_syscall_with_many_args {} {
     with_test_prefix "with many arguments" {
-	global gdb_prompt all_syscalls all_syscalls_numbers
+	global all_syscalls all_syscalls_numbers
 
 	insert_catch_syscall_with_many_args $all_syscalls $all_syscalls_numbers
 
@@ -221,8 +212,6 @@ proc test_catch_syscall_with_many_args {} {
 
 proc test_catch_syscall_with_wrong_args {} {
     with_test_prefix "wrong args" {
-	global gdb_prompt
-
 	# mlock is not called from the source
 	set syscall_name "mlock"
 	insert_catch_syscall_with_arg $syscall_name
@@ -237,8 +226,6 @@ proc test_catch_syscall_with_wrong_args {} {
 
 proc test_catch_syscall_restarting_inferior {} {
     with_test_prefix "restarting inferior" {
-	global gdb_prompt
-
 	set syscall_name "chroot"
 
 	with_test_prefix "entry" {
@@ -263,8 +250,6 @@ proc test_catch_syscall_restarting_inferior {} {
 
 proc test_catch_syscall_fail_nodatadir {} {
     with_test_prefix "fail no datadir" {
-	global gdb_prompt
-
 	# Sanitizing.
 	delete_breakpoints
 
@@ -289,8 +274,6 @@ proc test_catch_syscall_fail_nodatadir {} {
 }
 
 proc do_syscall_tests {} {
-    global gdb_prompt srcdir
-
     # NOTE: We don't have to point gdb at the correct data-directory.
     # For the build tree that is handled by INTERNAL_GDBFLAGS.
 
@@ -332,7 +315,7 @@ proc test_catch_syscall_without_args_noxml {} {
     with_test_prefix "without args noxml" {
 	# We will need the syscall names even not using it because we
 	# need to know know many syscalls are in the example file.
-	global gdb_prompt all_syscalls last_syscall
+	global all_syscalls last_syscall_number all_syscalls_numbers
 
 	delete_breakpoints
 
@@ -340,19 +323,15 @@ proc test_catch_syscall_without_args_noxml {} {
 
 	# Now, we should be able to set a catchpoint, and GDB shall
 	# not display the warning anymore.
-	foreach name $all_syscalls {
-	    # Unfortunately, we don't know the syscall number that
-	    # will be caught because this information is
-	    # arch-dependent.  Thus, we try to catch anything similar
-	    # to a number.
+	foreach name $all_syscalls number $all_syscalls_numbers {
 	    with_test_prefix "$name" {
-		check_continue "\[0-9\]*"
+		check_continue $number
 	    }
 	}
 
 	# At last but not least, we check if the inferior has called
 	# the last (exit) syscall.
-	check_call_to_syscall "\[0-9\]*"
+	check_call_to_syscall $last_syscall_number
 
 	delete_breakpoints
     }
@@ -360,25 +339,19 @@ proc test_catch_syscall_without_args_noxml {} {
 
 proc test_catch_syscall_with_args_noxml {} {
     with_test_prefix "with args noxml" {
-	global gdb_prompt
-
-	# The number of the "close" syscall.  This is our option for a
-	# "long-estabilished" syscall in all Linux architectures, but
-	# unfortunately x86_64 and a few other platforms don't "follow
-	# the convention".  Because of this, we need this ugly check
-	# :-(.
-	set close_number ""
-	if { [istarget "x86_64-*-linux*"] } {
-	    set close_number "3"
-	} else {
-	    set close_number "6"
-	}
+	global all_syscalls_numbers
 
 	delete_breakpoints
 
-	insert_catch_syscall_with_arg $close_number
+	# Inserting all syscalls numbers to be caught
+	foreach syscall_number $all_syscalls_numbers {
+	    insert_catch_syscall_with_arg $syscall_number
+	}
 
-	check_continue $close_number
+	# Checking that all syscalls are caught.
+	foreach syscall_number $all_syscalls_numbers {
+	    check_continue $syscall_number
+	}
 
 	delete_breakpoints
     }
@@ -386,8 +359,6 @@ proc test_catch_syscall_with_args_noxml {} {
 
 proc test_catch_syscall_with_wrong_args_noxml {} {
     with_test_prefix "with wrong args noxml" {
-	global gdb_prompt
-
 	delete_breakpoints
 
 	# Even without XML support, GDB should not accept unknown
@@ -400,8 +371,6 @@ proc test_catch_syscall_with_wrong_args_noxml {} {
 }
 
 proc do_syscall_tests_without_xml {} {
-    global gdb_prompt srcdir
-
     # Make sure GDB doesn't load the syscalls xml from the system data
     # directory.
     gdb_test_no_output "set data-directory /the/path/to/nowhere"
@@ -413,12 +382,6 @@ proc do_syscall_tests_without_xml {} {
     # The only valid argument "catch syscall" should accept is the
     # syscall number, and not the name (since it can't translate a
     # name to a number).
-    #
-    # It's worth mentioning that we only try to catch the syscall
-    # close().  This is because the syscall number is an arch-dependent
-    # information, so we can't assume that we know every syscall number
-    # in this system.  Therefore, we have decided to use a "long-estabilished"
-    # system call, and close() just sounded the right choice :-).
     if [runto_main] then { test_catch_syscall_with_args_noxml }
 
     # Now, we'll try to provide a syscall name (valid or not) to the command,
@@ -428,47 +391,35 @@ proc do_syscall_tests_without_xml {} {
 
 # This procedure fills the vector "all_syscalls_numbers" with the proper
 # numbers for the used syscalls according to the architecture.
+#
+# These numbers were taken from the respective <asm/unistd.h> files
+# from each architecture.
 proc fill_all_syscalls_numbers {} {
-    global all_syscalls_numbers
-
-    # For Linux on x86, PPC, PPC64, SPARC, SPARC64 and ARM,
-    # the numbers for the syscalls "close" and "chroot" are the same.
-    if { [istarget "i\[34567\]86-*-linux*"]
-         || [istarget "powerpc-*-linux*"] || [istarget "powerpc64-*-linux*"]
-         || [istarget "sparc-*-linux*"] || [istarget "sparc64-*-linux*"]
-         || [istarget "arm*-linux*"] } {
-         set all_syscalls_numbers { "6" "61" }
-    }
-}
+    global all_syscalls_numbers last_syscall_number
 
-# Start with a fresh gdb
+    set close_syscall [get_integer_valueof "close_syscall" -1]
+    set chroot_syscall [get_integer_valueof "chroot_syscall" -1]
+    set all_syscalls_numbers [list $close_syscall $chroot_syscall]
+    set last_syscall_number [get_integer_valueof "exit_group_syscall" -1]
+}
 
-gdb_exit
-set do_xml_test ![gdb_skip_xml_test]
-gdb_start
-gdb_reinitialize_dir $srcdir/$subdir
-gdb_load ${binfile}
+# Filling all the syscalls numbers before starting anything.
+fill_all_syscalls_numbers
 
 # Execute the tests, using XML support
-if $do_xml_test {
+if { ![gdb_skip_xml_test] } {
+  clean_restart $binfile
   do_syscall_tests
 
   # Now, we have to see if GDB displays a warning when we
   # don't set the data-directory but try to use catch syscall
   # anyway.  For that, we must restart GDB first.
-  gdb_exit
-  gdb_start
-  gdb_reinitialize_dir $srcdir/$subdir
-  gdb_load ${binfile}
+  clean_restart $binfile
   test_catch_syscall_fail_nodatadir
 }
 
 # Restart gdb
-
-gdb_exit
-gdb_start
-gdb_reinitialize_dir $srcdir/$subdir
-gdb_load ${binfile}
+clean_restart $binfile
 
 # Execute the tests, without XML support.  In this case, GDB will
 # only display syscall numbers, and not syscall names.

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

* Re: [PATCH] Improve and fix catch-syscall.exp
  2013-12-17 17:35       ` Sergio Durigan Junior
@ 2013-12-18 21:10         ` Doug Evans
  2013-12-18 22:26           ` Sergio Durigan Junior
  0 siblings, 1 reply; 12+ messages in thread
From: Doug Evans @ 2013-12-18 21:10 UTC (permalink / raw)
  To: Sergio Durigan Junior; +Cc: Pedro Alves, GDB Patches

On Tue, Dec 17, 2013 at 9:35 AM, Sergio Durigan Junior
<sergiodj@redhat.com> wrote:
> On Tuesday, December 17 2013, Pedro Alves wrote:
>
>> On 12/16/2013 11:09 PM, Sergio Durigan Junior wrote:
>>> +    set close_syscall [get_integer_valueof "close_syscall" 0]
>>> +    set chroot_syscall [get_integer_valueof "chroot_syscall" 0]
>>> +    set all_syscalls_numbers [list $close_syscall $chroot_syscall]
>>> +    set last_syscall_number [get_integer_valueof "exit_group_syscall" 0]
>>
>> Minor nit:
>>
>> As we saw in the other patch, using 0 for invalid syscall
>> number isn't a good idea.  It shouldn't really matter much in
>> practice but for code clarity I suggest defaulting to -1 instead.
>
> Thanks, fixed.
>
> I also modified catch-syscall.c to use SYS_bla instead of __NR_bla,
> according to Doug's suggestion on IRC.
>
> I believe Pedro already gave an approval.  Doug, are you OK with this
> version?
>
> --
> Sergio
>
> 2013-12-17  Sergio Durigan Junior  <sergiodj@redhat.com>
>
>         * gdb.base/catch-syscall.c: Include <sys/syscall.h>.
>         (close_syscall, chroot_syscall, exit_group_syscall): New
>         variables.
>         * gdb.base/catch-syscall.exp: Replace gdb_compile by
>         prepare_for_testing.  Call fill_all_syscalls_numbers before
>         starting.  Replace gdb_exit, gdb_start, gdb_reinitialize_dir and
>         gdb_load by clean_restart.
>         (check_info_bp_any_syscall, check_info_bp_specific_syscall)
>         (check_info_bp_many_syscalls): Remove global gdb_prompt.
>         (check_call_to_syscall): Likewise.  Add global decimal.  Improve
>         testing regex.
>         (check_return_from_syscall): Likewise.
>         (check_continue, insert_catch_syscall_with_arg): Remove global
>         gdb_prompt.
>         (insert_catch_syscall_with_many_args): Likewise.  Add global
>         decimal.  Fix $filter_str.  Improve testing regex.
>         (check_for_program_end): Remove global gdb_prompt.
>         (test_catch_syscall_without_args): Likewise.  Add global decimal.
>         Improve testing regex.
>         (test_catch_syscall_with_args, test_catch_syscall_with_many_args)
>         (test_catch_syscall_with_wrong_args)
>         (test_catch_syscall_restarting_inferior)
>         (test_catch_syscall_fail_nodatadir): Remove global gdb_prompt.
>         (do_syscall_tests): Likewise.  Remove global srcdir.
>         (test_catch_syscall_without_args_noxml): Remove global gdb_prompt.
>         Add global last_syscall_number.  Test for the exact syscall number
>         to be caught.
>         (test_catch_syscall_with_args_noxml): Remove global gdb_prompt.
>         Add global all_syscalls_numbers.  Test each syscall number to be
>         caught, instead of only testing "close".
>         (test_catch_syscall_with_wrong_args_noxml): Remove global gdb_prompt.
>         (do_syscall_tests_without_xml): Likewise.  Remove global srcdir.
>         Remove stale comment.
>         (fill_all_syscalls_numbers): Add global last_syscall_number.  Fill
>         the correct syscall numbers using information from the inferior.

Two nits.  Ok with the changes below.

> @@ -428,47 +391,35 @@ proc do_syscall_tests_without_xml {} {
>
>  # This procedure fills the vector "all_syscalls_numbers" with the proper
>  # numbers for the used syscalls according to the architecture.
> +#
> +# These numbers were taken from the respective <asm/unistd.h> files
> +# from each architecture.

This comment can go (no longer needed).

>
> -gdb_exit
> -set do_xml_test ![gdb_skip_xml_test]
> -gdb_start
> -gdb_reinitialize_dir $srcdir/$subdir
> -gdb_load ${binfile}
> +# Filling all the syscalls numbers before starting anything.

s/Filling/Fill/

> +fill_all_syscalls_numbers

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

* Re: [PATCH] Improve and fix catch-syscall.exp
  2013-12-18 21:10         ` Doug Evans
@ 2013-12-18 22:26           ` Sergio Durigan Junior
  0 siblings, 0 replies; 12+ messages in thread
From: Sergio Durigan Junior @ 2013-12-18 22:26 UTC (permalink / raw)
  To: Doug Evans; +Cc: Pedro Alves, GDB Patches

On Wednesday, December 18 2013, Doug Evans wrote:

> Two nits.  Ok with the changes below.
>
>> @@ -428,47 +391,35 @@ proc do_syscall_tests_without_xml {} {
>>
>>  # This procedure fills the vector "all_syscalls_numbers" with the proper
>>  # numbers for the used syscalls according to the architecture.
>> +#
>> +# These numbers were taken from the respective <asm/unistd.h> files
>> +# from each architecture.
>
> This comment can go (no longer needed).
>
>>
>> -gdb_exit
>> -set do_xml_test ![gdb_skip_xml_test]
>> -gdb_start
>> -gdb_reinitialize_dir $srcdir/$subdir
>> -gdb_load ${binfile}
>> +# Filling all the syscalls numbers before starting anything.
>
> s/Filling/Fill/

Fixed all nits.  Thank you and Pedro for the review.  Checked-in:

<https://sourceware.org/ml/gdb-cvs/2013-12/msg00097.html>

-- 
Sergio

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

end of thread, other threads:[~2013-12-18 22:26 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-12-13 23:06 [PATCH] Improve and fix catch-syscall.exp Sergio Durigan Junior
2013-12-15 18:31 ` Doug Evans
2013-12-15 18:44   ` Doug Evans
2013-12-16  4:24     ` Sergio Durigan Junior
2013-12-16  4:20   ` Sergio Durigan Junior
2013-12-16 23:09   ` Sergio Durigan Junior
2013-12-17 10:24     ` Pedro Alves
2013-12-17 17:35       ` Sergio Durigan Junior
2013-12-18 21:10         ` Doug Evans
2013-12-18 22:26           ` Sergio Durigan Junior
2013-12-16 17:50 ` Pedro Alves
2013-12-16 17:59   ` Sergio Durigan Junior

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).