[gdb/testsuite] XFAIL under Clang tests using label debug info

Message ID CAENS6EtFF4hPJe3uqJBkPf4q8c0n4RvV1_jO=6mWWEp=X2HbnA@mail.gmail.com
State Committed
Headers

Commit Message

David Blaikie April 13, 2014, 11:25 p.m. UTC
  Clang doesn't emit debug info for labels (Clang PR14500). XFAIL a
bunch of GDB tests that rely on debug info for labels.

For some reason gdb.linespec/ls-expr.exp gathered all tests into a
dictionary and then ran them. This made it hard to XFAIL just the
right tests. I refactored this to execute the tests directly, removing
the dictionary so I could XFAIL the right tests. Is there a reason it
would've been written that way? Does my patch break it in some way?
commit c438cb16b63292e415330f289616c4e4ecece63c
Author: David Blaikie <dblaikie@gmail.com>
Date:   Sun Apr 13 11:42:02 2014 -0700

    XFAIL under Clang tests using labels
    
    gdb/testsuite/
    	* gdb.base/label.exp: XFAIL label related tests under Clang.
    	* gdb.cp/cplabel.exp: Ditto.
    	* gdb.linespec/ls-errs.exp: Refactor tests to execute directly
    	and XFAIL under Clang those using labels.
  

Comments

Doug Evans April 23, 2014, 10:03 p.m. UTC | #1
David Blaikie writes:
 > Clang doesn't emit debug info for labels (Clang PR14500). XFAIL a
 > bunch of GDB tests that rely on debug info for labels.
 > 
 > For some reason gdb.linespec/ls-expr.exp gathered all tests into a
 > dictionary and then ran them. This made it hard to XFAIL just the
 > right tests. I refactored this to execute the tests directly, removing
 > the dictionary so I could XFAIL the right tests. Is there a reason it
 > would've been written that way? Does my patch break it in some way?
 > commit c438cb16b63292e415330f289616c4e4ecece63c
 > Author: David Blaikie <dblaikie@gmail.com>
 > Date:   Sun Apr 13 11:42:02 2014 -0700
 > 
 >     XFAIL under Clang tests using labels
 >     
 >     gdb/testsuite/
 >     	* gdb.base/label.exp: XFAIL label related tests under Clang.
 >     	* gdb.cp/cplabel.exp: Ditto.
 >     	* gdb.linespec/ls-errs.exp: Refactor tests to execute directly
 >     	and XFAIL under Clang those using labels.

LGTM

 > diff --git gdb/testsuite/ChangeLog gdb/testsuite/ChangeLog
 > index 730c116..b04b940 100644
 > --- gdb/testsuite/ChangeLog
 > +++ gdb/testsuite/ChangeLog
 > @@ -1,3 +1,9 @@
 > +2014-04-12  David Blaikie  <dblaikie@gmail.com>
 > +
 > +        * gdb.base/label.exp: XFAIL label related tests under Clang.
 > +	* gdb.cp/cplabel.exp: Ditto.
 > +	* gdb.linespec/ls-errs.exp: Refactor tests to execute directly and XFAIL under Clang those using labels.
 > +

Nit: space vs tabs.  Just use tabs.
Plus line is longer than 80 chars.
  
Pedro Alves April 24, 2014, 10:42 a.m. UTC | #2
Hi David,

On 04/14/2014 12:25 AM, David Blaikie wrote:
>  
> +if {[test_compiler_info {clang-*-*}]} {  setup_xfail clang/14500 *-*-* }
>  gdb_test "break here" \
>    "Breakpoint.*at.*" \
>    "breakpoint here"
>  
> +if {[test_compiler_info {clang-*-*}]} {  setup_xfail clang/14500 *-*-* }

A suggestion -- in these cases where we have a bunch of similar
setup_xfails for a single bug, it's good to do as e.g.,
gdb.cp/temargs.exp does.  That is, something like

set have_clang_14500_bug [test_compiler_info {clang-*-*}]

# Short description of bug here.
proc setup_xfail_clang_14500 {} {
  global have_clang_14500_bug

  if { $have_clang_14500_bug } {
     setup_xfail clang/14500 *-*-*
  }
}

And then:

+ setup_xfail_clang_14500
 gdb_test "break main:there" \
   "Breakpoint.*at.*" \
   "breakpoint there"

etc.

A style like that adds less clutter, and, is less typo prone than:

> +if {[test_compiler_info {clang-*-*}]} {  setup_xfail clang/14500 *-*-* }

all over.  And it's a single place to change the condition to be
dependent on clang version, in the future, if you want to.
  
