[PATCH 2/2] Add a test for "break thread thread 2".

[PATCH 2/2] Add a test for "break thread thread 2".

[Keith Seitz]

This is a follow-on patch to prove that we really do need the linespec
parser `keyword_ok' state.  This test is mentioned in the source code,
but it is never actually tested anywhere.  This patch adds it.

gdb/testsuite/ChangeLog
	* gdb.threads/bp_in_thread.c (noreturn): Rename to ...
	(thread): ... this.
	* gdb.threads/bp_in_thread.exp: Change all references of
	`noreturn' to `thread'.
	Add test for setting thread-specific breakpoint on the
	function called `thread'.
---
 gdb/testsuite/gdb.threads/bp_in_thread.c   |    4 ++--
 gdb/testsuite/gdb.threads/bp_in_thread.exp |   13 ++++++++-----
 2 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/gdb/testsuite/gdb.threads/bp_in_thread.c b/gdb/testsuite/gdb.threads/bp_in_thread.c
index 3b16b62..4e7e94c 100644
--- a/gdb/testsuite/gdb.threads/bp_in_thread.c
+++ b/gdb/testsuite/gdb.threads/bp_in_thread.c
@@ -30,7 +30,7 @@ cond_wait (pthread_cond_t *cond, pthread_mutex_t *mut)
 }
 
 void
-noreturn (void)
+thread (void)
 {
   pthread_mutex_t mut;
   pthread_cond_t cond;
@@ -46,7 +46,7 @@ noreturn (void)
 void *
 forever_pthread (void *unused)
 {
-  noreturn ();
+  thread ();
 }
 
 void
diff --git a/gdb/testsuite/gdb.threads/bp_in_thread.exp b/gdb/testsuite/gdb.threads/bp_in_thread.exp
index 306dabe..d6e2353 100644
--- a/gdb/testsuite/gdb.threads/bp_in_thread.exp
+++ b/gdb/testsuite/gdb.threads/bp_in_thread.exp
@@ -28,14 +28,17 @@ clean_restart $binfile
 
 runto_main
 
-gdb_test "break noreturn" \
+gdb_test "break thread" \
          "Breakpoint 2 at .*: file .*${srcfile}, line .*" \
-         "breakpoint on noreturn"
+         "breakpoint on thread"
 
 # Run the program and make sure GDB reports that we stopped after
-# hitting breakpoint 1 in noreturn().
+# hitting breakpoint 1 in thread().
 
 gdb_test "continue" \
-         ".*Breakpoint 2, noreturn ().*" \
-         "run to noreturn"
+         ".*Breakpoint 2, thread ().*" \
+         "run to thread()"
 
+# Make sure the linespec parser understands that "break thread thread 2"
+# is a valid thread-specific location.
+gdb_breakpoint "thread thread 2" message

[PATCH 1/2] Whitespace terminates keywords in the linespec parser.

[Keith Seitz]

This patch changes the linespec lexer so that any keyword seen
in the input stream is only a keyword if the next character is
whitespace (as opposed to a non-identifier character).

As a result, the lexer and find_condition_and_thread will both
share the same keyword-terminal behavior.

gdb/ChangeLog

	* linespec.c (linespec_lexer_lex_keyword): According to
	find_condition_and_thread, keywords must be followed by
	whitespace.  Follow that requirement here.
---
 gdb/linespec.c |    5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/gdb/linespec.c b/gdb/linespec.c
index 9ec4a5e..597df87 100644
--- a/gdb/linespec.c
+++ b/gdb/linespec.c
@@ -433,10 +433,9 @@ linespec_lexer_lex_keyword (const char *p)
 	  int len = strlen (linespec_keywords[i]);
 
 	  /* If P begins with one of the keywords and the next
-	     character is not a valid identifier character,
-	     we have found a keyword.  */
+	     character is whitespace, we have found a keyword.  */
 	  if (strncmp (p, linespec_keywords[i], len) == 0
-	      && !(isalnum (p[len]) || p[len] == '_'))
+	      && isspace (p[len]))
 	    return linespec_keywords[i];
 	}
     }

Re: [PATCH 1/2] Whitespace terminates keywords in the linespec parser.

[Joel Brobecker]

Hi Keith,

> This patch changes the linespec lexer so that any keyword seen
> in the input stream is only a keyword if the next character is
> whitespace (as opposed to a non-identifier character).
> 
> As a result, the lexer and find_condition_and_thread will both
> share the same keyword-terminal behavior.
> 
> gdb/ChangeLog
> 
> 	* linespec.c (linespec_lexer_lex_keyword): According to
> 	find_condition_and_thread, keywords must be followed by
> 	whitespace.  Follow that requirement here.

I think you know this code better than anyone, but what about
testing for the nul character as well? I'm guessing that, many times,
a keyword at the end of the linespec is going to result in an invalid
linespec, but it's still a keyword?

> ---
>  gdb/linespec.c |    5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/gdb/linespec.c b/gdb/linespec.c
> index 9ec4a5e..597df87 100644
> --- a/gdb/linespec.c
> +++ b/gdb/linespec.c
> @@ -433,10 +433,9 @@ linespec_lexer_lex_keyword (const char *p)
>  	  int len = strlen (linespec_keywords[i]);
>  
>  	  /* If P begins with one of the keywords and the next
> -	     character is not a valid identifier character,
> -	     we have found a keyword.  */
> +	     character is whitespace, we have found a keyword.  */
>  	  if (strncmp (p, linespec_keywords[i], len) == 0
> -	      && !(isalnum (p[len]) || p[len] == '_'))
> +	      && isspace (p[len]))
>  	    return linespec_keywords[i];
>  	}
>      }

-- 
Joel

Re: [PATCH 2/2] Add a test for "break thread thread 2".

[Joel Brobecker]

> gdb/testsuite/ChangeLog
> 	* gdb.threads/bp_in_thread.c (noreturn): Rename to ...
> 	(thread): ... this.
> 	* gdb.threads/bp_in_thread.exp: Change all references of
> 	`noreturn' to `thread'.
> 	Add test for setting thread-specific breakpoint on the
> 	function called `thread'.

Looks good to me.

Depending on the answer to my question about whether or not to test
for the nul character as well, we might want to augment this patch
with an additional test (Eg: "break thread thread" ?)

-- 
Joel

Re: [PATCH 1/2] Whitespace terminates keywords in the linespec parser.

[Keith Seitz]

On 03/19/2015 05:18 AM, Joel Brobecker wrote:

> I think you know this code better than anyone, but what about
> testing for the nul character as well? I'm guessing that, many times,
> a keyword at the end of the linespec is going to result in an invalid
> linespec, but it's still a keyword?

Right, but without getting too much further into this, I've elaborated 
this patch a whole lot to get better behavior.

So consider this more simplistic patch (and the test in #2) withdrawn. 
I've uncovered (and fixed) a bunch of keyword-related bugs/deficiencies.

Please see my follow-on patch, "Expand keyword lexing intelligence in 
the linespec parser."

Thank you for your review!

Keith