[PATCH] Fix kfail in gdb.base/callfuncs.exp

[PATCH] Fix kfail in gdb.base/callfuncs.exp

[Yao Qi]

Hi,
I find some tests in gdb.base/callfuncs.exp are not KFAIL'ed properly
in some configurations.  For native gdb configured as
'x86_64-unknown-linux-gnu',

$make check RUNTESTFLAGS='CC_FOR_TARGET=i686-pc-linux-gnu-gcc --target_board=unix/-m32 callfuncs.exp'

  KPASS: gdb.base/callfuncs.exp: p t_float_complex_values(fc1, fc2) (PRMS gdb/12798)
  KPASS: gdb.base/callfuncs.exp: p t_float_complex_many_args(fc1, fc2, fc3, fc4, fc1, fc2, fc3, fc4, fc1, fc2, fc3, fc4, fc1, fc2, fc3, fc4) (PRMS gdb/12800)
  KPASS: gdb.base/callfuncs.exp: p t_double_complex_values(dc1, dc2) (PRMS gdb/12798)
  KPASS: gdb.base/callfuncs.exp: p t_double_complex_many_args(dc1, dc2,dc3, dc4, dc1, dc2, dc3, dc4, dc1, dc2, dc3, dc4, dc1, dc2, dc3, dc4) (PRMS gdb/12800)
  KPASS: gdb.base/callfuncs.exp: call inferior func with struct - returns float _Complex (PRMS gdb/12796)

For native gdb configured as 'i686-pc-linux-gnu'

$ make check RUNTESTFLAGS='--target_board=native-gdbserver/-m64 callfuncs.exp'

  FAIL: gdb.base/callfuncs.exp: p t_float_complex_values(fc1, fc2)
  FAIL: gdb.base/callfuncs.exp: p t_float_complex_many_args(fc1, fc2, fc3, fc4, fc1, fc2, fc3, fc4, fc1, fc2, fc3, fc4, fc1, fc2, fc3, fc4)
  FAIL: gdb.base/callfuncs.exp: p t_double_complex_values(dc1, dc2)
  FAIL: gdb.base/callfuncs.exp: p t_double_complex_many_args(dc1, dc2, dc3, dc4, dc1, dc2, dc3, dc4, dc1, dc2, dc3, dc4, dc1, dc2, dc3, dc4)
  FAIL: gdb.base/callfuncs.exp: call inferior func with struct - returns float _Complex

The tests are known to fail only on 64-bit, so this patch below fixes
these KPASS or FAIL.  OK to apply?

gdb/testsuite:

2012-10-15  Yao Qi  <yao@codesourcery.com>

	* gdb.base/callfuncs.exp (do_function_calls): KFAIL some test
	for x86 with -m64.
---
 gdb/testsuite/gdb.base/callfuncs.exp |   30 ++++++++++++++++++++++++------
 1 files changed, 24 insertions(+), 6 deletions(-)