David Blaikie April 25, 2014, 3:23 a.m. UTC | #3
On Wed, Apr 23, 2014 at 3:03 PM, Doug Evans <dje@google.com> wrote:
> David Blaikie writes:
>  > Clang doesn't emit debug info for labels (Clang PR14500). XFAIL a
>  > bunch of GDB tests that rely on debug info for labels.
>  >
>  > For some reason gdb.linespec/ls-expr.exp gathered all tests into a
>  > dictionary and then ran them. This made it hard to XFAIL just the
>  > right tests. I refactored this to execute the tests directly, removing
>  > the dictionary so I could XFAIL the right tests. Is there a reason it
>  > would've been written that way? Does my patch break it in some way?
>  > commit c438cb16b63292e415330f289616c4e4ecece63c
>  > Author: David Blaikie <dblaikie@gmail.com>
>  > Date:   Sun Apr 13 11:42:02 2014 -0700
>  >
>  >     XFAIL under Clang tests using labels
>  >
>  >     gdb/testsuite/
>  >      * gdb.base/label.exp: XFAIL label related tests under Clang.
>  >      * gdb.cp/cplabel.exp: Ditto.
>  >      * gdb.linespec/ls-errs.exp: Refactor tests to execute directly
>  >      and XFAIL under Clang those using labels.
>
> LGTM
>
>  > diff --git gdb/testsuite/ChangeLog gdb/testsuite/ChangeLog
>  > index 730c116..b04b940 100644
>  > --- gdb/testsuite/ChangeLog
>  > +++ gdb/testsuite/ChangeLog
>  > @@ -1,3 +1,9 @@
>  > +2014-04-12  David Blaikie  <dblaikie@gmail.com>
>  > +
>  > +        * gdb.base/label.exp: XFAIL label related tests under Clang.
>  > +    * gdb.cp/cplabel.exp: Ditto.
>  > +    * gdb.linespec/ls-errs.exp: Refactor tests to execute directly and XFAIL under Clang those using labels.
>  > +
>
> Nit: space vs tabs.  Just use tabs.
> Plus line is longer than 80 chars.

Thanks for the catches - fixed those up and committed in
c2e827ad5340fcf1735df6c77cb0311e56b985ef.

Also refactored some of the xfails along the lines of what Pedro
suggested in the one test case that had several similar failures
(gdb.base/label.exp). If/when we fix this in Clang it might be worth
refactoring into a common function (though I'm personally not very
vested in keeping the test suite usable with anything other than ToT
Clang - perhaps others are).
  

Patch

diff --git gdb/testsuite/ChangeLog gdb/testsuite/ChangeLog
index 730c116..b04b940 100644
--- gdb/testsuite/ChangeLog
+++ gdb/testsuite/ChangeLog
@@ -1,3 +1,9 @@ 
+2014-04-12  David Blaikie  <dblaikie@gmail.com>
+
+        * gdb.base/label.exp: XFAIL label related tests under Clang.
+	* gdb.cp/cplabel.exp: Ditto.
+	* gdb.linespec/ls-errs.exp: Refactor tests to execute directly and XFAIL under Clang those using labels.
+
 2014-04-12  Siva Chandra Reddy  <sivachandra@google.com>
 	    Doug Evans  <xdje42@gmail.com>
 
diff --git gdb/testsuite/gdb.base/label.exp gdb/testsuite/gdb.base/label.exp
index 87f8623..3345a98 100644
--- gdb/testsuite/gdb.base/label.exp
+++ gdb/testsuite/gdb.base/label.exp
@@ -36,24 +36,29 @@  if {![runto_main]} {
   return -1
 }
 
+if {[test_compiler_info {clang-*-*}]} {  setup_xfail clang/14500 *-*-* }
 gdb_test "break here" \
   "Breakpoint.*at.*" \
   "breakpoint here"
 
+if {[test_compiler_info {clang-*-*}]} {  setup_xfail clang/14500 *-*-* }
 gdb_test "break main:there" \
   "Breakpoint.*at.*" \
   "breakpoint there"
 
+if {[test_compiler_info {clang-*-*}]} {  setup_xfail clang/14500 *-*-* }
 gdb_test "cont" \
   "Breakpoint 3,.*" \
   "continue to 'there'"
 
+if {[test_compiler_info {clang-*-*}]} {  setup_xfail clang/14500 *-*-* }
 gdb_test "cont" \
   "Breakpoint 2,.*" \
   "continue to 'here'"
 
 rerun_to_main
 
+if {[test_compiler_info {clang-*-*}]} {  setup_xfail clang/14500 *-*-* }
 gdb_test "cont" \
   "Breakpoint 3,.*" \
   "continue to 'there' after re-run"
