[patch, testsuite] require readline for gdb.linespec/explicit.exp tab-completion tests

[patch, testsuite] require readline for gdb.linespec/explicit.exp tab-completion tests

[Sandra Loosemore]

[-- Attachment #1: Type: text/plain, Size: 1269 bytes --]

Tab-completion tests require readline support.  The ones in 
gdb.base/completion.exp are already conditional, but the recently-added 
gdb.linespec/explicit.exp tests are not.  This patch fixes that, and 
also an obvious typo ("faill" -> "fail").  The patch is kind of long 
because I had to insert an "if" block and reindent, but there's nothing 
else going on here.  OK to commit?

BTW, independently of the readline-related failures on Windows host that 
are addressed by this patch, on Linux host I've also run into the 
failures already noted in this thread:

https://sourceware.org/ml/gdb-patches/2015-08/msg00432.html

The immediate problem is that the response to the "break -function 
mai\t" test doesn't always include the ^G character in the output 
pattern, and the other FAILs are cascading off of the testcase not being 
structured to recover from a pattern match failure (it's sending more 
commands without a newline to terminate the one that failed).  But, I 
have no clue why GDB is giving a ^G in some configurations and not 
others (I observed that this test fails on nios2-elf target but passes 
on nios2-linux-gnu, for instance), or whether this difference is 
indicative of an actual bug.  So I've not touched that part of the testcase.

-Sandra


[-- Attachment #2: explicit2.log --]
[-- Type: text/x-log, Size: 176 bytes --]

2015-09-17  Sandra Loosemore  <sandra@codesourcery.com>

	gdb/testsuite/
	* gdb.linespec/explicit.exp: Check for readline support for
	tab-completion tests.  Fix obvious typo.

[-- Attachment #3: explicit2.patch --]
[-- Type: text/x-patch, Size: 10606 bytes --]

diff --git a/gdb/testsuite/gdb.linespec/explicit.exp b/gdb/testsuite/gdb.linespec/explicit.exp
index a0851cd..1fa203f 100644
--- a/gdb/testsuite/gdb.linespec/explicit.exp
+++ b/gdb/testsuite/gdb.linespec/explicit.exp
@@ -145,186 +145,193 @@ namespace eval $testfile {
 	gdb_test "break $linespec thread 123" "Unknown thread 123."
     }
 
-    # Test the explicit location completer
-    foreach abbrev {"fun" "so" "lab" "li"}  full {"function" "source" "label" "line"} {
-	set tst "complete 'break -$abbrev'"
-	send_gdb "break -${abbrev}\t"
-	gdb_test_multiple "" $tst {
-	    "break -$full " {
-		send_gdb "\n"
-		gdb_test_multiple "" $tst {
-		    -re "missing argument for \"-$full\".*$gdb_prompt " {
-			pass $tst
+    # Tests below are about tab-completion, which doesn't work if readline
+    # library isn't used.  Check it first.
+    if { [readline_is_used] } {
+
+	# Test the explicit location completer
+	foreach abbrev {"fun" "so" "lab" "li"}  full {"function" "source" "label" "line"} {
+	    set tst "complete 'break -$abbrev'"
+	    send_gdb "break -${abbrev}\t"
+	    gdb_test_multiple "" $tst {
+		"break -$full " {
+		    send_gdb "\n"
+		    gdb_test_multiple "" $tst {
+			-re "missing argument for \"-$full\".*$gdb_prompt " {
+			    pass $tst
+			}
 		    }
 		}
 	    }
-	}
-	set tst "complete -$full with no value"
-	send_gdb "break -$full \t"
-	gdb_test_multiple "" $tst {
-	    -re ".*break -$full " {
-		send_gdb "\n"
-		gdb_test_multiple "" $tst {
-		    -re ".*Source filename requires function, label, or line offset\..*$gdb_prompt " {
-			if {[string equal $full "source"]} {
+	    set tst "complete -$full with no value"
+	    send_gdb "break -$full \t"
+	    gdb_test_multiple "" $tst {
+		-re ".*break -$full " {
+		    send_gdb "\n"
+		    gdb_test_multiple "" $tst {
+			-re ".*Source filename requires function, label, or line offset\..*$gdb_prompt " {
+			    if {[string equal $full "source"]} {
+				pass $tst
+			    } else {
+				fail $tst
+			    }
+			}
+			-re "missing argument for \"-$full\".*$gdb_prompt " {
 			    pass $tst
-			} else {
-			    faill $tst
 			}
 		    }
-		    -re "missing argument for \"-$full\".*$gdb_prompt " {
-			pass $tst
-		    }
 		}
 	    }
 	}
-    }
 
-    set tst "complete unique function name"
-    send_gdb "break -function mai\t"
-    gdb_test_multiple "" $tst {
-	"break -function mai\\\x07n" {
-	    send_gdb "\n"
-	    gdb_test "" ".*Breakpoint \[0-9\]+.*" $tst
-	    gdb_test_no_output "delete \$bpnum" "delete $tst breakpoint"
+	set tst "complete unique function name"
+	send_gdb "break -function mai\t"
+	gdb_test_multiple "" $tst {
+	    "break -function mai\\\x07n" {
+		send_gdb "\n"
+		gdb_test "" ".*Breakpoint \[0-9\]+.*" $tst
+		gdb_test_no_output "delete \$bpnum" "delete $tst breakpoint"
+	    }
 	}
-    }
 
-    set tst "complete non-unique function name"
-    send_gdb "break -function myfunc\t"
-    gdb_test_multiple "" $tst {
-	"break -function myfunc\\\x07tion" {
-	    send_gdb "\t\t"
-	    gdb_test_multiple "" $tst {
-		-re "\\\x07\r\nmyfunction\[ \t\]+myfunction2\[ \t\]+myfunction3\[ \t\]+myfunction4\[ \t\]+\r\n$gdb_prompt " {
-		    gdb_test "2" ".*Breakpoint \[0-9\]+.*" $tst
-		    gdb_test_no_output "delete \$bpnum" "delete $tst breakpoint"
+	set tst "complete non-unique function name"
+	send_gdb "break -function myfunc\t"
+	gdb_test_multiple "" $tst {
+	    "break -function myfunc\\\x07tion" {
+		send_gdb "\t\t"
+		gdb_test_multiple "" $tst {
+		    -re "\\\x07\r\nmyfunction\[ \t\]+myfunction2\[ \t\]+myfunction3\[ \t\]+myfunction4\[ \t\]+\r\n$gdb_prompt " {
+			gdb_test "2" ".*Breakpoint \[0-9\]+.*" $tst
+			gdb_test_no_output "delete \$bpnum" "delete $tst breakpoint"
+		    }
 		}
 	    }
 	}
-    }
 
-    set tst "complete non-existant function name"
-    send_gdb "break -function foo\t"
-    gdb_test_multiple "" $tst {
-	"break -function foo\\\x07" {
-	    send_gdb "\t\t"
-	    gdb_test_multiple "" $tst {
-		-re "\\\x07\\\x07" {
-		    send_gdb "\n"
-		    gdb_test "" {Function "foo" not defined.} $tst
+	set tst "complete non-existant function name"
+	send_gdb "break -function foo\t"
+	gdb_test_multiple "" $tst {
+	    "break -function foo\\\x07" {
+		send_gdb "\t\t"
+		gdb_test_multiple "" $tst {
+		    -re "\\\x07\\\x07" {
+			send_gdb "\n"
+			gdb_test "" {Function "foo" not defined.} $tst
+		    }
 		}
 	    }
 	}
-    }
 
-    set tst "complete unique file name"
-    send_gdb "break -source 3ex\t"
-    gdb_test_multiple "" $tst {
-	"break -source 3explicit.c " {
-	    send_gdb "\n"
-	    gdb_test "" \
-		{Source filename requires function, label, or line offset.} $tst
+	set tst "complete unique file name"
+	send_gdb "break -source 3ex\t"
+	gdb_test_multiple "" $tst {
+	    "break -source 3explicit.c " {
+		send_gdb "\n"
+		gdb_test "" \
+		    {Source filename requires function, label, or line offset.} $tst
+	    }
 	}
-    }
 
-    set tst "complete non-unique file name"
-    send_gdb "break -source exp\t"
-    gdb_test_multiple "" $tst {
-	"break -source exp\\\x07licit" {
-	    send_gdb "\t\t"
-	    gdb_test_multiple "" $tst {
-		-re "\\\x07\r\nexplicit.c\[ \t\]+explicit2.c\[ \t\]+\r\n$gdb_prompt" {
-		    send_gdb "\n"
-		    gdb_test "" \
-			{Source filename requires function, label, or line offset.} \
-			$tst
+	set tst "complete non-unique file name"
+	send_gdb "break -source exp\t"
+	gdb_test_multiple "" $tst {
+	    "break -source exp\\\x07licit" {
+		send_gdb "\t\t"
+		gdb_test_multiple "" $tst {
+		    -re "\\\x07\r\nexplicit.c\[ \t\]+explicit2.c\[ \t\]+\r\n$gdb_prompt" {
+			send_gdb "\n"
+			gdb_test "" \
+			    {Source filename requires function, label, or line offset.} \
+			    $tst
+		    }
 		}
 	    }
-	}
 
-	"break -source exp\\\x07l" {
-	    # This pattern may occur when glibc debuginfo is installed.
-	    send_gdb "\t\t"
-	    gdb_test_multiple "" $tst {
-		-re "\\\x07\r\nexplicit.c\[ \t\]+explicit2.c\[ \t\]+expl.*\r\n$gdb_prompt" {
-		    send_gdb "\n"
-		    gdb_test "" \
-			{Source filename requires function, label, or line offset.} \
-			$tst
+	    "break -source exp\\\x07l" {
+		# This pattern may occur when glibc debuginfo is installed.
+		send_gdb "\t\t"
+		gdb_test_multiple "" $tst {
+		    -re "\\\x07\r\nexplicit.c\[ \t\]+explicit2.c\[ \t\]+expl.*\r\n$gdb_prompt" {
+			send_gdb "\n"
+			gdb_test "" \
+			    {Source filename requires function, label, or line offset.} \
+			    $tst
+		    }
 		}
 	    }
 	}
-    }
 
-    set tst "complete non-existant file name"
-    send_gdb "break -source foo\t"
-    gdb_test_multiple "" $tst {
-	"break -source foo" {
-	    send_gdb "\t\t"
-	    gdb_test_multiple "" $tst {
-		"\\\x07\\\x07" {
-		    send_gdb "\n"
-		    gdb_test "" \
-			{Source filename requires function, label, or line offset.} \
-			$tst
+	set tst "complete non-existant file name"
+	send_gdb "break -source foo\t"
+	gdb_test_multiple "" $tst {
+	    "break -source foo" {
+		send_gdb "\t\t"
+		gdb_test_multiple "" $tst {
+		    "\\\x07\\\x07" {
+			send_gdb "\n"
+			gdb_test "" \
+			    {Source filename requires function, label, or line offset.} \
+			    $tst
+		    }
 		}
 	    }
 	}
-    }
 
-    set tst "complete filename and unique function name"
-    send_gdb "break -source explicit.c -function ma\t"
-    gdb_test_multiple "" $tst {
-	"break -source explicit.c -function main " {
-	    send_gdb "\n"
-	    gdb_test "" ".*Breakpoint .*" $tst
-	    gdb_test_no_output "delete \$bpnum" "delete $tst breakpoint"
+	set tst "complete filename and unique function name"
+	send_gdb "break -source explicit.c -function ma\t"
+	gdb_test_multiple "" $tst {
+	    "break -source explicit.c -function main " {
+		send_gdb "\n"
+		gdb_test "" ".*Breakpoint .*" $tst
+		gdb_test_no_output "delete \$bpnum" "delete $tst breakpoint"
+	    }
 	}
-    }
 
-    set tst "complete filename and non-unique function name"
-    send_gdb "break -so 3explicit.c -func myfunc\t"
-    gdb_test_multiple "" $tst {
-	"break -so 3explicit.c -func myfunc\\\x07tion" {
-	    send_gdb "\t\t"
-	    gdb_test_multiple "" $tst {
-		-re "\\\x07\r\nmyfunction3\[ \t\]+myfunction4\[ \t\]+\r\n$gdb_prompt " {
-		    gdb_test "3" ".*Breakpoint \[0-9\]+.*" $tst
-		    gdb_test_no_output "delete \$bpnum" "delete $tst breakpoint"
+	set tst "complete filename and non-unique function name"
+	send_gdb "break -so 3explicit.c -func myfunc\t"
+	gdb_test_multiple "" $tst {
+	    "break -so 3explicit.c -func myfunc\\\x07tion" {
+		send_gdb "\t\t"
+		gdb_test_multiple "" $tst {
+		    -re "\\\x07\r\nmyfunction3\[ \t\]+myfunction4\[ \t\]+\r\n$gdb_prompt " {
+			gdb_test "3" ".*Breakpoint \[0-9\]+.*" $tst
+			gdb_test_no_output "delete \$bpnum" "delete $tst breakpoint"
+		    }
 		}
 	    }
 	}
-    }
 
-    set tst "complete filename and non-existant function name"
-    send_gdb "break -sou 3explicit.c -fun foo\t"
-    gdb_test_multiple "" $tst {
-	"break -sou 3explicit.c -fun foo\\\x07" {
-	    send_gdb "\t\t"
-	    gdb_test_multiple "" $tst {
-		"\\\x07\\\x07" {
-		    send_gdb "\n"
-		    gdb_test "" \
-			{Function "foo" not defined in "3explicit.c".} $tst
+	set tst "complete filename and non-existant function name"
+	send_gdb "break -sou 3explicit.c -fun foo\t"
+	gdb_test_multiple "" $tst {
+	    "break -sou 3explicit.c -fun foo\\\x07" {
+		send_gdb "\t\t"
+		gdb_test_multiple "" $tst {
+		    "\\\x07\\\x07" {
+			send_gdb "\n"
+			gdb_test "" \
+			    {Function "foo" not defined in "3explicit.c".} $tst
+		    }
 		}
 	    }
 	}
-    }
 
-    set tst "complete filename and function reversed"
-    send_gdb "break -func myfunction4 -source 3ex\t"
-    gdb_test_multiple "" $tst {
-	"break -func myfunction4 -source 3explicit.c " {
-	    send_gdb "\n"
-	    gdb_test "" "Breakpoint \[0-9\]+.*" $tst
-	    gdb_test_no_output "delete \$bpnum" "delete $tst breakpoint"
+	set tst "complete filename and function reversed"
+	send_gdb "break -func myfunction4 -source 3ex\t"
+	gdb_test_multiple "" $tst {
+	    "break -func myfunction4 -source 3explicit.c " {
+		send_gdb "\n"
+		gdb_test "" "Breakpoint \[0-9\]+.*" $tst
+		gdb_test_no_output "delete \$bpnum" "delete $tst breakpoint"
+	    }
 	}
-    }
 
-    # NOTE: We don't bother testing more elaborate combinations of options,
-    # such as "-func main -sour 3ex\t" (main is defined in explicit.c).  The
-    # completer cannot handle these yet.
+	# NOTE: We don't bother testing more elaborate combinations of options,
+	# such as "-func main -sour 3ex\t" (main is defined in explicit.c).
+	# The completer cannot handle these yet.
+
+    }
+    # End of completion tests.
 
     # Test pending explicit breakpoints
     gdb_exit

Re: [patch, testsuite] require readline for gdb.linespec/explicit.exp tab-completion tests

[Keith Seitz]

[-- Attachment #1: Type: text/plain, Size: 1876 bytes --]

On 09/17/2015 12:28 AM, Sandra Loosemore wrote:
> Tab-completion tests require readline support.  The ones in
> gdb.base/completion.exp are already conditional, but the recently-added
> gdb.linespec/explicit.exp tests are not.  This patch fixes that, and
> also an obvious typo ("faill" -> "fail").  The patch is kind of long
> because I had to insert an "if" block and reindent, but there's nothing
> else going on here.  OK to commit?

I'm not a maintainer, but I wouldn't hesitate to commit this as obvious.
Thank you for catching (and fixing) that.

> The immediate problem is that the response to the "break -function
> mai\t" test doesn't always include the ^G character in the output
> pattern, and the other FAILs are cascading off of the testcase not being
> structured to recover from a pattern match failure (it's sending more
> commands without a newline to terminate the one that failed).  But, I
> have no clue why GDB is giving a ^G in some configurations and not
> others (I observed that this test fails on nios2-elf target but passes
> on nios2-linux-gnu, for instance), or whether this difference is
> indicative of an actual bug.  So I've not touched that part of the
> testcase.

Can you reliably reproduce the failure? I have been unable to do so
locally, but I have a couple of guesses why this test might fail.

I've attached a patch where I've made two changes. First, I've switched
from using "main" as a unique completion to some goofy new function
where the name is hopefully truly unique.

Second I believe the test is incorrect (or at least in combination with
some readline versions?). ^G is output only for non-unique completions.
So with this particular test, I don't think it should be present at all.
I've removed it in the patch.

If you can reliably reproduce the failure, I sure would love to know
whether this patch helps at all.

Keith

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: explicit.exp-unique-function.patch --]
[-- Type: text/x-patch; name="explicit.exp-unique-function.patch", Size: 1631 bytes --]

diff --git a/gdb/testsuite/gdb.linespec/explicit.c b/gdb/testsuite/gdb.linespec/explicit.c
index 4e1c635..5b65311 100644
--- a/gdb/testsuite/gdb.linespec/explicit.c
+++ b/gdb/testsuite/gdb.linespec/explicit.c
@@ -1,6 +1,6 @@
 /* This testcase is part of GDB, the GNU debugger.
 
-   Copyright 2012-2013 Free Software Foundation, Inc.
+   Copyright 2012-2015 Free Software Foundation, Inc.
 
    This program is free software; you can redistribute it and/or modify
    it under the terms of the GNU General Public License as published by
@@ -42,6 +42,12 @@ myfunction (int arg)
   return r;
 }
 
+static int
+explicit_exp_really_unique_function_name (int arg)
+{
+  return arg;
+}
+
 int
 main (void)
 {
@@ -52,5 +58,5 @@ main (void)
   for (i = 0, j = 0; i < 1000; ++i)
     j += myfunction (0);
 
-  return myfunction2 (j);
+  return explicit_exp_really_unique_function_name (myfunction2 (j));
 }
diff --git a/gdb/testsuite/gdb.linespec/explicit.exp b/gdb/testsuite/gdb.linespec/explicit.exp
index a0851cd..6b9c8e0 100644
--- a/gdb/testsuite/gdb.linespec/explicit.exp
+++ b/gdb/testsuite/gdb.linespec/explicit.exp
@@ -181,9 +181,9 @@ namespace eval $testfile {
     }
 
     set tst "complete unique function name"
-    send_gdb "break -function mai\t"
+    send_gdb "break -function explicit_exp_really_unique_f\t"
     gdb_test_multiple "" $tst {
-	"break -function mai\\\x07n" {
+	"break -function explicit_exp_really_unique_function_name " {
 	    send_gdb "\n"
 	    gdb_test "" ".*Breakpoint \[0-9\]+.*" $tst
 	    gdb_test_no_output "delete \$bpnum" "delete $tst breakpoint"

Re: [patch, testsuite] require readline for gdb.linespec/explicit.exp tab-completion tests

[Sandra Loosemore]

On 09/17/2015 12:50 PM, Keith Seitz wrote:
> On 09/17/2015 12:28 AM, Sandra Loosemore wrote:
>
>> The immediate problem is that the response to the "break -function
>> mai\t" test doesn't always include the ^G character in the output
>> pattern, and the other FAILs are cascading off of the testcase not being
>> structured to recover from a pattern match failure (it's sending more
>> commands without a newline to terminate the one that failed).  But, I
>> have no clue why GDB is giving a ^G in some configurations and not
>> others (I observed that this test fails on nios2-elf target but passes
>> on nios2-linux-gnu, for instance), or whether this difference is
>> indicative of an actual bug.  So I've not touched that part of the
>> testcase.
>
> Can you reliably reproduce the failure? I have been unable to do so
> locally, but I have a couple of guesses why this test might fail.
>
> I've attached a patch where I've made two changes. First, I've switched
> from using "main" as a unique completion to some goofy new function
> where the name is hopefully truly unique.
>
> Second I believe the test is incorrect (or at least in combination with
> some readline versions?). ^G is output only for non-unique completions.
> So with this particular test, I don't think it should be present at all.
> I've removed it in the patch.
>
> If you can reliably reproduce the failure, I sure would love to know
> whether this patch helps at all.

Your patch makes these tests all PASS on both nios2-elf (where it 
formerly FAILed due to not beeping) and nios2-linux-gnu (where it used 
to beep and PASS).  I had come to pretty much the same conclusions -- I 
didn't think it was supposed to beep if there was a unique completion, 
and I thought there must be some sort of a duplicate symbol table entry 
for "main" on the Linux target coming from the startup code or something 
like that.  But it looks to me like the completer *is* finding a unique 
completion after beeping anyway.

So, while your patch does fix the FAILs, I'm wondering if there is a 
genuine code bug in the completer here; is it using different logic for 
deciding whether to beep than in what it does afterwards?  Personally, 
I'd feel uncomfortable changing the testcase to use something other than 
"main" without understanding why it beeps in some configurations and not 
others, because that might just be papering over the bug instead of 
fixing a bad testcase.

-Sandra

Re: [patch, testsuite] require readline for gdb.linespec/explicit.exp tab-completion tests

[Doug Evans]

Keith Seitz <keiths@redhat.com> writes:
> On 09/17/2015 12:28 AM, Sandra Loosemore wrote:
>> Tab-completion tests require readline support.  The ones in
>> gdb.base/completion.exp are already conditional, but the recently-added
>> gdb.linespec/explicit.exp tests are not.  This patch fixes that, and
>> also an obvious typo ("faill" -> "fail").  The patch is kind of long
>> because I had to insert an "if" block and reindent, but there's nothing
>> else going on here.  OK to commit?
>
> I'm not a maintainer, but I wouldn't hesitate to commit this as obvious.
> Thank you for catching (and fixing) that.

Agreed.

Re: [patch, testsuite] require readline for gdb.linespec/explicit.exp tab-completion tests

[Sandra Loosemore]

On 09/17/2015 10:30 PM, Doug Evans wrote:
> Keith Seitz <keiths@redhat.com> writes:
>> On 09/17/2015 12:28 AM, Sandra Loosemore wrote:
>>> Tab-completion tests require readline support.  The ones in
>>> gdb.base/completion.exp are already conditional, but the recently-added
>>> gdb.linespec/explicit.exp tests are not.  This patch fixes that, and
>>> also an obvious typo ("faill" -> "fail").  The patch is kind of long
>>> because I had to insert an "if" block and reindent, but there's nothing
>>> else going on here.  OK to commit?
>>
>> I'm not a maintainer, but I wouldn't hesitate to commit this as obvious.
>> Thank you for catching (and fixing) that.
>
> Agreed.

OK, since this seems "obvious" and uncontroversial I've gone ahead and 
pushed this patch to get it out of my pile.

-Sandra