diff --git a/gdb/testsuite/gdb.base/callfuncs.exp b/gdb/testsuite/gdb.base/callfuncs.exp
index f56761e..fe9dd1f 100644
--- a/gdb/testsuite/gdb.base/callfuncs.exp
+++ b/gdb/testsuite/gdb.base/callfuncs.exp
@@ -149,19 +149,34 @@ proc do_function_calls {} {
     }
 
     if [support_complex_tests] {
-	setup_kfail_for_target gdb/12798 "x86_64-*-*"
+	# x86-64 or x86 with -m64.
+	if { ([istarget  "x86_64-*-*"] || [istarget "i\[34567\]86-*-*"])
+	     && [is_lp64_target] } {
+	    setup_kfail gdb/12798 "*-*-*"
+	}
 	gdb_test "p t_float_complex_values(fc1, fc2)" " = 1"
 	gdb_test "p t_float_complex_values(fc3, fc4)" " = 0"
 
-	setup_kfail_for_target gdb/12800 "x86_64-*-*"
+	# x86-64 or x86 with -m64.
+	if { ([istarget  "x86_64-*-*"] || [istarget "i\[34567\]86-*-*"])
+	     && [is_lp64_target] } {
+	    setup_kfail gdb/12800 "*-*-*"
+	}
 	gdb_test "p t_float_complex_many_args(fc1, fc2, fc3, fc4, fc1, fc2, fc3, fc4, fc1, fc2, fc3, fc4, fc1, fc2, fc3, fc4)" " = 1"
 	gdb_test "p t_float_complex_many_args(fc1, fc1, fc1, fc1, fc1, fc1, fc1, fc1, fc1, fc1, fc1, fc1, fc1, fc1, fc1, fc1)" " = 0"
 
-	setup_kfail_for_target gdb/12798 "x86_64-*-*"
+	# x86-64 or x86 with -m64.
+	if { ([istarget  "x86_64-*-*"] || [istarget "i\[34567\]86-*-*"])
+	     && [is_lp64_target] } {
+	    setup_kfail gdb/12798 "*-*-*"
+	}
 	gdb_test "p t_double_complex_values(dc1, dc2)" " = 1"
 	gdb_test "p t_double_complex_values(dc3, dc4)" " = 0"
 
-	setup_kfail_for_target gdb/12800 "x86_64-*-*"
+	if { ([istarget  "x86_64-*-*"] || [istarget "i\[34567\]86-*-*"])
+	     && [is_lp64_target] } {
+	    setup_kfail gdb/12800 "*-*-*"
+	}
 	gdb_test "p t_double_complex_many_args(dc1, dc2, dc3, dc4, dc1, dc2, dc3, dc4, dc1, dc2, dc3, dc4, dc1, dc2, dc3, dc4)" " = 1"
 	gdb_test "p t_double_complex_many_args(dc1, dc1, dc1, dc1, dc1, dc1, dc1, dc1, dc1, dc1, dc1, dc1, dc1, dc1, dc1, dc1)" " = 0"
 
@@ -245,8 +260,11 @@ proc do_function_calls {} {
     }
 
     if [support_complex_tests] {
-
-	setup_kfail_for_target gdb/12796 "x86_64-*-*"
+	# x86-64 or x86 with -m64.
+	if { ([istarget  "x86_64-*-*"] || [istarget "i\[34567\]86-*-*"])
+	     && [is_lp64_target] } {
+	    setup_kfail gdb/12796 "*-*-*"
+	}
 	gdb_test "p t_structs_fc(struct_val1)" ".*= 3 \\+ 3 \\* I" \
 	    "call inferior func with struct - returns float _Complex"
 
-- 
1.7.7.6

ping: [PATCH] Fix kfail in gdb.base/callfuncs.exp

[Yao Qi]

On 10/15/2012 07:46 PM, Yao Qi wrote:
> gdb/testsuite:
>
> 2012-10-15  Yao Qi<yao@codesourcery.com>
>
> 	* gdb.base/callfuncs.exp (do_function_calls): KFAIL some test
> 	for x86 with -m64.

Ping.  http://sourceware.org/ml/gdb-patches/2012-10/msg00212.html

-- 
Yao

Re: ping: [PATCH] Fix kfail in gdb.base/callfuncs.exp

[Mark Kettenis]

> Date: Tue, 23 Oct 2012 14:59:20 +0800
> From: Yao Qi <yao@codesourcery.com>
> 
> On 10/15/2012 07:46 PM, Yao Qi wrote:
> > gdb/testsuite:
> >
> > 2012-10-15  Yao Qi<yao@codesourcery.com>
> >
> > 	* gdb.base/callfuncs.exp (do_function_calls): KFAIL some test
> > 	for x86 with -m64.
> 
> Ping.  http://sourceware.org/ml/gdb-patches/2012-10/msg00212.html

I don't think decoupling the triplet from the setup_kfail this way is
a good idea.

Re: ping: [PATCH] Fix kfail in gdb.base/callfuncs.exp

[Yao Qi]

On 10/23/2012 04:36 PM, Mark Kettenis wrote:
> I don't think decoupling the triplet from the setup_kfail this way is
> a good idea.

In general, yes, but for the target supports bi-arch, we can't 
differentiate each arch in the same target triplet, unless we teach 
setup_kfail to understand each arch in the same target triplet.  WDYT?

-- 
Yao

Re: ping: [PATCH] Fix kfail in gdb.base/callfuncs.exp

[Mark Kettenis]

> Date: Tue, 23 Oct 2012 18:06:17 +0800
> From: Yao Qi <yao@codesourcery.com>
> 
> On 10/23/2012 04:36 PM, Mark Kettenis wrote:
> > I don't think decoupling the triplet from the setup_kfail this way is
> > a good idea.
> 
> In general, yes, but for the target supports bi-arch, we can't 
> differentiate each arch in the same target triplet, unless we teach 
> setup_kfail to understand each arch in the same target triplet.  WDYT?

To be frank I think there something really wrong the way two different
target triplets now can actually designate the same three ABIs.

How about fixing the bug instead? ;

  <http://sourceware.org/ml/gdb-patches/2012-10/msg00424.html>

There you are!  I'll remove the setup_kfails in a follow-up diff.

Re: [PATCH] Fix kfail in gdb.base/callfuncs.exp

[Tom Tromey]

>>>>> "Yao" == Yao Qi <yao@codesourcery.com> writes:

No comment on the patch, just a question

Yao> -	setup_kfail_for_target gdb/12798 "x86_64-*-*"

I didn't know this proc, so I looked it up:

proc setup_kfail_for_target { PR target } {
    if { [istarget $target] } {
	setup_kfail $PR $target
    }
}

But setup_kfail itself just does:

proc setup_kfail { args } {
[...]
	if {[istarget $sub_arg]} {
	    set kfail_flag 1
	    continue
	}

So I don't understand how setup_kfail_for_target is different.
If I'm missing something, I'd like to know what.
Otherwise I think we can nuke this proc entirely.

Tom

Re: [PATCH] Fix kfail in gdb.base/callfuncs.exp

[Yao Qi]

On 10/24/2012 02:41 AM, Tom Tromey wrote:
>>>>>> "Yao" == Yao Qi <yao@codesourcery.com> writes:
>
> No comment on the patch, just a question
>
> Yao> -	setup_kfail_for_target gdb/12798 "x86_64-*-*"
>
> I didn't know this proc, so I looked it up:
>
> proc setup_kfail_for_target { PR target } {
>      if { [istarget $target] } {
> 	setup_kfail $PR $target
>      }
> }
>
> But setup_kfail itself just does:
>
> proc setup_kfail { args } {
> [...]
> 	if {[istarget $sub_arg]} {
> 	    set kfail_flag 1
> 	    continue
> 	}
>
> So I don't understand how setup_kfail_for_target is different.
> If I'm missing something, I'd like to know what.
> Otherwise I think we can nuke this proc entirely.
>

Tom,
This proc is to link the same fail to different PRs according to
  different target triplets (See more here
http://sourceware.org/ml/gdb-patches/2011-05/msg00508.html).  For
  example, in gdb.base/varargs.exp

     setup_kfail_for_target gdb/12790 "x86_64-*-*"
     setup_kfail_for_target gdb/12791 "arm*-*-*"
     gdb_test $test ".*= 4 \\+ 4 \\* I" $test

'setup_kfail_for_target' can be replaced with 'setup_kfail' if there is
  only one PR number for a given fail, but it is still useful in
gdb.base/varargs.exp.

The patch below is to replace 'setup_kfail_for_target' with
'setup_kfail', and add one sentence of comment to proc
'setup_kfail_for_target'.  Is it OK?

-- 
Yao

gdb/testsuite:

2012-10-24  Yao Qi  <yao@codesourcery.com>

	* gdb.base/callfuncs.exp (do_function_calls): User 'setup_kfail'
	instead of 'setup_kfail_for_target'.
	* gdb.trace/ftrace.exp (test_fast_tracepoints): Likewise.
	* lib/gdb.exp (setup_kfail_for_target): Add comment.
---
  gdb/testsuite/gdb.base/callfuncs.exp |    4 ++--
  gdb/testsuite/gdb.trace/ftrace.exp   |    4 ++--
  gdb/testsuite/lib/gdb.exp            |    3 ++-
  3 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/gdb/testsuite/gdb.base/callfuncs.exp 
b/gdb/testsuite/gdb.base/callfuncs.exp
index 75ea5f0..53ffafd 100644
--- a/gdb/testsuite/gdb.base/callfuncs.exp
+++ b/gdb/testsuite/gdb.base/callfuncs.exp
@@ -245,11 +245,11 @@ proc do_function_calls {} {
  	gdb_test "p t_structs_fc(struct_val1)" ".*= 3 \\+ 3 \\* I" \
  	    "call inferior func with struct - returns float _Complex"

-	setup_kfail_for_target gdb/12783 "i?86-*-*"
+	setup_kfail gdb/12783 "i?86-*-*"
  	gdb_test "p t_structs_dc(struct_val1)" ".*= 4 \\+ 4 \\* I" \
  	    "call inferior func with struct - returns double _Complex"

-	setup_kfail_for_target gdb/12783 "i?86-*-*"
+	setup_kfail gdb/12783 "i?86-*-*"
  	gdb_test "p t_structs_ldc(struct_val1)" "= 5 \\+ 5 \\* I" \
  	    "call inferior func with struct - returns long double _Complex"
      }
diff --git a/gdb/testsuite/gdb.trace/ftrace.exp 
b/gdb/testsuite/gdb.trace/ftrace.exp
index 56cd2fa..2a3dbae 100644
--- a/gdb/testsuite/gdb.trace/ftrace.exp
+++ b/gdb/testsuite/gdb.trace/ftrace.exp
@@ -132,13 +132,13 @@ proc test_fast_tracepoints {} {
  	gdb_test "tfind pc *set_point" "Found trace frame .*" \
  	    "tfind set_point frame, first time"

-	setup_kfail_for_target "gdb/13808" "x86_64-*-linux*"
+	setup_kfail "gdb/13808" "x86_64-*-linux*"
  	gdb_test "print globvar" " = 1"

  	gdb_test "tfind pc *set_point" "Found trace frame .*" \
  	    "tfind set_point frame, second time"

-	setup_kfail_for_target "gdb/13808" "x86_64-*-linux*"
+	setup_kfail "gdb/13808" "x86_64-*-linux*"
  	gdb_test "print anarg" " = 200"

  	gdb_test "tfind start" "Found trace frame .*" \
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index f27d4a7..29bf8f7 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -3435,7 +3435,8 @@ proc setup_xfail_format { format } {
  }

  # Like setup_kfail, but only call setup_kfail conditionally if
-# istarget[TARGET] returns true.
+# istarget[TARGET] returns true.  This proc is useful to link
+# the same fail to different PRs on different triplets.
  proc setup_kfail_for_target { PR target } {
      if { [istarget $target] } {
  	setup_kfail $PR $target
-- 
1.7.7.6

Re: ping: [PATCH] Fix kfail in gdb.base/callfuncs.exp

[Yao Qi]

On 10/24/2012 02:22 AM, Mark Kettenis wrote:
> How about fixing the bug instead? ;
>
>    <http://sourceware.org/ml/gdb-patches/2012-10/msg00424.html>
>
> There you are!  I'll remove the setup_kfails in a follow-up diff.

Mark, it looks good to me.  Thanks for giving a quick fix.

-- 
Yao

Re: [PATCH] Fix kfail in gdb.base/callfuncs.exp

[Tom Tromey]

>>>>> "Yao" == Yao Qi <yao@codesourcery.com> writes:

Yao> This proc is to link the same fail to different PRs according to
Yao>  different target triplets (See more here
Yao> http://sourceware.org/ml/gdb-patches/2011-05/msg00508.html).  For
Yao>  example, in gdb.base/varargs.exp

Ok, I see.  Thanks.

Yao> The patch below is to replace 'setup_kfail_for_target' with
Yao> 'setup_kfail', and add one sentence of comment to proc
Yao> 'setup_kfail_for_target'.  Is it OK?

Looks good to me, thanks.

Tom

Re: [PATCH] Fix kfail in gdb.base/callfuncs.exp

[Yao Qi]

On 10/24/2012 10:56 PM, Tom Tromey wrote:
> Yao> The patch below is to replace 'setup_kfail_for_target' with
> Yao> 'setup_kfail', and add one sentence of comment to proc
> Yao> 'setup_kfail_for_target'.  Is it OK?
> 
> Looks good to me, thanks.

When I prepare to commit this patch today, I find that Mark
committed this patch, which makes setup_kfail_for_target useless,
because we don't have to link the same fail to different PRs.

  Re: [PATCH] PR gdb/12776 is now fixed as well; adjust testsuite
  http://sourceware.org/ml/gdb-patches/2012-10/msg00491.html

As we discussed above, I remove proc 'setup_kfail_for_target'
in this patch together.  Committed as the delta part is obvious.

-- 
Yao

2012-10-25  Yao Qi  <yao@codesourcery.com>

	* gdb.trace/ftrace.exp (test_fast_tracepoints): Use
	'setup_kfail' instead of 'setup_kfail_for_target'.
	* gdb.base/varargs.exp: Likewise.
	* lib/gdb.exp (setup_kfail_for_target): Remove.

Index: testsuite/gdb.base/varargs.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.base/varargs.exp,v
retrieving revision 1.21
diff -u -r1.21 varargs.exp
--- testsuite/gdb.base/varargs.exp	25 Oct 2012 08:59:07 -0000	1.21
+++ testsuite/gdb.base/varargs.exp	25 Oct 2012 13:00:30 -0000
@@ -116,15 +116,15 @@
     global gdb_prompt
 
     set test "print find_max_float_real(4, fc1, fc2, fc3, fc4)"
-    setup_kfail_for_target gdb/12791 "arm*-*-*"
+    setup_kfail gdb/12791 "arm*-*-*"
     gdb_test $test ".*= 4 \\+ 4 \\* I" $test
 
     set test "print find_max_double_real(4, dc1, dc2, dc3, dc4)"
-    setup_kfail_for_target gdb/12791 "arm*-*-*"
+    setup_kfail gdb/12791 "arm*-*-*"
     gdb_test $test ".*= 4 \\+ 4 \\* I" $test
 
     set test "print find_max_long_double_real(4, ldc1, ldc2, ldc3, ldc4)"
-    setup_kfail_for_target gdb/12791 "arm*-*-*"
+    setup_kfail gdb/12791 "arm*-*-*"
     gdb_test $test ".*= 4 \\+ 4 \\* I" $test
 
 }
Index: testsuite/gdb.trace/ftrace.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.trace/ftrace.exp,v
retrieving revision 1.6
diff -u -r1.6 ftrace.exp
--- testsuite/gdb.trace/ftrace.exp	26 Jun 2012 18:25:19 -0000	1.6
+++ testsuite/gdb.trace/ftrace.exp	25 Oct 2012 13:00:49 -0000
@@ -132,13 +132,13 @@
 	gdb_test "tfind pc *set_point" "Found trace frame .*" \
 	    "tfind set_point frame, first time"
 
-	setup_kfail_for_target "gdb/13808" "x86_64-*-linux*"
+	setup_kfail "gdb/13808" "x86_64-*-linux*"
 	gdb_test "print globvar" " = 1"
 
 	gdb_test "tfind pc *set_point" "Found trace frame .*" \
 	    "tfind set_point frame, second time"
 
-	setup_kfail_for_target "gdb/13808" "x86_64-*-linux*"
+	setup_kfail "gdb/13808" "x86_64-*-linux*"
 	gdb_test "print anarg" " = 200"
 
 	gdb_test "tfind start" "Found trace frame .*" \
cvs diff: Diffing testsuite/gdb.xml
cvs diff: Diffing testsuite/lib
Index: testsuite/lib/gdb.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/lib/gdb.exp,v
retrieving revision 1.220
diff -u -r1.220 gdb.exp
--- testsuite/lib/gdb.exp	15 Oct 2012 17:35:54 -0000	1.220
+++ testsuite/lib/gdb.exp	25 Oct 2012 13:01:22 -0000
@@ -3434,14 +3434,6 @@
     return $ret;
 }
 
-# Like setup_kfail, but only call setup_kfail conditionally if
-# istarget[TARGET] returns true.
-proc setup_kfail_for_target { PR target } {
-    if { [istarget $target] } {
-	setup_kfail $PR $target
-    }
-}
-
 # gdb_get_line_number TEXT [FILE]
 #
 # Search the source file FILE, and return the line number of the