diff --git gdb/testsuite/gdb.cp/cplabel.exp gdb/testsuite/gdb.cp/cplabel.exp
index a9cbfec..9c0e7fe 100644
--- gdb/testsuite/gdb.cp/cplabel.exp
+++ gdb/testsuite/gdb.cp/cplabel.exp
@@ -34,6 +34,7 @@  set labels {"to_the_top" "get_out_of_here"}
 foreach m $methods {
     foreach l $labels {
 	set line [gdb_get_line_number "$m:$l"]
+	if {[test_compiler_info {clang-*-*}]} { setup_xfail clang/14500 *-*-* }
 	gdb_test "break foo::$m:$l" \
 	    "Breakpoint $decimal at $hex: file .*$srcfile, line $line\."
     }
diff --git gdb/testsuite/gdb.linespec/ls-errs.exp gdb/testsuite/gdb.linespec/ls-errs.exp
index dbff230..86056c5 100644
--- gdb/testsuite/gdb.linespec/ls-errs.exp
+++ gdb/testsuite/gdb.linespec/ls-errs.exp
@@ -28,16 +28,12 @@  gdb_test_no_output "set breakpoint pending off"
 
 # We intentionally do not use gdb_breakpoint for these tests.
 
-# Add the (invalid) LINESPEC to the test array named in ARRAY_NAME.
-# Use the index into ::error_messages MSG_ID and ARGS to create
-# an error message which is the expect result of attempting to
-# break on the given LINESPEC.
-proc add {array_name linespec msg_id args} {
+# Break at 'linespec' and expect the message in ::error_messages indexed by
+# msg_id with the associated args.
+proc test_break {linespec msg_id args} {
     global error_messages
-    upvar $array_name tests
 
-    lappend tests(linespecs) $linespec
-    set tests("$linespec") [string_to_regexp \
+    gdb_test "break $linespec" [string_to_regexp \
 				[eval format \$error_messages($msg_id) $args]]
 }
 
@@ -64,15 +60,9 @@  set spaces [list ":" ": " " :" " : " "\t:  " "  :\t" "\t:\t" " \t:\t " \
 # A list of invalid offsets.
 set invalid_offsets [list -100 +500 1000]
 
-# THE_TESTS will hold all of our test information.  Array index
-# "linespecs" will contain the complete list of all linespecs
-# to be tested.  An array index of \"$linespec\" will contain
-# the expected result.
-set the_tests(linespecs) {}
-
 # Try some simple, invalid linespecs involving spaces.
 foreach x $spaces {
-    add the_tests $x unexpected "colon"
+    test_break $x unexpected "colon"
 }
 
 # Test invalid filespecs starting with offset.  This is done
@@ -86,25 +76,25 @@  foreach x $invalid_offsets {
 	[string index $x 0] == "-"} {
 	incr offset 16
     }
-    add the_tests $x invalid_offset $offset
+    test_break $x invalid_offset $offset
 }
 
 # Test offsets with trailing tokens w/ and w/o spaces.
 foreach x $spaces {
-    add the_tests "3$x" unexpected "colon"
-    add the_tests "+10$x" unexpected "colon"
-    add the_tests "-10$x" unexpected "colon"
+    test_break "3$x" unexpected "colon"
+    test_break "+10$x" unexpected "colon"
+    test_break "-10$x" unexpected "colon"
 }
 
 foreach x {1 +1 +100 -10} {
-    add the_tests "3 $x" unexpected_opt "number" $x
-    add the_tests "+10 $x" unexpected_opt "number" $x
-    add the_tests "-10 $x" unexpected_opt "number" $x
+    test_break "3 $x" unexpected_opt "number" $x
+    test_break "+10 $x" unexpected_opt "number" $x
+    test_break "-10 $x" unexpected_opt "number" $x
 }
 
-add the_tests "3 foo" unexpected_opt "string" "foo"
-add the_tests "+10 foo" unexpected_opt "string" "foo"
-add the_tests "-10 foo" unexpected_opt "string" "foo"
+test_break "3 foo" unexpected_opt "string" "foo"
+test_break "+10 foo" unexpected_opt "string" "foo"
+test_break "-10 foo" unexpected_opt "string" "foo"
 
 # Test invalid linespecs starting with filename.
 foreach x [list "this_file_doesn't_exist.c" \
@@ -118,74 +108,70 @@  foreach x [list "this_file_doesn't_exist.c" \
 	       "\"spaces: and :colons.c\"" \
 	       "'more: :spaces: :and  colons::.c'"] {
     # Remove any quoting from FILENAME for the error message.
-    add the_tests "$x:3" invalid_file [string trim $x \"']
+    test_break "$x:3" invalid_file [string trim $x \"']
 }
 
 # Test unmatched quotes.
 foreach x {"\"src-file.c'" "'src-file.c"} {
-    add the_tests "$x:3" unmatched_quote
+    test_break "$x:3" unmatched_quote
 }
 
-add the_tests $srcfile invalid_function $srcfile
+test_break $srcfile invalid_function $srcfile
 foreach x {"foo" " foo" " foo "} {
     # Trim any leading/trailing whitespace for error messages.
-    add the_tests "$srcfile:$x" invalid_function_f [string trim $x] $srcfile
-    add the_tests "$srcfile:main:$x" invalid_label [string trim $x] "main" 
+    test_break "$srcfile:$x" invalid_function_f [string trim $x] $srcfile
+    test_break "$srcfile:main:$x" invalid_label [string trim $x] "main"
 }
 
 foreach x $spaces {
-    add the_tests "$srcfile$x" unexpected "end of input"
-    add the_tests "$srcfile:main$x" unexpected "end of input"
+    test_break "$srcfile$x" unexpected "end of input"
+    test_break "$srcfile:main$x" unexpected "end of input"
 }
 
-add the_tests "${srcfile}::" invalid_function "${srcfile}::"
-add the_tests "$srcfile:3 1" unexpected_opt "number" "1"
-add the_tests "$srcfile:3 +100" unexpected_opt "number" "+100"
-add the_tests "$srcfile:3 -100" unexpected_opt "number" "-100"
-add the_tests "$srcfile:3 foo" unexpected_opt "string" "foo"
+test_break "${srcfile}::" invalid_function "${srcfile}::"
+test_break "$srcfile:3 1" unexpected_opt "number" "1"
+test_break "$srcfile:3 +100" unexpected_opt "number" "+100"
+test_break "$srcfile:3 -100" unexpected_opt "number" "-100"
+test_break "$srcfile:3 foo" unexpected_opt "string" "foo"
 
 foreach x $invalid_offsets {
-    add the_tests "$srcfile:$x" invalid_offset_f $x $srcfile
-    add the_tests "\"$srcfile:$x\"" invalid_offset_f $x $srcfile
-    add the_tests "'$srcfile:$x'" invalid_offset_f $x $srcfile
+    test_break "$srcfile:$x" invalid_offset_f $x $srcfile
+    test_break "\"$srcfile:$x\"" invalid_offset_f $x $srcfile
+    test_break "'$srcfile:$x'" invalid_offset_f $x $srcfile
 }
 
 # Test invalid filespecs starting with function.
 foreach x {"foobar" "foo::bar" "foo.bar" "foo ." "foo bar" "foo 1" \
 	       "foo 0" "foo +10" "foo -10" "foo +100" "foo -100"} {
-    add the_tests $x invalid_function $x
+    test_break $x invalid_function $x
 }
 
 foreach x $spaces {
-    add the_tests "main${x}there" invalid_label "there" "main"
-    add the_tests "main:here${x}" unexpected "end of input"
+    test_break "main${x}there" invalid_label "there" "main"
+    if {[test_compiler_info {clang-*-*}]} {  setup_xfail clang/14500 *-*-* }
+    test_break "main:here${x}" unexpected "end of input"
 }
 
-add the_tests "main 3" invalid_function "main 3"
-add the_tests "main +100" invalid_function "main +100"
-add the_tests "main -100" invalid_function "main -100"
-add the_tests "main foo" invalid_function "main foo"
+test_break "main 3" invalid_function "main 3"
+test_break "main +100" invalid_function "main +100"
+test_break "main -100" invalid_function "main -100"
+test_break "main foo" invalid_function "main foo"
 
 foreach x {"3" "+100" "-100" "foo"} {
-    add the_tests "main:here $x" invalid_label "here $x" "main"
+    test_break "main:here $x" invalid_label "here $x" "main"
 }
 
 foreach x {"if" "task" "thread"} {
-    add the_tests $x invalid_function $x
+    test_break $x invalid_function $x
 }
 
-add the_tests "'main.c'flubber" unexpected_opt "string" "flubber"
-add the_tests "'main.c',21" invalid_function "main.c"
-add the_tests "'main.c' " invalid_function "main.c"
-add the_tests "'main.c'3" unexpected_opt "number" "3"
-add the_tests "'main.c'+3" unexpected_opt "number" "+3"
+test_break "'main.c'flubber" unexpected_opt "string" "flubber"
+test_break "'main.c',21" invalid_function "main.c"
+test_break "'main.c' " invalid_function "main.c"
+test_break "'main.c'3" unexpected_opt "number" "3"
+test_break "'main.c'+3" unexpected_opt "number" "+3"
 
 # Test undefined convenience variables.
 set x {$zippo}
-add the_tests $x invalid_var_or_func $x
-add the_tests "$srcfile:$x" invalid_var_or_func_f $x $srcfile
-
-# Run the tests
-foreach linespec $the_tests(linespecs) {
-    gdb_test "break $linespec" $the_tests("$linespec")
-}
+test_break $x invalid_var_or_func $x
+test_break "$srcfile:$x" invalid_var_or_func_f $x $srcfile