public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* Re: [PATCH] Fix problem handling colon in linespec, PR breakpoints/18303
@ 2016-01-11 22:34 Doug Evans
  2016-01-12  0:17 ` Don Breazeal
  2016-01-12 18:26 ` Keith Seitz
  0 siblings, 2 replies; 17+ messages in thread
From: Doug Evans @ 2016-01-11 22:34 UTC (permalink / raw)
  To: Keith Seitz; +Cc: donb, gdb-patches@sourceware.org ml

Keith Seitz writes:
  > Hi,
  >
  > Thank you for pointing this out and supplying a patch (and *tests*!).
  >
  > On 01/08/2016 10:44 AM, Don Breazeal wrote:
  >
  > > During GDB's parsing of the linespec, when the filename was not found  
in
  > > the program's symbols, GDB tried to determine if the filename string
  > > could be some other valid identifier.  Eventually it would get to a  
point
  > > where it concluded that the Windows logical volume, or whatever was to  
the
  > > left of the colon, was a C++ namespace.  When it found only one colon,  
it
  > > would assert.
  >
  > I have to admit, when I first read this, my reaction was that this is a
  > symbol lookup error. After investigating it a bit, I think it is. [More
  > rationale below.]
  >
  > > GDB's linespec grammar allows for several different linespecs that  
contain
  > > ':'.  The only one that has a number after the colon is  
filename:linenum.
  >
  > True, but for how long? I don't know. The parser actually does/could (or
  > for some brief time *did*) support offsets just about anywhere, e.g.,
  > foo.c:function:label:3. That support was removed and legacy (buggy)
  > behavior of ignoring the offset was maintained in the parser/sal
  > converter. There is no reason we couldn't support it, though.
  >
  > > There is another possible solution.  After no symbols are found for the
  > > file and we determine that it is a filename:linenum style linespec, we
  > > could just consume the filename token and parse the rest of the
  > > linespec.  That works as well, but there is no advantage to it.
  >
  > And, of course, I came up with an entirely different solution. :-)
  >
  > Essentially what is happening is that the linespec parser is calling
  > lookup_symbol on the user input (e.g., "spaces: and :colons.cc"). That
  > is causing several problems, all which assume the input is well-formed.
  > As you've discovered, that is a pretty poor assumption. Malformed (user)
  > input should not cause an assertion failure IMO.
  >
  > > I created two new tests for this.  One is just gdb.linespec/ls-errs.exp
  > > copied and converted to use C++ instead of C, and to add the Windows
  > > logical drive case.  The other is an MI test to provide a regression
  > > test for the specific case reported in PR 18303.
  >
  > EXCELLENT! Thank you so much for doing this. These tests were
  > outrageously useful while attempting to understand the problem.
  >
  > With that said, I offer *for discussion* this alternative solution,
  > which removes the assumption that input to lookup_symbol is/must be
  > well-formed.
  >
  > [There is one additional related/necessary fixlet snuck in here... The
  > C++ name parser always assumed that ':' was followed by another ':'. Bad
  > assumption. So it would return an incorrect result in the malformed  
case.]
  >
  > WDYT?
  >
  > Keith
  >
  > [apologies on mailer wrapping]
  >
  > Author: Keith Seitz <keiths@redhat.com>
  > Date:   Fri Jan 8 14:00:22 2016 -0800
  >
  > Tolerate malformed input for lookup_symbol-called functions.
  >
  > lookup_symbol is often called with user input. Consequently, any
  > function called from lookup_symbol{,_in_language} should attempt to deal
  > with malformed input gracefully. After all, malformed user input is
  > not a programming/API error.
  >
  > This patch does not attempt to find/correct all instances of this.
  > It only fixes locations in the code that trigger test suite failures.
  >
  >     gdb/ChangeLog
  >
  >     	* cp-namespace.c (cp_lookup_bare_symbol): Do not assert on
  >     	user input.
  >     	(cp_search_static_and_baseclasses): Return null_block_symbol for
  >     	malformed input.
  >     	Remove assertions.
  >     	* cp-support.c (cp_find_first_component_aux): Do not return
  >     	a prefix length for ':' unless the next character is also ':'.
  >
  > diff --git a/gdb/cp-namespace.c b/gdb/cp-namespace.c
  > index 72002d6..aa225fe 100644
  > --- a/gdb/cp-namespace.c
  > +++ b/gdb/cp-namespace.c
  > @@ -166,12 +166,6 @@ cp_lookup_bare_symbol (const struct language_defn
  > *langdef,
  >  {
  >    struct block_symbol sym;
  >
  > -  /* Note: We can't do a simple assert for ':' not being in NAME because
  > -     ':' may be in the args of a template spec.  This isn't intended to  
be
  > -     a complete test, just cheap and documentary.  */
  > -  if (strchr (name, '<') == NULL && strchr (name, '(') == NULL)
  > -    gdb_assert (strchr (name, ':') == NULL);
  > -

Heya.

The assert is intended to catch (some) violations of this
(from the function comment):

    NAME is guaranteed to not have any scope (no "::") in its name, though
    if for example NAME is a template spec then "::" may appear in the
    argument list.

This is an invariant on what name can (and cannot) be.
IOW, it is wrong to call this function with name == "foo::bar",
handling that is the caller's responsibility.
Thus I think having an assert here is ok and good
(as long as it tests for the correct thing!).

Whether it is ok to call this with name == "c:mumble" is the issue.
[or even "c:::mumble" or whatever else a user could type that
this function's caller isn't expected to handle]
On that I'm kinda ambivalent, but I like having the assert
watch for the stated invariant.

Thoughts?

  >    sym = lookup_symbol_in_static_block (name, block, domain);
  >    if (sym.symbol != NULL)
  >      return sym;
  > @@ -246,10 +240,9 @@ cp_search_static_and_baseclasses (const char *name,
  >    struct block_symbol klass_sym;
  >    struct type *klass_type;
  >
  > -  /* The test here uses <= instead of < because Fortran also uses this,
  > -     and the module.exp testcase will pass "modmany::" for NAME here.   
*/
  > -  gdb_assert (prefix_len + 2 <= strlen (name));
  > -  gdb_assert (name[prefix_len + 1] == ':');
  > +  /* Check for malformed input.  */
  > +  if (prefix_len + 2 > strlen (name) || name[prefix_len + 1] != ':')
  > +    return null_block_symbol;
  >
  >    /* Find the name of the class and the name of the method, variable,
  > etc.  */
  >
  > diff --git a/gdb/cp-support.c b/gdb/cp-support.c
  > index df127c4..a71c6ad 100644
  > --- a/gdb/cp-support.c
  > +++ b/gdb/cp-support.c
  > @@ -1037,8 +1037,13 @@ cp_find_first_component_aux (const char *name,
  > int permissive)
  >  	      return strlen (name);
  >  	    }
  >  	case '\0':
  > -	case ':':
  >  	  return index;
  > +	case ':':
  > +	  /* ':' marks a component iff the next character is also a ':'.
  > +	     Otherwise it is probably malformed input.  */
  > +	  if (name[index + 1] == ':')
  > +	    return index;
  > +	  break;

What if name[index+2] is also ':'? :-)

[btw, I think "a::::b" is illegal (note four colons),
but I don't know that for sure]

  >  	case 'o':
  >  	  /* Operator names can screw up the recursion.  */
  >  	  if (operator_possible
  >

-- 
/dje

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

* Re: [PATCH] Fix problem handling colon in linespec, PR breakpoints/18303
  2016-01-11 22:34 [PATCH] Fix problem handling colon in linespec, PR breakpoints/18303 Doug Evans
@ 2016-01-12  0:17 ` Don Breazeal
  2016-01-12 18:26 ` Keith Seitz
  1 sibling, 0 replies; 17+ messages in thread
From: Don Breazeal @ 2016-01-12  0:17 UTC (permalink / raw)
  To: Doug Evans, Keith Seitz; +Cc: gdb-patches@sourceware.org ml

On 1/11/2016 2:34 PM, Doug Evans wrote:
> Keith Seitz writes:
>   > Hi,
>   >
>   > Thank you for pointing this out and supplying a patch (and *tests*!).
>   >
>   > On 01/08/2016 10:44 AM, Don Breazeal wrote:
>   >
>   > > During GDB's parsing of the linespec, when the filename was not found  
> in
>   > > the program's symbols, GDB tried to determine if the filename string
>   > > could be some other valid identifier.  Eventually it would get to a  
> point
>   > > where it concluded that the Windows logical volume, or whatever was to  
> the
>   > > left of the colon, was a C++ namespace.  When it found only one colon,  
> it
>   > > would assert.
>   >
>   > I have to admit, when I first read this, my reaction was that this is a
>   > symbol lookup error. After investigating it a bit, I think it is. [More
>   > rationale below.]
>   >
>   > > GDB's linespec grammar allows for several different linespecs that  
> contain
>   > > ':'.  The only one that has a number after the colon is  
> filename:linenum.
>   >
>   > True, but for how long? I don't know. The parser actually does/could (or
>   > for some brief time *did*) support offsets just about anywhere, e.g.,
>   > foo.c:function:label:3. That support was removed and legacy (buggy)
>   > behavior of ignoring the offset was maintained in the parser/sal
>   > converter. There is no reason we couldn't support it, though.
>   >
>   > > There is another possible solution.  After no symbols are found for the
>   > > file and we determine that it is a filename:linenum style linespec, we
>   > > could just consume the filename token and parse the rest of the
>   > > linespec.  That works as well, but there is no advantage to it.
>   >
>   > And, of course, I came up with an entirely different solution. :-)
>   >
>   > Essentially what is happening is that the linespec parser is calling
>   > lookup_symbol on the user input (e.g., "spaces: and :colons.cc"). That
>   > is causing several problems, all which assume the input is well-formed.
>   > As you've discovered, that is a pretty poor assumption. Malformed (user)
>   > input should not cause an assertion failure IMO.
>   >
>   > > I created two new tests for this.  One is just gdb.linespec/ls-errs.exp
>   > > copied and converted to use C++ instead of C, and to add the Windows
>   > > logical drive case.  The other is an MI test to provide a regression
>   > > test for the specific case reported in PR 18303.
>   >
>   > EXCELLENT! Thank you so much for doing this. These tests were
>   > outrageously useful while attempting to understand the problem.
>   >
>   > With that said, I offer *for discussion* this alternative solution,
>   > which removes the assumption that input to lookup_symbol is/must be
>   > well-formed.
>   >
>   > [There is one additional related/necessary fixlet snuck in here... The
>   > C++ name parser always assumed that ':' was followed by another ':'. Bad
>   > assumption. So it would return an incorrect result in the malformed  
> case.]
>   >
>   > WDYT?
>   >
>   > Keith
>   >
>   > [apologies on mailer wrapping]
>   >
>   > Author: Keith Seitz <keiths@redhat.com>
>   > Date:   Fri Jan 8 14:00:22 2016 -0800
>   >
>   > Tolerate malformed input for lookup_symbol-called functions.
>   >
>   > lookup_symbol is often called with user input. Consequently, any
>   > function called from lookup_symbol{,_in_language} should attempt to deal
>   > with malformed input gracefully. After all, malformed user input is
>   > not a programming/API error.
>   >
>   > This patch does not attempt to find/correct all instances of this.
>   > It only fixes locations in the code that trigger test suite failures.
>   >
>   >     gdb/ChangeLog
>   >
>   >     	* cp-namespace.c (cp_lookup_bare_symbol): Do not assert on
>   >     	user input.
>   >     	(cp_search_static_and_baseclasses): Return null_block_symbol for
>   >     	malformed input.
>   >     	Remove assertions.
>   >     	* cp-support.c (cp_find_first_component_aux): Do not return
>   >     	a prefix length for ':' unless the next character is also ':'.
>   >
>   > diff --git a/gdb/cp-namespace.c b/gdb/cp-namespace.c
>   > index 72002d6..aa225fe 100644
>   > --- a/gdb/cp-namespace.c
>   > +++ b/gdb/cp-namespace.c
>   > @@ -166,12 +166,6 @@ cp_lookup_bare_symbol (const struct language_defn
>   > *langdef,
>   >  {
>   >    struct block_symbol sym;
>   >
>   > -  /* Note: We can't do a simple assert for ':' not being in NAME because
>   > -     ':' may be in the args of a template spec.  This isn't intended to  
> be
>   > -     a complete test, just cheap and documentary.  */
>   > -  if (strchr (name, '<') == NULL && strchr (name, '(') == NULL)
>   > -    gdb_assert (strchr (name, ':') == NULL);
>   > -
> 
> Heya.
> 
> The assert is intended to catch (some) violations of this
> (from the function comment):
> 
>     NAME is guaranteed to not have any scope (no "::") in its name, though
>     if for example NAME is a template spec then "::" may appear in the
>     argument list.
> 
> This is an invariant on what name can (and cannot) be.
> IOW, it is wrong to call this function with name == "foo::bar",
> handling that is the caller's responsibility.
> Thus I think having an assert here is ok and good
> (as long as it tests for the correct thing!).
> 
> Whether it is ok to call this with name == "c:mumble" is the issue.
> [or even "c:::mumble" or whatever else a user could type that
> this function's caller isn't expected to handle]
> On that I'm kinda ambivalent, but I like having the assert
> watch for the stated invariant.
> 
> Thoughts?

Hi Doug

I don't think the change in question directly relates to the issue my
original patch was intended to address (PR 18303) -- with only the other
changes in Keith's patch, that problem is solved.  Maybe this change
could be dealt with in a separate patch?  Keith?

Regardless, I tried a bunch of different commands and didn't ever see a
"name" containing a ':' passed to cp_lookup_bare_symbol.  Is there a way
to make that happen?  If there is a way, it seems like this assertion:

gdb_assert (cp_entire_prefix_len (name) == 0);

would address the issue while still allowing "c:mumble".  In some cases
it would be a redundant call to cp_entire_prefix_len, but that might be
better than trying to re-implement the functionality in an expression
passed to gdb_assert.

Thanks
--Don

> 
>   >    sym = lookup_symbol_in_static_block (name, block, domain);
>   >    if (sym.symbol != NULL)
>   >      return sym;
>   > @@ -246,10 +240,9 @@ cp_search_static_and_baseclasses (const char *name,
>   >    struct block_symbol klass_sym;
>   >    struct type *klass_type;
>   >
>   > -  /* The test here uses <= instead of < because Fortran also uses this,
>   > -     and the module.exp testcase will pass "modmany::" for NAME here.   
> */
>   > -  gdb_assert (prefix_len + 2 <= strlen (name));
>   > -  gdb_assert (name[prefix_len + 1] == ':');
>   > +  /* Check for malformed input.  */
>   > +  if (prefix_len + 2 > strlen (name) || name[prefix_len + 1] != ':')
>   > +    return null_block_symbol;
>   >
>   >    /* Find the name of the class and the name of the method, variable,
>   > etc.  */
>   >
>   > diff --git a/gdb/cp-support.c b/gdb/cp-support.c
>   > index df127c4..a71c6ad 100644
>   > --- a/gdb/cp-support.c
>   > +++ b/gdb/cp-support.c
>   > @@ -1037,8 +1037,13 @@ cp_find_first_component_aux (const char *name,
>   > int permissive)
>   >  	      return strlen (name);
>   >  	    }
>   >  	case '\0':
>   > -	case ':':
>   >  	  return index;
>   > +	case ':':
>   > +	  /* ':' marks a component iff the next character is also a ':'.
>   > +	     Otherwise it is probably malformed input.  */
>   > +	  if (name[index + 1] == ':')
>   > +	    return index;
>   > +	  break;
> 
> What if name[index+2] is also ':'? :-)
> 
> [btw, I think "a::::b" is illegal (note four colons),
> but I don't know that for sure]
> 
>   >  	case 'o':
>   >  	  /* Operator names can screw up the recursion.  */
>   >  	  if (operator_possible
>   >
> 

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

* Re: [PATCH] Fix problem handling colon in linespec, PR breakpoints/18303
  2016-01-11 22:34 [PATCH] Fix problem handling colon in linespec, PR breakpoints/18303 Doug Evans
  2016-01-12  0:17 ` Don Breazeal
@ 2016-01-12 18:26 ` Keith Seitz
  2016-01-25 17:26   ` [PING] " Don Breazeal
  1 sibling, 1 reply; 17+ messages in thread
From: Keith Seitz @ 2016-01-12 18:26 UTC (permalink / raw)
  To: Doug Evans; +Cc: gdb-patches@sourceware.org ml

On 01/11/2016 02:34 PM, Doug Evans wrote:
>  > -     a complete test, just cheap and documentary.  */
>  > -  if (strchr (name, '<') == NULL && strchr (name, '(') == NULL)
>  > -    gdb_assert (strchr (name, ':') == NULL);
>  > -
> 
> Heya.
> 
> The assert is intended to catch (some) violations of this
> (from the function comment):
> 
>    NAME is guaranteed to not have any scope (no "::") in its name, though
>    if for example NAME is a template spec then "::" may appear in the
>    argument list.
[snip]
> On that I'm kinda ambivalent, but I like having the assert
> watch for the stated invariant.
> 
> Thoughts?

I missed that comment. [Well, I didn't even look at it. I'm so used to
seeing no/minimal comments for symbol searching functions that I seldom
even look for them. My bad.]

That seems like a reasonable assertion, then, as long as it really does
test what it is supposed to. How about:

  if (strchr (name, '<') == NULL && strchr (name, '(') == NULL)
    gdb_assert (strstr (name, "::") == NULL);

Or something like that?

>  > diff --git a/gdb/cp-support.c b/gdb/cp-support.c
>  > index df127c4..a71c6ad 100644
>  > --- a/gdb/cp-support.c
>  > +++ b/gdb/cp-support.c
>  > @@ -1037,8 +1037,13 @@ cp_find_first_component_aux (const char *name,
>  > int permissive)
>  >            return strlen (name);
>  >          }
>  >      case '\0':
>  > -    case ':':
>  >        return index;
>  > +    case ':':
>  > +      /* ':' marks a component iff the next character is also a ':'.
>  > +         Otherwise it is probably malformed input.  */
>  > +      if (name[index + 1] == ':')
>  > +        return index;
>  > +      break;
> 
> What if name[index+2] is also ':'? :-)
> 

I don't think that matters at all. It isn't the scope operator in C++
unless it is *two* colons. Not just a single colon. [Note that I believe
we are going to have to deal with the general single-colon issue when
running this code with abitags, but that's a patch for some other time.
Or maybe this patch already mitigates that to a degree. I haven't
checked into it at all.]

Keith

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

* [PING] Re: [PATCH] Fix problem handling colon in linespec, PR breakpoints/18303
  2016-01-12 18:26 ` Keith Seitz
@ 2016-01-25 17:26   ` Don Breazeal
  2016-01-26 16:56     ` Doug Evans
  0 siblings, 1 reply; 17+ messages in thread
From: Don Breazeal @ 2016-01-25 17:26 UTC (permalink / raw)
  To: Doug Evans; +Cc: Keith Seitz, gdb-patches@sourceware.org ml

On 1/12/2016 10:26 AM, Keith Seitz wrote:
> On 01/11/2016 02:34 PM, Doug Evans wrote:
>>  > -     a complete test, just cheap and documentary.  */
>>  > -  if (strchr (name, '<') == NULL && strchr (name, '(') == NULL)
>>  > -    gdb_assert (strchr (name, ':') == NULL);
>>  > -
>>
>> Heya.
>>
>> The assert is intended to catch (some) violations of this
>> (from the function comment):
>>
>>    NAME is guaranteed to not have any scope (no "::") in its name, though
>>    if for example NAME is a template spec then "::" may appear in the
>>    argument list.
> [snip]
>> On that I'm kinda ambivalent, but I like having the assert
>> watch for the stated invariant.
>>
>> Thoughts?
> 
> I missed that comment. [Well, I didn't even look at it. I'm so used to
> seeing no/minimal comments for symbol searching functions that I seldom
> even look for them. My bad.]
> 
> That seems like a reasonable assertion, then, as long as it really does
> test what it is supposed to. How about:
> 
>   if (strchr (name, '<') == NULL && strchr (name, '(') == NULL)
>     gdb_assert (strstr (name, "::") == NULL);
> 
> Or something like that?
> 
>>  > diff --git a/gdb/cp-support.c b/gdb/cp-support.c
>>  > index df127c4..a71c6ad 100644
>>  > --- a/gdb/cp-support.c
>>  > +++ b/gdb/cp-support.c
>>  > @@ -1037,8 +1037,13 @@ cp_find_first_component_aux (const char *name,
>>  > int permissive)
>>  >            return strlen (name);
>>  >          }
>>  >      case '\0':
>>  > -    case ':':
>>  >        return index;
>>  > +    case ':':
>>  > +      /* ':' marks a component iff the next character is also a ':'.
>>  > +         Otherwise it is probably malformed input.  */
>>  > +      if (name[index + 1] == ':')
>>  > +        return index;
>>  > +      break;
>>
>> What if name[index+2] is also ':'? :-)
>>
> 
> I don't think that matters at all. It isn't the scope operator in C++
> unless it is *two* colons. Not just a single colon. [Note that I believe
> we are going to have to deal with the general single-colon issue when
> running this code with abitags, but that's a patch for some other time.
> Or maybe this patch already mitigates that to a degree. I haven't
> checked into it at all.]
> 
> Keith
> 

Hi Doug, any thoughts on earlier responses from Keith and me to your
comments on this issue?
Thanks
--Don

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

* Re: [PING] Re: [PATCH] Fix problem handling colon in linespec, PR breakpoints/18303
  2016-01-25 17:26   ` [PING] " Don Breazeal
@ 2016-01-26 16:56     ` Doug Evans
  2016-01-28  1:21       ` [PATCH v2] PR 18303, Tolerate malformed input for lookup_symbol-called functions Don Breazeal
  0 siblings, 1 reply; 17+ messages in thread
From: Doug Evans @ 2016-01-26 16:56 UTC (permalink / raw)
  To: Don Breazeal; +Cc: Keith Seitz, gdb-patches@sourceware.org ml

On Mon, Jan 25, 2016 at 9:26 AM, Don Breazeal <donb@codesourcery.com> wrote:
> On 1/12/2016 10:26 AM, Keith Seitz wrote:
>> On 01/11/2016 02:34 PM, Doug Evans wrote:
>>>  > -     a complete test, just cheap and documentary.  */
>>>  > -  if (strchr (name, '<') == NULL && strchr (name, '(') == NULL)
>>>  > -    gdb_assert (strchr (name, ':') == NULL);
>>>  > -
>>>
>>> Heya.
>>>
>>> The assert is intended to catch (some) violations of this
>>> (from the function comment):
>>>
>>>    NAME is guaranteed to not have any scope (no "::") in its name, though
>>>    if for example NAME is a template spec then "::" may appear in the
>>>    argument list.
>> [snip]
>>> On that I'm kinda ambivalent, but I like having the assert
>>> watch for the stated invariant.
>>>
>>> Thoughts?
>>
>> I missed that comment. [Well, I didn't even look at it. I'm so used to
>> seeing no/minimal comments for symbol searching functions that I seldom
>> even look for them. My bad.]
>>
>> That seems like a reasonable assertion, then, as long as it really does
>> test what it is supposed to. How about:
>>
>>   if (strchr (name, '<') == NULL && strchr (name, '(') == NULL)
>>     gdb_assert (strstr (name, "::") == NULL);
>>
>> Or something like that?
>>
>>>  > diff --git a/gdb/cp-support.c b/gdb/cp-support.c
>>>  > index df127c4..a71c6ad 100644
>>>  > --- a/gdb/cp-support.c
>>>  > +++ b/gdb/cp-support.c
>>>  > @@ -1037,8 +1037,13 @@ cp_find_first_component_aux (const char *name,
>>>  > int permissive)
>>>  >            return strlen (name);
>>>  >          }
>>>  >      case '\0':
>>>  > -    case ':':
>>>  >        return index;
>>>  > +    case ':':
>>>  > +      /* ':' marks a component iff the next character is also a ':'.
>>>  > +         Otherwise it is probably malformed input.  */
>>>  > +      if (name[index + 1] == ':')
>>>  > +        return index;
>>>  > +      break;
>>>
>>> What if name[index+2] is also ':'? :-)
>>>
>>
>> I don't think that matters at all. It isn't the scope operator in C++
>> unless it is *two* colons. Not just a single colon. [Note that I believe
>> we are going to have to deal with the general single-colon issue when
>> running this code with abitags, but that's a patch for some other time.
>> Or maybe this patch already mitigates that to a degree. I haven't
>> checked into it at all.]
>>
>> Keith
>>
>
> Hi Doug, any thoughts on earlier responses from Keith and me to your
> comments on this issue?
> Thanks
> --Don

Sorry, nothing more to add.
Keith's suggestion is fine by me.

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

* [PATCH v2] PR 18303, Tolerate malformed input for lookup_symbol-called functions.
  2016-01-26 16:56     ` Doug Evans
@ 2016-01-28  1:21       ` Don Breazeal
  2016-01-28 12:06         ` Pedro Alves
  0 siblings, 1 reply; 17+ messages in thread
From: Don Breazeal @ 2016-01-28  1:21 UTC (permalink / raw)
  To: gdb-patches

This patch includes an updated version of the alternate fix that
Keith proposed for my patch that addressed PR breakpoints/18303.  Doug
has approved (in concept, at least) the code portion of the patch.

I've also done some renaming of the new tests and consolidated all three
new tests into this one patch along with Keith's patch for the code.

Thanks,
--Don

-----------

lookup_symbol is often called with user input.  Consequently, any
function called from lookup_symbol{,_in_language} should attempt to
deal with malformed input gracefully.  After all, malformed user
input is not a programming/API error.

This patch does not attempt to find/correct all instances of this.  It
only fixes locations in the code that trigger test suite failures.

This patch fixes PR breakpoints/18303, "Assertion: -breakpoint-insert
with windows paths of file in non-current directory".

The patch includes three new tests related to this.  One is just
gdb.linespec/ls-errs.exp copied and converted to use C++ instead of C, and
to add a case using a file name containing a Windows-style logical drive
specifier.  The others include an MI test to provide a regression test for
the specific case reported in PR 18303, and a C++ test for proper error
handling of access to a program variable when using a file scope specifier
that refers to a non-existent file.

Tested on x86_64 native Linux.

gdb/ChangeLog
2016-01-27  Keith Seitz  <keiths@redhat.com>

	PR breakpoints/18303
	* cp-namespace.c (cp_lookup_bare_symbol): Change assertion to
	look for "::" instead of simply ":".
	(cp_search_static_and_baseclasses): Return null_block_symbol for
	malformed input.
	Remove assertions.
	* cp-support.c (cp_find_first_component_aux): Do not return
	a prefix length for ':' unless the next character is also ':'.

gdb/testsuite/ChangeLog
2016-01-27  Don Breazeal  <donb@codesourcery.com>

	gdb.cp/scope-err.cc: New test program.
	gdb.cp/scope-err.exp: New test script.
	gdb.linespec/ls-errs-cp.cc: New test program.
	gdb.linespec/ls-errs-cp.exp: New test script.
	gdb.mi/mi-linespec-err-cp.cc: New test program.
	gdb.mi/mi-linespec-err-cp.exp: New test script.
---
 gdb/cp-namespace.c                          |   9 +-
 gdb/cp-support.c                            |   7 +-
 gdb/testsuite/gdb.cp/scope-err.cc           |  35 ++++
 gdb/testsuite/gdb.linespec/ls-errs-cp.cc    |  36 +++++
 gdb/testsuite/gdb.linespec/ls-errs-cp.exp   | 240 ++++++++++++++++++++++++++++
 gdb/testsuite/gdb.mi/mi-linespec-err-cp.cc  |  35 ++++
 gdb/testsuite/gdb.mi/mi-linespec-err-cp.exp |  57 +++++++
 7 files changed, 413 insertions(+), 6 deletions(-)

diff --git a/gdb/cp-namespace.c b/gdb/cp-namespace.c
index 72002d6..016a42f 100644
--- a/gdb/cp-namespace.c
+++ b/gdb/cp-namespace.c
@@ -170,7 +170,7 @@ cp_lookup_bare_symbol (const struct language_defn *langdef,
      ':' may be in the args of a template spec.  This isn't intended to be
      a complete test, just cheap and documentary.  */
   if (strchr (name, '<') == NULL && strchr (name, '(') == NULL)
-    gdb_assert (strchr (name, ':') == NULL);
+    gdb_assert (strstr (name, "::") == NULL);
 
   sym = lookup_symbol_in_static_block (name, block, domain);
   if (sym.symbol != NULL)
@@ -246,10 +246,9 @@ cp_search_static_and_baseclasses (const char *name,
   struct block_symbol klass_sym;
   struct type *klass_type;
 
-  /* The test here uses <= instead of < because Fortran also uses this,
-     and the module.exp testcase will pass "modmany::" for NAME here.  */
-  gdb_assert (prefix_len + 2 <= strlen (name));
-  gdb_assert (name[prefix_len + 1] == ':');
+  /* Check for malformed input.  */
+  if (prefix_len + 2 > strlen (name) || name[prefix_len + 1] != ':')
+    return null_block_symbol;
 
   /* Find the name of the class and the name of the method, variable, etc.  */
 
diff --git a/gdb/cp-support.c b/gdb/cp-support.c
index df127c4..a71c6ad 100644
--- a/gdb/cp-support.c
+++ b/gdb/cp-support.c
@@ -1037,8 +1037,13 @@ cp_find_first_component_aux (const char *name, int permissive)
 	      return strlen (name);
 	    }
 	case '\0':
-	case ':':
 	  return index;
+	case ':':
+	  /* ':' marks a component iff the next character is also a ':'.
+	     Otherwise it is probably malformed input.  */
+	  if (name[index + 1] == ':')
+	    return index;
+	  break;
 	case 'o':
 	  /* Operator names can screw up the recursion.  */
 	  if (operator_possible
diff --git a/gdb/testsuite/gdb.cp/scope-err.cc b/gdb/testsuite/gdb.cp/scope-err.cc
new file mode 100644
index 0000000..19c92d8
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/scope-err.cc
@@ -0,0 +1,35 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2016 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
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+int
+myfunction (int aa)
+{
+  int i;
+
+  i = aa + 42;
+  return i;    /* set breakpoint here */
+}
+
+int
+main (void)
+{
+  int a;
+
+  a = myfunction (a);
+
+  return a;
+}
diff --git a/gdb/testsuite/gdb.linespec/ls-errs-cp.cc b/gdb/testsuite/gdb.linespec/ls-errs-cp.cc
new file mode 100644
index 0000000..a3a43db
--- /dev/null
+++ b/gdb/testsuite/gdb.linespec/ls-errs-cp.cc
@@ -0,0 +1,36 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2012-2016 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
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+int
+myfunction (int aa)
+{
+  int i;
+
+  i = aa + 42;
+  return i;    /* set breakpoint here */
+}
+
+int
+main (void)
+{
+  int a;
+
+  a = myfunction (a);
+
+ here:
+  return a;
+}
diff --git a/gdb/testsuite/gdb.linespec/ls-errs-cp.exp b/gdb/testsuite/gdb.linespec/ls-errs-cp.exp
new file mode 100644
index 0000000..be1c3ba
--- /dev/null
+++ b/gdb/testsuite/gdb.linespec/ls-errs-cp.exp
@@ -0,0 +1,240 @@
+# Copyright 2012-2016 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
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Tests for linespec errors with C++.
+# Derived from gdb.linespec/ls-errs.exp.
+
+if { [skip_cplus_tests] } { continue }
+
+standard_testfile .cc
+set exefile $testfile
+
+if {[prepare_for_testing $testfile $exefile $srcfile {debug c++}]} {
+    return -1
+}
+
+# Turn off the pending breakpoint queries.
+gdb_test_no_output "set breakpoint pending off"
+
+# Turn off completion limiting
+gdb_test_no_output "set max-completions unlimited"
+
+if ![runto_main] {
+    fail "Can't run to main"
+    return 0
+}
+
+# Run to a location in the file.
+set bp_location [gdb_get_line_number "set breakpoint here"]
+
+gdb_test "break $srcfile:$bp_location" \
+    "Breakpoint.*at.* file .*$srcfile, line $bp_location\\." \
+    "breakpoint line number in file"
+
+gdb_continue_to_breakpoint "$bp_location"
+
+# We intentionally do not use gdb_breakpoint for these tests.
+
+# 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
+
+    gdb_test "break $linespec" [string_to_regexp \
+				[eval format \$error_messages($msg_id) $args]]
+}
+
+# Common error message format strings.
+array set error_messages {
+    invalid_file "No source file named %s."
+    invalid_function "Function \"%s\" not defined."
+    invalid_var_or_func "Undefined convenience variable or function \"%s\" not defined."
+    invalid_function_f "Function \"%s\" not defined in \"%s\"."
+    invalid_var_or_func_f \
+	"Undefined convenience variable or function \"%s\" not defined in \"%s\"."
+    invalid_label "No label \"%s\" defined in function \"%s\"."
+    invalid_parm "invalid linespec argument, \"%s\""
+    invalid_offset "No line %d in the current file."
+    invalid_offset_f "No line %d in file \"%s\"."
+    malformed_line_offset "malformed line offset: \"%s\""
+    source_incomplete \
+	"Source filename requires function, label, or line offset."
+    unexpected "malformed linespec error: unexpected %s"
+    unexpected_opt "malformed linespec error: unexpected %s, \"%s\""
+    unmatched_quote "unmatched quote"
+    garbage "Garbage '%s' at end of command"
+}
+
+# Some commonly used whitespace tests around ':'.
+set spaces [list ":" ": " " :" " : " "\t:  " "  :\t" "\t:\t" " \t:\t " \
+		"\t  \t:\t  \t  \t"]
+
+# A list of invalid offsets.
+set invalid_offsets [list -100 +500 1000]
+
+# Try some simple, invalid linespecs involving spaces.
+foreach x $spaces {
+    test_break $x unexpected "colon"
+}
+
+# Test invalid filespecs starting with offset.  This is done
+# first so that default offsets are tested.
+foreach x $invalid_offsets {
+    set offset $x
+
+    # Relative offsets are relative to line 16.  Adjust
+    # expected offset from error message accordingly.
+    if {[string index $x 0] == "+" ||
+	[string index $x 0] == "-"} {
+	incr offset 24
+    }
+    test_break $x invalid_offset $offset
+    test_break "-line $x" invalid_offset $offset
+}
+
+# Test offsets with trailing tokens w/ and w/o spaces.
+foreach x $spaces {
+    test_break "3$x" unexpected "colon"
+    test_break "+10$x" unexpected "colon"
+    test_break "-10$x" unexpected "colon"
+}
+
+foreach x {1 +1 +100 -10} {
+    test_break "3 $x" unexpected_opt "number" $x
+    test_break "-line 3 $x" garbage $x
+    test_break "+10 $x" unexpected_opt "number" $x
+    test_break "-line +10 $x" garbage $x
+    test_break "-10 $x" unexpected_opt "number" $x
+    test_break "-line -10 $x" garbage $x
+}
+
+foreach x {3 +10 -10} {
+    test_break "$x foo" unexpected_opt "string" "foo"
+    test_break "-line $x foo" garbage "foo"
+}
+
+# Test invalid linespecs starting with filename.
+set invalid_files [list "this_file_doesn't_exist.cc" \
+		   "this file has spaces.cc" \
+	           "\"file::colons.cc\"" \
+	           "'file::colons.cc'" \
+	           "\"this \"file\" has quotes.cc\"" \
+	           "'this \"file\" has quotes.cc'" \
+	           "'this 'file' has quotes.cc'" \
+	           "\"this 'file' has quotes.cc\"" \
+	           "\"spaces: and :colons.cc\"" \
+	           "'more: :spaces: :and  colons::.cc'" \
+		   "C:/nonexist-with-windrive.cc"]
+
+foreach x $invalid_files {
+    # Remove any quoting from FILENAME for the error message.
+    test_break "$x:3" invalid_file [string trim $x \"']
+}
+foreach x [list "this_file_doesn't_exist.cc" \
+	       "file::colons.cc" \
+	       "'file::colons.cc'"] {
+    test_break "-source $x -line 3" \
+	invalid_file [string trim $x \"']
+}
+
+# Test that option lexing stops at whitespace boundaries
+test_break "-source this file has spaces.cc -line 3" \
+    invalid_file "this"
+
+test_break "-function function whitespace" \
+    invalid_function "function"
+
+test_break "-source $srcfile -function function whitespace" \
+    invalid_function_f "function" $srcfile
+
+test_break "-function main -label label whitespace" \
+    invalid_label "label" "main"
+
+# Test unmatched quotes.
+foreach x {"\"src-file.cc'" "'src-file.cc"} {
+    test_break "$x:3" unmatched_quote
+}
+
+test_break $srcfile invalid_function $srcfile
+foreach x {"foo" " foo" " foo "} {
+    # Trim any leading/trailing whitespace for error messages.
+    test_break "$srcfile:$x" invalid_function_f [string trim $x] $srcfile
+    test_break "-source $srcfile -function $x" \
+	invalid_function_f [string trim $x] $srcfile
+    test_break "$srcfile:main:$x" invalid_label [string trim $x] "main"
+    test_break "-source $srcfile -function main -label $x" \
+	invalid_label [string trim $x] "main"
+}
+
+foreach x $spaces {
+    test_break "$srcfile$x" unexpected "end of input"
+    test_break "$srcfile:main$x" unexpected "end of input"
+}
+
+test_break "${srcfile}::" invalid_function "${srcfile}::"
+test_break "$srcfile:3 1" unexpected_opt "number" "1"
+test_break "-source $srcfile -line 3 1" garbage "1"
+test_break "$srcfile:3 +100" unexpected_opt "number" "+100"
+test_break "-source $srcfile -line 3 +100" garbage "+100"
+test_break "$srcfile:3 -100" unexpected_opt "number" "-100"
+test_break "$srcfile:3 foo" unexpected_opt "string" "foo"
+test_break "-source $srcfile -line 3 foo" garbage "foo"
+
+foreach x $invalid_offsets {
+    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_break "-source $srcfile -line $x" invalid_offset_f $x $srcfile
+}
+test_break "-source $srcfile -line -x" malformed_line_offset "-x"
+
+# 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"} {
+    test_break $x invalid_function $x
+    test_break "-function \"$x\"" invalid_function $x
+}
+
+foreach x $spaces {
+    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"
+}
+
+foreach x {"3" "+100" "-100" "foo"} {
+    test_break "main 3" invalid_function "main 3"
+    test_break "-function \"main $x\"" invalid_function "main $x"
+    test_break "main:here $x" invalid_label "here $x" "main"
+    test_break "-function main -label \"here $x\"" \
+	invalid_label "here $x" "main"
+}
+
+foreach x {"if" "task" "thread"} {
+    test_break $x invalid_function $x
+}
+
+test_break "'main.cc'flubber" unexpected_opt "string" "flubber"
+test_break "'main.cc',21" invalid_function "main.cc"
+test_break "'main.cc' " invalid_function "main.cc"
+test_break "'main.cc'3" unexpected_opt "number" "3"
+test_break "'main.cc'+3" unexpected_opt "number" "+3"
+
+# Test undefined convenience variables.
+set x {$zippo}
+test_break $x invalid_var_or_func $x
+test_break "$srcfile:$x" invalid_var_or_func_f $x $srcfile
+
+# Explicit linespec-specific tests
+test_break "-source $srcfile" source_incomplete
diff --git a/gdb/testsuite/gdb.mi/mi-linespec-err-cp.cc b/gdb/testsuite/gdb.mi/mi-linespec-err-cp.cc
new file mode 100644
index 0000000..19c92d8
--- /dev/null
+++ b/gdb/testsuite/gdb.mi/mi-linespec-err-cp.cc
@@ -0,0 +1,35 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2016 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
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+int
+myfunction (int aa)
+{
+  int i;
+
+  i = aa + 42;
+  return i;    /* set breakpoint here */
+}
+
+int
+main (void)
+{
+  int a;
+
+  a = myfunction (a);
+
+  return a;
+}
diff --git a/gdb/testsuite/gdb.mi/mi-linespec-err-cp.exp b/gdb/testsuite/gdb.mi/mi-linespec-err-cp.exp
new file mode 100644
index 0000000..1a8682e
--- /dev/null
+++ b/gdb/testsuite/gdb.mi/mi-linespec-err-cp.exp
@@ -0,0 +1,57 @@
+# Copyright 2016 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
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Regression test for PR breakpoints/18303.  Tests that the correct
+# errors is generated when setting a breakpoint in a non-existent
+# file with a Windows-style logical drive names and C++.
+
+if { [skip_cplus_tests] } { continue }
+
+load_lib mi-support.exp
+set MIFLAGS "-i=mi"
+
+standard_testfile .cc
+set exefile $testfile
+
+if {[prepare_for_testing $testfile $exefile $srcfile {debug c++}]} {
+    return -1
+}
+
+gdb_exit
+if [mi_gdb_start] {
+    continue
+}
+
+# Turn off the pending breakpoint queries.
+mi_gdb_test "-interpreter-exec console \"set breakpoint pending off\"" \
+  {=cmd-param-changed,param=\"breakpoint pending\",.*\^done} \
+  "-interpreter-exec console \"set breakpoint pending off\""
+
+mi_run_to_main
+
+# Run to a location in the file.
+set bp_location [gdb_get_line_number "set breakpoint here"]
+
+mi_gdb_test "-break-insert ${srcfile}:${bp_location}" \
+    {\^done,bkpt=.number="2",type="breakpoint".*\}} "set breakpoint"
+
+mi_execute_to "exec-continue" "breakpoint-hit" "myfunction" ".*" ".*" "24" \
+    { "" "disp=\"keep\"" } "breakpoint hit"
+
+# Set a breakpoint in a C++ source file whose name contains a
+# Windows-style logical drive.
+mi_gdb_test \
+    "-break-insert -f \"c:/uu.cpp:13\"" \
+    ".*No source file named c:/uu.cpp.*"
-- 
1.8.1.1

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

* Re: [PATCH v2] PR 18303, Tolerate malformed input for lookup_symbol-called functions.
  2016-01-28  1:21       ` [PATCH v2] PR 18303, Tolerate malformed input for lookup_symbol-called functions Don Breazeal
@ 2016-01-28 12:06         ` Pedro Alves
  2016-01-28 22:43           ` [PATCH v3] " Don Breazeal
  0 siblings, 1 reply; 17+ messages in thread
From: Pedro Alves @ 2016-01-28 12:06 UTC (permalink / raw)
  To: Don Breazeal, gdb-patches

On 01/28/2016 01:21 AM, Don Breazeal wrote:
> The patch includes three new tests related to this.  One is just
> gdb.linespec/ls-errs.exp copied and converted to use C++ instead of C, and
> to add a case using a file name containing a Windows-style logical drive
> specifier.  
....
> gdb/testsuite/gdb.linespec/ls-errs-cp.cc    |  36 +++++
> gdb/testsuite/gdb.linespec/ls-errs-cp.exp   | 240 ++++++++++++++++++++++++++++
...

Can't we somehow reuse the existing test?  Say, either:

 - move the main body of ls-errs.exp a procedure, and call it twice,
   once for each language, or,

 - make gdb.linespec/ls-errs-cp.exp set some $language var and then
   source gdb.linespec/ls-errs.exp, like gdb.base/checkpoint-ns.exp.

Thanks,
Pedro Alves

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

* [PATCH v3] PR 18303, Tolerate malformed input for lookup_symbol-called functions.
  2016-01-28 12:06         ` Pedro Alves
@ 2016-01-28 22:43           ` Don Breazeal
  2016-01-28 22:52             ` [PATCH v4] " Don Breazeal
  0 siblings, 1 reply; 17+ messages in thread
From: Don Breazeal @ 2016-01-28 22:43 UTC (permalink / raw)
  To: gdb-patches

This patch differs from the previous version in that the new test 
gdb.linespec/ls-errs-cp.exp is gone, and instead (per Pedro's suggestion)
gdb.linespec/ls-errs.exp has been modified to test both C & C++, and to
incorporate the small amount of extra testing from ls-errs-cp.exp.

It also includes, unchanged from the previous version of the patch:

 * an updated version of the alternate fix that Keith proposed for my
   patch that addressed PR breakpoints/18303.  Doug has approved (in
   concept, at least) the code portion of the patch.

 * a couple of other new tests of colons in linespecs.

Thanks,
--Don

-----------

lookup_symbol is often called with user input.  Consequently, any
function called from lookup_symbol{,_in_language} should attempt to
deal with malformed input gracefully.  After all, malformed user
input is not a programming/API error.

This patch does not attempt to find/correct all instances of this.  It
only fixes locations in the code that trigger test suite failures.

This patch fixes PR breakpoints/18303, "Assertion: -breakpoint-insert
with windows paths of file in non-current directory".

The patch includes three new tests related to this.  One is just
gdb.linespec/ls-errs.exp copied and converted to use C++ instead of C, and
to add a case using a file name containing a Windows-style logical drive
specifier.  The others include an MI test to provide a regression test for
the specific case reported in PR 18303, and a C++ test for proper error
handling of access to a program variable when using a file scope specifier
that refers to a non-existent file.

Tested on x86_64 native Linux.

gdb/ChangeLog
2016-01-28  Keith Seitz  <keiths@redhat.com>

	PR breakpoints/18303
	* cp-namespace.c (cp_lookup_bare_symbol): Change assertion to
	look for "::" instead of simply ":".
	(cp_search_static_and_baseclasses): Return null_block_symbol for
	malformed input.
	Remove assertions.
	* cp-support.c (cp_find_first_component_aux): Do not return
	a prefix length for ':' unless the next character is also ':'.

gdb/testsuite/ChangeLog
2016-01-28  Don Breazeal  <donb@codesourcery.com>

	* gdb.cp/scope-err.cc: New test program.
	* gdb.cp/scope-err.exp: New test script.
	* gdb.linespec/ls-errs.c (myfunction): Expanded to have multiple
	lines and "set breakpoint here" comment.
	* gdb.linespec/ls-errs.exp: Added C++ testing and new test case.
	* gdb.mi/mi-linespec-err-cp.cc: New test program.
	* gdb.mi/mi-linespec-err-cp.exp: New test script.

---
 gdb/cp-namespace.c                          |   9 +-
 gdb/cp-support.c                            |   7 +-
 gdb/testsuite/gdb.cp/scope-err.cc           |  35 +++
 gdb/testsuite/gdb.linespec/ls-errs.c        |  13 +-
 gdb/testsuite/gdb.linespec/ls-errs.exp      | 384 +++++++++++++++-------------
 gdb/testsuite/gdb.mi/mi-linespec-err-cp.cc  |  35 +++
 gdb/testsuite/gdb.mi/mi-linespec-err-cp.exp |  57 +++++
 7 files changed, 358 insertions(+), 182 deletions(-)

diff --git a/gdb/cp-namespace.c b/gdb/cp-namespace.c
index 72002d6..016a42f 100644
--- a/gdb/cp-namespace.c
+++ b/gdb/cp-namespace.c
@@ -170,7 +170,7 @@ cp_lookup_bare_symbol (const struct language_defn *langdef,
      ':' may be in the args of a template spec.  This isn't intended to be
      a complete test, just cheap and documentary.  */
   if (strchr (name, '<') == NULL && strchr (name, '(') == NULL)
-    gdb_assert (strchr (name, ':') == NULL);
+    gdb_assert (strstr (name, "::") == NULL);
 
   sym = lookup_symbol_in_static_block (name, block, domain);
   if (sym.symbol != NULL)
@@ -246,10 +246,9 @@ cp_search_static_and_baseclasses (const char *name,
   struct block_symbol klass_sym;
   struct type *klass_type;
 
-  /* The test here uses <= instead of < because Fortran also uses this,
-     and the module.exp testcase will pass "modmany::" for NAME here.  */
-  gdb_assert (prefix_len + 2 <= strlen (name));
-  gdb_assert (name[prefix_len + 1] == ':');
+  /* Check for malformed input.  */
+  if (prefix_len + 2 > strlen (name) || name[prefix_len + 1] != ':')
+    return null_block_symbol;
 
   /* Find the name of the class and the name of the method, variable, etc.  */
 
diff --git a/gdb/cp-support.c b/gdb/cp-support.c
index df127c4..a71c6ad 100644
--- a/gdb/cp-support.c
+++ b/gdb/cp-support.c
@@ -1037,8 +1037,13 @@ cp_find_first_component_aux (const char *name, int permissive)
 	      return strlen (name);
 	    }
 	case '\0':
-	case ':':
 	  return index;
+	case ':':
+	  /* ':' marks a component iff the next character is also a ':'.
+	     Otherwise it is probably malformed input.  */
+	  if (name[index + 1] == ':')
+	    return index;
+	  break;
 	case 'o':
 	  /* Operator names can screw up the recursion.  */
 	  if (operator_possible
diff --git a/gdb/testsuite/gdb.cp/scope-err.cc b/gdb/testsuite/gdb.cp/scope-err.cc
new file mode 100644
index 0000000..19c92d8
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/scope-err.cc
@@ -0,0 +1,35 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2016 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
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+int
+myfunction (int aa)
+{
+  int i;
+
+  i = aa + 42;
+  return i;    /* set breakpoint here */
+}
+
+int
+main (void)
+{
+  int a;
+
+  a = myfunction (a);
+
+  return a;
+}
diff --git a/gdb/testsuite/gdb.linespec/ls-errs.c b/gdb/testsuite/gdb.linespec/ls-errs.c
index ca41342..a3a43db 100644
--- a/gdb/testsuite/gdb.linespec/ls-errs.c
+++ b/gdb/testsuite/gdb.linespec/ls-errs.c
@@ -15,14 +15,21 @@
    You should have received a copy of the GNU General Public License
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
-int myfunction (void) { return 0; }
+int
+myfunction (int aa)
+{
+  int i;
+
+  i = aa + 42;
+  return i;    /* set breakpoint here */
+}
 
 int
 main (void)
-{  
+{
   int a;
 
-  a = myfunction ();
+  a = myfunction (a);
 
  here:
   return a;
diff --git a/gdb/testsuite/gdb.linespec/ls-errs.exp b/gdb/testsuite/gdb.linespec/ls-errs.exp
index 16d4574..35f8f78 100644
--- a/gdb/testsuite/gdb.linespec/ls-errs.exp
+++ b/gdb/testsuite/gdb.linespec/ls-errs.exp
@@ -13,209 +13,247 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
-# Tests for linespec error conditions
+# Tests for linespec errors with C and C++.
 
-standard_testfile
-set exefile $testfile
+# The test proper.  LANG is either C or C++.
 
-if {[prepare_for_testing $testfile $exefile $srcfile \
-	 {debug nowarnings}]} {
-    return -1
-}
+proc do_test {lang} {
+    global testfile srcfile error_messages compiler_info
 
-# Turn off the pending breakpoint queries.
-gdb_test_no_output "set breakpoint pending off"
+    standard_testfile
+    set exefile $testfile
+    if [info exists compiler_info] {
+	# Unsetting compiler_info allows us to switch compilers
+	# used by prepare_for_testing.
+        unset compiler_info
+    }
+    set options {debug}
 
-# Turn off completion limiting
-gdb_test_no_output "set max-completions unlimited"
+    if { $lang == "C++" } {
+	if { [skip_cplus_tests] } { return 0 }
+	# Build ".c" source file with g++.
+	lappend options "c++"
+    }
 
-# We intentionally do not use gdb_breakpoint for these tests.
+    if {[prepare_for_testing $testfile $exefile $srcfile $options]} {
+	return -1
+    }
 
-# 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
+    # Turn off the pending breakpoint queries.
+    gdb_test_no_output "set breakpoint pending off"
 
-    gdb_test "break $linespec" [string_to_regexp \
-				[eval format \$error_messages($msg_id) $args]]
-}
+    # Turn off completion limiting
+    gdb_test_no_output "set max-completions unlimited"
 
-# Common error message format strings.
-array set error_messages {
-    invalid_file "No source file named %s."
-    invalid_function "Function \"%s\" not defined."
-    invalid_var_or_func "Undefined convenience variable or function \"%s\" not defined."
-    invalid_function_f "Function \"%s\" not defined in \"%s\"."
-    invalid_var_or_func_f \
-	"Undefined convenience variable or function \"%s\" not defined in \"%s\"."
-    invalid_label "No label \"%s\" defined in function \"%s\"."
-    invalid_parm "invalid linespec argument, \"%s\""
-    invalid_offset "No line %d in the current file."
-    invalid_offset_f "No line %d in file \"%s\"."
-    malformed_line_offset "malformed line offset: \"%s\""
-    source_incomplete \
-	"Source filename requires function, label, or line offset."
-    unexpected "malformed linespec error: unexpected %s"
-    unexpected_opt "malformed linespec error: unexpected %s, \"%s\""
-    unmatched_quote "unmatched quote"
-    garbage "Garbage '%s' at end of command"
-}
+    if ![runto_main] {
+	fail "Can't run to main"
+	return 0
+    }
 
-# Some commonly used whitespace tests around ':'.
-set spaces [list ":" ": " " :" " : " "\t:  " "  :\t" "\t:\t" " \t:\t " \
-		"\t  \t:\t  \t  \t"]
+    # Run to a location in the file.
+    set bp_location [gdb_get_line_number "set breakpoint here"]
+
+    gdb_test "break $srcfile:$bp_location" \
+    "Breakpoint.*at.* file .*$srcfile, line $bp_location\\." \
+    "breakpoint line number in file"
+
+    gdb_continue_to_breakpoint "$bp_location"
+
+    # Common error message format strings.
+    array set error_messages {
+	invalid_file "No source file named %s."
+	invalid_function "Function \"%s\" not defined."
+	invalid_var_or_func 
+	    "Undefined convenience variable or function \"%s\" not defined."
+	invalid_function_f "Function \"%s\" not defined in \"%s\"."
+	invalid_var_or_func_f \
+	    "Undefined convenience variable or function \"%s\" not defined in \"%s\"."
+	invalid_label "No label \"%s\" defined in function \"%s\"."
+	invalid_parm "invalid linespec argument, \"%s\""
+	invalid_offset "No line %d in the current file."
+	invalid_offset_f "No line %d in file \"%s\"."
+	malformed_line_offset "malformed line offset: \"%s\""
+	source_incomplete \
+	    "Source filename requires function, label, or line offset."
+	unexpected "malformed linespec error: unexpected %s"
+	unexpected_opt "malformed linespec error: unexpected %s, \"%s\""
+	unmatched_quote "unmatched quote"
+	garbage "Garbage '%s' at end of command"
+    }
 
-# A list of invalid offsets.
-set invalid_offsets [list -100 +500 1000]
+    # We intentionally do not use gdb_breakpoint for these tests.
 
-# Try some simple, invalid linespecs involving spaces.
-foreach x $spaces {
-    test_break $x unexpected "colon"
-}
+    # 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
 
-# Test invalid filespecs starting with offset.  This is done
-# first so that default offsets are tested.
-foreach x $invalid_offsets {
-    set offset $x
-
-    # Relative offsets are relative to line 16.  Adjust
-    # expected offset from error message accordingly.
-    if {[string index $x 0] == "+" ||
-	[string index $x 0] == "-"} {
-	incr offset 16
+	gdb_test "break $linespec" [string_to_regexp \
+				    [eval format \$error_messages($msg_id) \
+				     $args]]
     }
-    test_break $x invalid_offset $offset
-    test_break "-line $x" invalid_offset $offset
-}
 
-# Test offsets with trailing tokens w/ and w/o spaces.
-foreach x $spaces {
-    test_break "3$x" unexpected "colon"
-    test_break "+10$x" unexpected "colon"
-    test_break "-10$x" unexpected "colon"
-}
+    # Some commonly used whitespace tests around ':'.
+    set spaces [list ":" ": " " :" " : " "\t:  " "  :\t" "\t:\t" \
+		     " \t:\t " "\t  \t:\t  \t  \t"]
 
-foreach x {1 +1 +100 -10} {
-    test_break "3 $x" unexpected_opt "number" $x
-    test_break "-line 3 $x" garbage $x
-    test_break "+10 $x" unexpected_opt "number" $x
-    test_break "-line +10 $x" garbage $x
-    test_break "-10 $x" unexpected_opt "number" $x
-    test_break "-line -10 $x" garbage $x
-}
+    # A list of invalid offsets.
+    set invalid_offsets [list -100 +500 1000]
 
-foreach x {3 +10 -10} {
-    test_break "$x foo" unexpected_opt "string" "foo"
-    test_break "-line $x foo" garbage "foo"
-}
+    # Try some simple, invalid linespecs involving spaces.
+    foreach x $spaces {
+	test_break $x unexpected "colon"
+    }
 
-# Test invalid linespecs starting with filename.
-foreach x [list "this_file_doesn't_exist.c" \
-	       "this file has spaces.c" \
-	       "\"file::colons.c\"" \
-	       "'file::colons.c'" \
-	       "\"this \"file\" has quotes.c\"" \
-	       "'this \"file\" has quotes.c'" \
-	       "'this 'file' has quotes.c'" \
-	       "\"this 'file' has quotes.c\"" \
-	       "\"spaces: and :colons.c\"" \
-	       "'more: :spaces: :and  colons::.c'"] {
-    # Remove any quoting from FILENAME for the error message.
-    test_break "$x:3" invalid_file [string trim $x \"']
-}
-foreach x [list "this_file_doesn't_exist.c" \
-	       "file::colons.c" \
-	       "'file::colons.c'"] {
-    test_break "-source $x -line 3" \
-	invalid_file [string trim $x \"']
-}
+    # Test invalid filespecs starting with offset.  This is done
+    # first so that default offsets are tested.
+    foreach x $invalid_offsets {
+	set offset $x
+
+	# Relative offsets are relative to line 16.  Adjust
+	# expected offset from error message accordingly.
+	if {[string index $x 0] == "+" || [string index $x 0] == "-"} {
+ 	    incr offset 24
+	}
+	test_break $x invalid_offset $offset
+	test_break "-line $x" invalid_offset $offset
+    }
+
+    # Test offsets with trailing tokens w/ and w/o spaces.
+    foreach x $spaces {
+	test_break "3$x" unexpected "colon"
+	test_break "+10$x" unexpected "colon"
+	test_break "-10$x" unexpected "colon"
+    }
+
+    foreach x {1 +1 +100 -10} {
+	test_break "3 $x" unexpected_opt "number" $x
+	test_break "-line 3 $x" garbage $x
+	test_break "+10 $x" unexpected_opt "number" $x
+	test_break "-line +10 $x" garbage $x
+	test_break "-10 $x" unexpected_opt "number" $x
+	test_break "-line -10 $x" garbage $x
+    }
 
-# Test that option lexing stops at whitespace boundaries
-test_break "-source this file has spaces.c -line 3" \
-    invalid_file "this"
+    foreach x {3 +10 -10} {
+	test_break "$x foo" unexpected_opt "string" "foo"
+	test_break "-line $x foo" garbage "foo"
+    }
 
-test_break "-function function whitespace" \
-    invalid_function "function"
+    # Test invalid linespecs starting with filename.
+    # It's OK to use the ".c" extension for the C++ test
+    # since the extension doesn't affect GDB's lookup.
+    set invalid_files [list "this_file_doesn't_exist.c" \
+			    "this file has spaces.c" \
+			    "\"file::colons.c\"" \
+			    "'file::colons.c'" \
+			    "\"this \"file\" has quotes.c\"" \
+			    "'this \"file\" has quotes.c'" \
+			    "'this 'file' has quotes.c'" \
+			    "\"this 'file' has quotes.c\"" \
+			    "\"spaces: and :colons.c\"" \
+			    "'more: :spaces: :and  colons::.c'" \
+			    "C:/nonexist-with-windrive.c"]
+
+    foreach x $invalid_files {
+	# Remove any quoting from FILENAME for the error message.
+	test_break "$x:3" invalid_file [string trim $x \"']
+    }
+    foreach x [list "this_file_doesn't_exist.c" \
+		    "file::colons.c" \
+		    "'file::colons.c'"] {
+	test_break "-source $x -line 3" invalid_file [string trim $x \"']
+    }
 
-test_break "-source $srcfile -function function whitespace" \
-    invalid_function_f "function" $srcfile
+    # Test that option lexing stops at whitespace boundaries
+    test_break "-source this file has spaces.c -line 3" invalid_file "this"
+    test_break "-function function whitespace" invalid_function "function"
+    test_break "-source $srcfile -function function whitespace" \
+	       invalid_function_f "function" $srcfile
 
-test_break "-function main -label label whitespace" \
-    invalid_label "label" "main"
+    test_break "-function main -label label whitespace" \
+	       invalid_label "label" "main"
 
-# Test unmatched quotes.
-foreach x {"\"src-file.c'" "'src-file.c"} {
-    test_break "$x:3" unmatched_quote
-}
+    # Test unmatched quotes.
+    foreach x {"\"src-file.c'" "'src-file.c"} {
+	test_break "$x:3" unmatched_quote
+    }
 
-test_break $srcfile invalid_function $srcfile
-foreach x {"foo" " foo" " foo "} {
-    # Trim any leading/trailing whitespace for error messages.
-    test_break "$srcfile:$x" invalid_function_f [string trim $x] $srcfile
-    test_break "-source $srcfile -function $x" \
-	invalid_function_f [string trim $x] $srcfile
-    test_break "$srcfile:main:$x" invalid_label [string trim $x] "main"
-    test_break "-source $srcfile -function main -label $x" \
-	invalid_label [string trim $x] "main"
-}
+    test_break $srcfile invalid_function $srcfile
+    foreach x {"foo" " foo" " foo "} {
+	# Trim any leading/trailing whitespace for error messages.
+	test_break "$srcfile:$x" invalid_function_f [string trim $x] $srcfile
+	test_break "-source $srcfile -function $x" \
+		   invalid_function_f [string trim $x] $srcfile
+	test_break "$srcfile:main:$x" invalid_label [string trim $x] "main"
+	test_break "-source $srcfile -function main -label $x" \
+		   invalid_label [string trim $x] "main"
+    }
 
-foreach x $spaces {
-    test_break "$srcfile$x" unexpected "end of input"
-    test_break "$srcfile:main$x" unexpected "end of input"
-}
+    foreach x $spaces {
+	test_break "$srcfile$x" unexpected "end of input"
+	test_break "$srcfile:main$x" unexpected "end of input"
+    }
 
-test_break "${srcfile}::" invalid_function "${srcfile}::"
-test_break "$srcfile:3 1" unexpected_opt "number" "1"
-test_break "-source $srcfile -line 3 1" garbage "1"
-test_break "$srcfile:3 +100" unexpected_opt "number" "+100"
-test_break "-source $srcfile -line 3 +100" garbage "+100"
-test_break "$srcfile:3 -100" unexpected_opt "number" "-100"
-test_break "$srcfile:3 foo" unexpected_opt "string" "foo"
-test_break "-source $srcfile -line 3 foo" garbage "foo"
-
-foreach x $invalid_offsets {
-    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_break "-source $srcfile -line $x" invalid_offset_f $x $srcfile
-}
-test_break "-source $srcfile -line -x" malformed_line_offset "-x"
+    test_break "${srcfile}::" invalid_function "${srcfile}::"
+    test_break "$srcfile:3 1" unexpected_opt "number" "1"
+    test_break "-source $srcfile -line 3 1" garbage "1"
+    test_break "$srcfile:3 +100" unexpected_opt "number" "+100"
+    test_break "-source $srcfile -line 3 +100" garbage "+100"
+    test_break "$srcfile:3 -100" unexpected_opt "number" "-100"
+    test_break "$srcfile:3 foo" unexpected_opt "string" "foo"
+    test_break "-source $srcfile -line 3 foo" garbage "foo"
+
+    foreach x $invalid_offsets {
+	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_break "-source $srcfile -line $x" invalid_offset_f $x $srcfile
+    }
+    test_break "-source $srcfile -line -x" malformed_line_offset "-x"
 
-# Test invalid filespecs starting with function.
-foreach x {"foobar" "foo::bar" "foo.bar" "foo ." "foo bar" "foo 1" \
+    # 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"} {
-    test_break $x invalid_function $x
-    test_break "-function \"$x\"" invalid_function $x
-}
+	test_break $x invalid_function $x
+	test_break "-function \"$x\"" invalid_function $x
+    }
 
-foreach x $spaces {
-    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"
-}
+    foreach x $spaces {
+	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"
+    }
 
-foreach x {"3" "+100" "-100" "foo"} {
-    test_break "main 3" invalid_function "main 3"
-    test_break "-function \"main $x\"" invalid_function "main $x"
-    test_break "main:here $x" invalid_label "here $x" "main"
-    test_break "-function main -label \"here $x\"" \
-	invalid_label "here $x" "main"
-}
+    foreach x {"3" "+100" "-100" "foo"} {
+	test_break "main 3" invalid_function "main 3"
+	test_break "-function \"main $x\"" invalid_function "main $x"
+	test_break "main:here $x" invalid_label "here $x" "main"
+	test_break "-function main -label \"here $x\"" \
+		   invalid_label "here $x" "main"
+    }
 
-foreach x {"if" "task" "thread"} {
-    test_break $x invalid_function $x
-}
+    foreach x {"if" "task" "thread"} {
+	test_break $x invalid_function $x
+    }
 
-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_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}
-test_break $x invalid_var_or_func $x
-test_break "$srcfile:$x" invalid_var_or_func_f $x $srcfile
+    # Test undefined convenience variables.
+    set x {$zippo}
+    test_break $x invalid_var_or_func $x
+    test_break "$srcfile:$x" invalid_var_or_func_f $x $srcfile
 
-# Explicit linespec-specific tests
-test_break "-source $srcfile" source_incomplete
+    # Explicit linespec-specific tests
+    test_break "-source $srcfile" source_incomplete
+}
+
+foreach_with_prefix lang {"C" "C++"} {
+    do_test ${lang}
+}
diff --git a/gdb/testsuite/gdb.mi/mi-linespec-err-cp.cc b/gdb/testsuite/gdb.mi/mi-linespec-err-cp.cc
new file mode 100644
index 0000000..19c92d8
--- /dev/null
+++ b/gdb/testsuite/gdb.mi/mi-linespec-err-cp.cc
@@ -0,0 +1,35 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2016 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
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+int
+myfunction (int aa)
+{
+  int i;
+
+  i = aa + 42;
+  return i;    /* set breakpoint here */
+}
+
+int
+main (void)
+{
+  int a;
+
+  a = myfunction (a);
+
+  return a;
+}
diff --git a/gdb/testsuite/gdb.mi/mi-linespec-err-cp.exp b/gdb/testsuite/gdb.mi/mi-linespec-err-cp.exp
new file mode 100644
index 0000000..1a8682e
--- /dev/null
+++ b/gdb/testsuite/gdb.mi/mi-linespec-err-cp.exp
@@ -0,0 +1,57 @@
+# Copyright 2016 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
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Regression test for PR breakpoints/18303.  Tests that the correct
+# errors is generated when setting a breakpoint in a non-existent
+# file with a Windows-style logical drive names and C++.
+
+if { [skip_cplus_tests] } { continue }
+
+load_lib mi-support.exp
+set MIFLAGS "-i=mi"
+
+standard_testfile .cc
+set exefile $testfile
+
+if {[prepare_for_testing $testfile $exefile $srcfile {debug c++}]} {
+    return -1
+}
+
+gdb_exit
+if [mi_gdb_start] {
+    continue
+}
+
+# Turn off the pending breakpoint queries.
+mi_gdb_test "-interpreter-exec console \"set breakpoint pending off\"" \
+  {=cmd-param-changed,param=\"breakpoint pending\",.*\^done} \
+  "-interpreter-exec console \"set breakpoint pending off\""
+
+mi_run_to_main
+
+# Run to a location in the file.
+set bp_location [gdb_get_line_number "set breakpoint here"]
+
+mi_gdb_test "-break-insert ${srcfile}:${bp_location}" \
+    {\^done,bkpt=.number="2",type="breakpoint".*\}} "set breakpoint"
+
+mi_execute_to "exec-continue" "breakpoint-hit" "myfunction" ".*" ".*" "24" \
+    { "" "disp=\"keep\"" } "breakpoint hit"
+
+# Set a breakpoint in a C++ source file whose name contains a
+# Windows-style logical drive.
+mi_gdb_test \
+    "-break-insert -f \"c:/uu.cpp:13\"" \
+    ".*No source file named c:/uu.cpp.*"
-- 
1.8.1.1

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

* [PATCH v4] PR 18303, Tolerate malformed input for lookup_symbol-called functions.
  2016-01-28 22:43           ` [PATCH v3] " Don Breazeal
@ 2016-01-28 22:52             ` Don Breazeal
  2016-02-04 18:37               ` [PING] " Don Breazeal
  0 siblings, 1 reply; 17+ messages in thread
From: Don Breazeal @ 2016-01-28 22:52 UTC (permalink / raw)
  To: gdb-patches

Apologies -- the previous version of this patch was missing one file,
added here.  Sorry for the noise.
--Don

---V3 Comment---
This patch differs from v2 in that the new test gdb.linespec/ls-errs-cp.exp
is gone, and instead (per Pedro's suggestion) gdb.linespec/ls-errs.exp has
been modified to test both C & C++, and to incorporate the small amount of
extra testing from ls-errs-cp.exp.

It also includes, unchanged from the previous version of the patch:

 * an updated version of the alternate fix that Keith proposed for my
   patch that addressed PR breakpoints/18303.  Doug has approved (in
   concept, at least) the code portion of the patch.

 * a couple of other new tests of colons in linespecs.

Thanks,
--Don

-----------
lookup_symbol is often called with user input.  Consequently, any
function called from lookup_symbol{,_in_language} should attempt to
deal with malformed input gracefully.  After all, malformed user
input is not a programming/API error.

This patch does not attempt to find/correct all instances of this.  It
only fixes locations in the code that trigger test suite failures.

This patch fixes PR breakpoints/18303, "Assertion: -breakpoint-insert
with windows paths of file in non-current directory".

The patch includes three new tests related to this.  One is just
gdb.linespec/ls-errs.exp copied and converted to use C++ instead of C, and
to add a case using a file name containing a Windows-style logical drive
specifier.  The others include an MI test to provide a regression test for
the specific case reported in PR 18303, and a C++ test for proper error
handling of access to a program variable when using a file scope specifier
that refers to a non-existent file.

Tested on x86_64 native Linux.

gdb/ChangeLog
2016-01-28  Keith Seitz  <keiths@redhat.com>

	PR breakpoints/18303
	* cp-namespace.c (cp_lookup_bare_symbol): Change assertion to
	look for "::" instead of simply ":".
	(cp_search_static_and_baseclasses): Return null_block_symbol for
	malformed input.
	Remove assertions.
	* cp-support.c (cp_find_first_component_aux): Do not return
	a prefix length for ':' unless the next character is also ':'.

gdb/testsuite/ChangeLog
2016-01-28  Don Breazeal  <donb@codesourcery.com>

	* gdb.cp/scope-err.cc: New test program.
	* gdb.cp/scope-err.exp: New test script.
	* gdb.linespec/ls-errs.c (myfunction): Expanded to have multiple
	lines and "set breakpoint here" comment.
	* gdb.linespec/ls-errs.exp: Added C++ testing and new test case.
	* gdb.mi/mi-linespec-err-cp.cc: New test program.
	* gdb.mi/mi-linespec-err-cp.exp: New test script.

---
 gdb/cp-namespace.c                          |   9 +-
 gdb/cp-support.c                            |   7 +-
 gdb/testsuite/gdb.cp/scope-err.cc           |  35 +++
 gdb/testsuite/gdb.cp/scope-err.exp          |  47 ++++
 gdb/testsuite/gdb.linespec/ls-errs.c        |  13 +-
 gdb/testsuite/gdb.linespec/ls-errs.exp      | 384 +++++++++++++++-------------
 gdb/testsuite/gdb.mi/mi-linespec-err-cp.cc  |  35 +++
 gdb/testsuite/gdb.mi/mi-linespec-err-cp.exp |  57 +++++
 8 files changed, 405 insertions(+), 182 deletions(-)

diff --git a/gdb/cp-namespace.c b/gdb/cp-namespace.c
index 72002d6..016a42f 100644
--- a/gdb/cp-namespace.c
+++ b/gdb/cp-namespace.c
@@ -170,7 +170,7 @@ cp_lookup_bare_symbol (const struct language_defn *langdef,
      ':' may be in the args of a template spec.  This isn't intended to be
      a complete test, just cheap and documentary.  */
   if (strchr (name, '<') == NULL && strchr (name, '(') == NULL)
-    gdb_assert (strchr (name, ':') == NULL);
+    gdb_assert (strstr (name, "::") == NULL);
 
   sym = lookup_symbol_in_static_block (name, block, domain);
   if (sym.symbol != NULL)
@@ -246,10 +246,9 @@ cp_search_static_and_baseclasses (const char *name,
   struct block_symbol klass_sym;
   struct type *klass_type;
 
-  /* The test here uses <= instead of < because Fortran also uses this,
-     and the module.exp testcase will pass "modmany::" for NAME here.  */
-  gdb_assert (prefix_len + 2 <= strlen (name));
-  gdb_assert (name[prefix_len + 1] == ':');
+  /* Check for malformed input.  */
+  if (prefix_len + 2 > strlen (name) || name[prefix_len + 1] != ':')
+    return null_block_symbol;
 
   /* Find the name of the class and the name of the method, variable, etc.  */
 
diff --git a/gdb/cp-support.c b/gdb/cp-support.c
index df127c4..a71c6ad 100644
--- a/gdb/cp-support.c
+++ b/gdb/cp-support.c
@@ -1037,8 +1037,13 @@ cp_find_first_component_aux (const char *name, int permissive)
 	      return strlen (name);
 	    }
 	case '\0':
-	case ':':
 	  return index;
+	case ':':
+	  /* ':' marks a component iff the next character is also a ':'.
+	     Otherwise it is probably malformed input.  */
+	  if (name[index + 1] == ':')
+	    return index;
+	  break;
 	case 'o':
 	  /* Operator names can screw up the recursion.  */
 	  if (operator_possible
diff --git a/gdb/testsuite/gdb.cp/scope-err.cc b/gdb/testsuite/gdb.cp/scope-err.cc
new file mode 100644
index 0000000..19c92d8
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/scope-err.cc
@@ -0,0 +1,35 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2016 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
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+int
+myfunction (int aa)
+{
+  int i;
+
+  i = aa + 42;
+  return i;    /* set breakpoint here */
+}
+
+int
+main (void)
+{
+  int a;
+
+  a = myfunction (a);
+
+  return a;
+}
diff --git a/gdb/testsuite/gdb.cp/scope-err.exp b/gdb/testsuite/gdb.cp/scope-err.exp
new file mode 100644
index 0000000..9d93578
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/scope-err.exp
@@ -0,0 +1,47 @@
+# Copyright 2012-2016 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
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Tests for linespec errors with C++.
+# Derived from gdb.linespec/ls-errs.exp.
+
+if { [skip_cplus_tests] } { continue }
+
+standard_testfile .cc
+set exefile $testfile
+
+if {[prepare_for_testing $testfile $exefile $srcfile {debug c++}]} {
+    return -1
+}
+
+if ![runto_main] {
+    fail "Can't run to main"
+    return 0
+}
+
+# Run to a location in the file.
+set bp_location [gdb_get_line_number "set breakpoint here"]
+
+gdb_test "break $srcfile:$bp_location" \
+    "Breakpoint.*at.* file .*$srcfile, line $bp_location\\." \
+    "breakpoint line number in file"
+
+gdb_continue_to_breakpoint "$bp_location"
+
+# Try to access a variable using scope that is a non-existent filename
+# with a Windows-style logical drive in the name.
+set nonexistent_file C:/does/not/exist.cc
+gdb_test "print '$nonexistent_file'::var" \
+	 ".*No symbol \"$nonexistent_file\" in current context.*" \
+	 "print var from \"$nonexistent_file\""
diff --git a/gdb/testsuite/gdb.linespec/ls-errs.c b/gdb/testsuite/gdb.linespec/ls-errs.c
index ca41342..a3a43db 100644
--- a/gdb/testsuite/gdb.linespec/ls-errs.c
+++ b/gdb/testsuite/gdb.linespec/ls-errs.c
@@ -15,14 +15,21 @@
    You should have received a copy of the GNU General Public License
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
-int myfunction (void) { return 0; }
+int
+myfunction (int aa)
+{
+  int i;
+
+  i = aa + 42;
+  return i;    /* set breakpoint here */
+}
 
 int
 main (void)
-{  
+{
   int a;
 
-  a = myfunction ();
+  a = myfunction (a);
 
  here:
   return a;
diff --git a/gdb/testsuite/gdb.linespec/ls-errs.exp b/gdb/testsuite/gdb.linespec/ls-errs.exp
index 16d4574..35f8f78 100644
--- a/gdb/testsuite/gdb.linespec/ls-errs.exp
+++ b/gdb/testsuite/gdb.linespec/ls-errs.exp
@@ -13,209 +13,247 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
-# Tests for linespec error conditions
+# Tests for linespec errors with C and C++.
 
-standard_testfile
-set exefile $testfile
+# The test proper.  LANG is either C or C++.
 
-if {[prepare_for_testing $testfile $exefile $srcfile \
-	 {debug nowarnings}]} {
-    return -1
-}
+proc do_test {lang} {
+    global testfile srcfile error_messages compiler_info
 
-# Turn off the pending breakpoint queries.
-gdb_test_no_output "set breakpoint pending off"
+    standard_testfile
+    set exefile $testfile
+    if [info exists compiler_info] {
+	# Unsetting compiler_info allows us to switch compilers
+	# used by prepare_for_testing.
+        unset compiler_info
+    }
+    set options {debug}
 
-# Turn off completion limiting
-gdb_test_no_output "set max-completions unlimited"
+    if { $lang == "C++" } {
+	if { [skip_cplus_tests] } { return 0 }
+	# Build ".c" source file with g++.
+	lappend options "c++"
+    }
 
-# We intentionally do not use gdb_breakpoint for these tests.
+    if {[prepare_for_testing $testfile $exefile $srcfile $options]} {
+	return -1
+    }
 
-# 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
+    # Turn off the pending breakpoint queries.
+    gdb_test_no_output "set breakpoint pending off"
 
-    gdb_test "break $linespec" [string_to_regexp \
-				[eval format \$error_messages($msg_id) $args]]
-}
+    # Turn off completion limiting
+    gdb_test_no_output "set max-completions unlimited"
 
-# Common error message format strings.
-array set error_messages {
-    invalid_file "No source file named %s."
-    invalid_function "Function \"%s\" not defined."
-    invalid_var_or_func "Undefined convenience variable or function \"%s\" not defined."
-    invalid_function_f "Function \"%s\" not defined in \"%s\"."
-    invalid_var_or_func_f \
-	"Undefined convenience variable or function \"%s\" not defined in \"%s\"."
-    invalid_label "No label \"%s\" defined in function \"%s\"."
-    invalid_parm "invalid linespec argument, \"%s\""
-    invalid_offset "No line %d in the current file."
-    invalid_offset_f "No line %d in file \"%s\"."
-    malformed_line_offset "malformed line offset: \"%s\""
-    source_incomplete \
-	"Source filename requires function, label, or line offset."
-    unexpected "malformed linespec error: unexpected %s"
-    unexpected_opt "malformed linespec error: unexpected %s, \"%s\""
-    unmatched_quote "unmatched quote"
-    garbage "Garbage '%s' at end of command"
-}
+    if ![runto_main] {
+	fail "Can't run to main"
+	return 0
+    }
 
-# Some commonly used whitespace tests around ':'.
-set spaces [list ":" ": " " :" " : " "\t:  " "  :\t" "\t:\t" " \t:\t " \
-		"\t  \t:\t  \t  \t"]
+    # Run to a location in the file.
+    set bp_location [gdb_get_line_number "set breakpoint here"]
+
+    gdb_test "break $srcfile:$bp_location" \
+    "Breakpoint.*at.* file .*$srcfile, line $bp_location\\." \
+    "breakpoint line number in file"
+
+    gdb_continue_to_breakpoint "$bp_location"
+
+    # Common error message format strings.
+    array set error_messages {
+	invalid_file "No source file named %s."
+	invalid_function "Function \"%s\" not defined."
+	invalid_var_or_func 
+	    "Undefined convenience variable or function \"%s\" not defined."
+	invalid_function_f "Function \"%s\" not defined in \"%s\"."
+	invalid_var_or_func_f \
+	    "Undefined convenience variable or function \"%s\" not defined in \"%s\"."
+	invalid_label "No label \"%s\" defined in function \"%s\"."
+	invalid_parm "invalid linespec argument, \"%s\""
+	invalid_offset "No line %d in the current file."
+	invalid_offset_f "No line %d in file \"%s\"."
+	malformed_line_offset "malformed line offset: \"%s\""
+	source_incomplete \
+	    "Source filename requires function, label, or line offset."
+	unexpected "malformed linespec error: unexpected %s"
+	unexpected_opt "malformed linespec error: unexpected %s, \"%s\""
+	unmatched_quote "unmatched quote"
+	garbage "Garbage '%s' at end of command"
+    }
 
-# A list of invalid offsets.
-set invalid_offsets [list -100 +500 1000]
+    # We intentionally do not use gdb_breakpoint for these tests.
 
-# Try some simple, invalid linespecs involving spaces.
-foreach x $spaces {
-    test_break $x unexpected "colon"
-}
+    # 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
 
-# Test invalid filespecs starting with offset.  This is done
-# first so that default offsets are tested.
-foreach x $invalid_offsets {
-    set offset $x
-
-    # Relative offsets are relative to line 16.  Adjust
-    # expected offset from error message accordingly.
-    if {[string index $x 0] == "+" ||
-	[string index $x 0] == "-"} {
-	incr offset 16
+	gdb_test "break $linespec" [string_to_regexp \
+				    [eval format \$error_messages($msg_id) \
+				     $args]]
     }
-    test_break $x invalid_offset $offset
-    test_break "-line $x" invalid_offset $offset
-}
 
-# Test offsets with trailing tokens w/ and w/o spaces.
-foreach x $spaces {
-    test_break "3$x" unexpected "colon"
-    test_break "+10$x" unexpected "colon"
-    test_break "-10$x" unexpected "colon"
-}
+    # Some commonly used whitespace tests around ':'.
+    set spaces [list ":" ": " " :" " : " "\t:  " "  :\t" "\t:\t" \
+		     " \t:\t " "\t  \t:\t  \t  \t"]
 
-foreach x {1 +1 +100 -10} {
-    test_break "3 $x" unexpected_opt "number" $x
-    test_break "-line 3 $x" garbage $x
-    test_break "+10 $x" unexpected_opt "number" $x
-    test_break "-line +10 $x" garbage $x
-    test_break "-10 $x" unexpected_opt "number" $x
-    test_break "-line -10 $x" garbage $x
-}
+    # A list of invalid offsets.
+    set invalid_offsets [list -100 +500 1000]
 
-foreach x {3 +10 -10} {
-    test_break "$x foo" unexpected_opt "string" "foo"
-    test_break "-line $x foo" garbage "foo"
-}
+    # Try some simple, invalid linespecs involving spaces.
+    foreach x $spaces {
+	test_break $x unexpected "colon"
+    }
 
-# Test invalid linespecs starting with filename.
-foreach x [list "this_file_doesn't_exist.c" \
-	       "this file has spaces.c" \
-	       "\"file::colons.c\"" \
-	       "'file::colons.c'" \
-	       "\"this \"file\" has quotes.c\"" \
-	       "'this \"file\" has quotes.c'" \
-	       "'this 'file' has quotes.c'" \
-	       "\"this 'file' has quotes.c\"" \
-	       "\"spaces: and :colons.c\"" \
-	       "'more: :spaces: :and  colons::.c'"] {
-    # Remove any quoting from FILENAME for the error message.
-    test_break "$x:3" invalid_file [string trim $x \"']
-}
-foreach x [list "this_file_doesn't_exist.c" \
-	       "file::colons.c" \
-	       "'file::colons.c'"] {
-    test_break "-source $x -line 3" \
-	invalid_file [string trim $x \"']
-}
+    # Test invalid filespecs starting with offset.  This is done
+    # first so that default offsets are tested.
+    foreach x $invalid_offsets {
+	set offset $x
+
+	# Relative offsets are relative to line 16.  Adjust
+	# expected offset from error message accordingly.
+	if {[string index $x 0] == "+" || [string index $x 0] == "-"} {
+ 	    incr offset 24
+	}
+	test_break $x invalid_offset $offset
+	test_break "-line $x" invalid_offset $offset
+    }
+
+    # Test offsets with trailing tokens w/ and w/o spaces.
+    foreach x $spaces {
+	test_break "3$x" unexpected "colon"
+	test_break "+10$x" unexpected "colon"
+	test_break "-10$x" unexpected "colon"
+    }
+
+    foreach x {1 +1 +100 -10} {
+	test_break "3 $x" unexpected_opt "number" $x
+	test_break "-line 3 $x" garbage $x
+	test_break "+10 $x" unexpected_opt "number" $x
+	test_break "-line +10 $x" garbage $x
+	test_break "-10 $x" unexpected_opt "number" $x
+	test_break "-line -10 $x" garbage $x
+    }
 
-# Test that option lexing stops at whitespace boundaries
-test_break "-source this file has spaces.c -line 3" \
-    invalid_file "this"
+    foreach x {3 +10 -10} {
+	test_break "$x foo" unexpected_opt "string" "foo"
+	test_break "-line $x foo" garbage "foo"
+    }
 
-test_break "-function function whitespace" \
-    invalid_function "function"
+    # Test invalid linespecs starting with filename.
+    # It's OK to use the ".c" extension for the C++ test
+    # since the extension doesn't affect GDB's lookup.
+    set invalid_files [list "this_file_doesn't_exist.c" \
+			    "this file has spaces.c" \
+			    "\"file::colons.c\"" \
+			    "'file::colons.c'" \
+			    "\"this \"file\" has quotes.c\"" \
+			    "'this \"file\" has quotes.c'" \
+			    "'this 'file' has quotes.c'" \
+			    "\"this 'file' has quotes.c\"" \
+			    "\"spaces: and :colons.c\"" \
+			    "'more: :spaces: :and  colons::.c'" \
+			    "C:/nonexist-with-windrive.c"]
+
+    foreach x $invalid_files {
+	# Remove any quoting from FILENAME for the error message.
+	test_break "$x:3" invalid_file [string trim $x \"']
+    }
+    foreach x [list "this_file_doesn't_exist.c" \
+		    "file::colons.c" \
+		    "'file::colons.c'"] {
+	test_break "-source $x -line 3" invalid_file [string trim $x \"']
+    }
 
-test_break "-source $srcfile -function function whitespace" \
-    invalid_function_f "function" $srcfile
+    # Test that option lexing stops at whitespace boundaries
+    test_break "-source this file has spaces.c -line 3" invalid_file "this"
+    test_break "-function function whitespace" invalid_function "function"
+    test_break "-source $srcfile -function function whitespace" \
+	       invalid_function_f "function" $srcfile
 
-test_break "-function main -label label whitespace" \
-    invalid_label "label" "main"
+    test_break "-function main -label label whitespace" \
+	       invalid_label "label" "main"
 
-# Test unmatched quotes.
-foreach x {"\"src-file.c'" "'src-file.c"} {
-    test_break "$x:3" unmatched_quote
-}
+    # Test unmatched quotes.
+    foreach x {"\"src-file.c'" "'src-file.c"} {
+	test_break "$x:3" unmatched_quote
+    }
 
-test_break $srcfile invalid_function $srcfile
-foreach x {"foo" " foo" " foo "} {
-    # Trim any leading/trailing whitespace for error messages.
-    test_break "$srcfile:$x" invalid_function_f [string trim $x] $srcfile
-    test_break "-source $srcfile -function $x" \
-	invalid_function_f [string trim $x] $srcfile
-    test_break "$srcfile:main:$x" invalid_label [string trim $x] "main"
-    test_break "-source $srcfile -function main -label $x" \
-	invalid_label [string trim $x] "main"
-}
+    test_break $srcfile invalid_function $srcfile
+    foreach x {"foo" " foo" " foo "} {
+	# Trim any leading/trailing whitespace for error messages.
+	test_break "$srcfile:$x" invalid_function_f [string trim $x] $srcfile
+	test_break "-source $srcfile -function $x" \
+		   invalid_function_f [string trim $x] $srcfile
+	test_break "$srcfile:main:$x" invalid_label [string trim $x] "main"
+	test_break "-source $srcfile -function main -label $x" \
+		   invalid_label [string trim $x] "main"
+    }
 
-foreach x $spaces {
-    test_break "$srcfile$x" unexpected "end of input"
-    test_break "$srcfile:main$x" unexpected "end of input"
-}
+    foreach x $spaces {
+	test_break "$srcfile$x" unexpected "end of input"
+	test_break "$srcfile:main$x" unexpected "end of input"
+    }
 
-test_break "${srcfile}::" invalid_function "${srcfile}::"
-test_break "$srcfile:3 1" unexpected_opt "number" "1"
-test_break "-source $srcfile -line 3 1" garbage "1"
-test_break "$srcfile:3 +100" unexpected_opt "number" "+100"
-test_break "-source $srcfile -line 3 +100" garbage "+100"
-test_break "$srcfile:3 -100" unexpected_opt "number" "-100"
-test_break "$srcfile:3 foo" unexpected_opt "string" "foo"
-test_break "-source $srcfile -line 3 foo" garbage "foo"
-
-foreach x $invalid_offsets {
-    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_break "-source $srcfile -line $x" invalid_offset_f $x $srcfile
-}
-test_break "-source $srcfile -line -x" malformed_line_offset "-x"
+    test_break "${srcfile}::" invalid_function "${srcfile}::"
+    test_break "$srcfile:3 1" unexpected_opt "number" "1"
+    test_break "-source $srcfile -line 3 1" garbage "1"
+    test_break "$srcfile:3 +100" unexpected_opt "number" "+100"
+    test_break "-source $srcfile -line 3 +100" garbage "+100"
+    test_break "$srcfile:3 -100" unexpected_opt "number" "-100"
+    test_break "$srcfile:3 foo" unexpected_opt "string" "foo"
+    test_break "-source $srcfile -line 3 foo" garbage "foo"
+
+    foreach x $invalid_offsets {
+	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_break "-source $srcfile -line $x" invalid_offset_f $x $srcfile
+    }
+    test_break "-source $srcfile -line -x" malformed_line_offset "-x"
 
-# Test invalid filespecs starting with function.
-foreach x {"foobar" "foo::bar" "foo.bar" "foo ." "foo bar" "foo 1" \
+    # 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"} {
-    test_break $x invalid_function $x
-    test_break "-function \"$x\"" invalid_function $x
-}
+	test_break $x invalid_function $x
+	test_break "-function \"$x\"" invalid_function $x
+    }
 
-foreach x $spaces {
-    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"
-}
+    foreach x $spaces {
+	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"
+    }
 
-foreach x {"3" "+100" "-100" "foo"} {
-    test_break "main 3" invalid_function "main 3"
-    test_break "-function \"main $x\"" invalid_function "main $x"
-    test_break "main:here $x" invalid_label "here $x" "main"
-    test_break "-function main -label \"here $x\"" \
-	invalid_label "here $x" "main"
-}
+    foreach x {"3" "+100" "-100" "foo"} {
+	test_break "main 3" invalid_function "main 3"
+	test_break "-function \"main $x\"" invalid_function "main $x"
+	test_break "main:here $x" invalid_label "here $x" "main"
+	test_break "-function main -label \"here $x\"" \
+		   invalid_label "here $x" "main"
+    }
 
-foreach x {"if" "task" "thread"} {
-    test_break $x invalid_function $x
-}
+    foreach x {"if" "task" "thread"} {
+	test_break $x invalid_function $x
+    }
 
-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_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}
-test_break $x invalid_var_or_func $x
-test_break "$srcfile:$x" invalid_var_or_func_f $x $srcfile
+    # Test undefined convenience variables.
+    set x {$zippo}
+    test_break $x invalid_var_or_func $x
+    test_break "$srcfile:$x" invalid_var_or_func_f $x $srcfile
 
-# Explicit linespec-specific tests
-test_break "-source $srcfile" source_incomplete
+    # Explicit linespec-specific tests
+    test_break "-source $srcfile" source_incomplete
+}
+
+foreach_with_prefix lang {"C" "C++"} {
+    do_test ${lang}
+}
diff --git a/gdb/testsuite/gdb.mi/mi-linespec-err-cp.cc b/gdb/testsuite/gdb.mi/mi-linespec-err-cp.cc
new file mode 100644
index 0000000..19c92d8
--- /dev/null
+++ b/gdb/testsuite/gdb.mi/mi-linespec-err-cp.cc
@@ -0,0 +1,35 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2016 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
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+int
+myfunction (int aa)
+{
+  int i;
+
+  i = aa + 42;
+  return i;    /* set breakpoint here */
+}
+
+int
+main (void)
+{
+  int a;
+
+  a = myfunction (a);
+
+  return a;
+}
diff --git a/gdb/testsuite/gdb.mi/mi-linespec-err-cp.exp b/gdb/testsuite/gdb.mi/mi-linespec-err-cp.exp
new file mode 100644
index 0000000..1a8682e
--- /dev/null
+++ b/gdb/testsuite/gdb.mi/mi-linespec-err-cp.exp
@@ -0,0 +1,57 @@
+# Copyright 2016 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
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Regression test for PR breakpoints/18303.  Tests that the correct
+# errors is generated when setting a breakpoint in a non-existent
+# file with a Windows-style logical drive names and C++.
+
+if { [skip_cplus_tests] } { continue }
+
+load_lib mi-support.exp
+set MIFLAGS "-i=mi"
+
+standard_testfile .cc
+set exefile $testfile
+
+if {[prepare_for_testing $testfile $exefile $srcfile {debug c++}]} {
+    return -1
+}
+
+gdb_exit
+if [mi_gdb_start] {
+    continue
+}
+
+# Turn off the pending breakpoint queries.
+mi_gdb_test "-interpreter-exec console \"set breakpoint pending off\"" \
+  {=cmd-param-changed,param=\"breakpoint pending\",.*\^done} \
+  "-interpreter-exec console \"set breakpoint pending off\""
+
+mi_run_to_main
+
+# Run to a location in the file.
+set bp_location [gdb_get_line_number "set breakpoint here"]
+
+mi_gdb_test "-break-insert ${srcfile}:${bp_location}" \
+    {\^done,bkpt=.number="2",type="breakpoint".*\}} "set breakpoint"
+
+mi_execute_to "exec-continue" "breakpoint-hit" "myfunction" ".*" ".*" "24" \
+    { "" "disp=\"keep\"" } "breakpoint hit"
+
+# Set a breakpoint in a C++ source file whose name contains a
+# Windows-style logical drive.
+mi_gdb_test \
+    "-break-insert -f \"c:/uu.cpp:13\"" \
+    ".*No source file named c:/uu.cpp.*"
-- 
1.8.1.1

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

* [PING] Re: [PATCH v4] PR 18303, Tolerate malformed input for lookup_symbol-called functions.
  2016-01-28 22:52             ` [PATCH v4] " Don Breazeal
@ 2016-02-04 18:37               ` Don Breazeal
  2016-02-18 18:22                 ` [PING] Re: [PATCH v4] PR 18303, Tolerate malformed input for lookup_symbol-called functions (was: [PATCH] Fix problem handling colon in linespec, PR breakpoints/18303) Don Breazeal
  0 siblings, 1 reply; 17+ messages in thread
From: Don Breazeal @ 2016-02-04 18:37 UTC (permalink / raw)
  To: gdb-patches

On 1/28/2016 2:52 PM, Don Breazeal wrote:
> Apologies -- the previous version of this patch was missing one file,
> added here.  Sorry for the noise.
> --Don
> 
> ---V3 Comment---
> This patch differs from v2 in that the new test gdb.linespec/ls-errs-cp.exp
> is gone, and instead (per Pedro's suggestion) gdb.linespec/ls-errs.exp has
> been modified to test both C & C++, and to incorporate the small amount of
> extra testing from ls-errs-cp.exp.
> 
> It also includes, unchanged from the previous version of the patch:
> 
>  * an updated version of the alternate fix that Keith proposed for my
>    patch that addressed PR breakpoints/18303.  Doug has approved (in
>    concept, at least) the code portion of the patch.
> 
>  * a couple of other new tests of colons in linespecs.
> 
> Thanks,
> --Don
> 
> -----------
> lookup_symbol is often called with user input.  Consequently, any
> function called from lookup_symbol{,_in_language} should attempt to
> deal with malformed input gracefully.  After all, malformed user
> input is not a programming/API error.
> 
> This patch does not attempt to find/correct all instances of this.  It
> only fixes locations in the code that trigger test suite failures.
> 
> This patch fixes PR breakpoints/18303, "Assertion: -breakpoint-insert
> with windows paths of file in non-current directory".
> 
> The patch includes three new tests related to this.  One is just
> gdb.linespec/ls-errs.exp copied and converted to use C++ instead of C, and
> to add a case using a file name containing a Windows-style logical drive
> specifier.  The others include an MI test to provide a regression test for
> the specific case reported in PR 18303, and a C++ test for proper error
> handling of access to a program variable when using a file scope specifier
> that refers to a non-existent file.
> 
> Tested on x86_64 native Linux.
> 
> gdb/ChangeLog
> 2016-01-28  Keith Seitz  <keiths@redhat.com>
> 
> 	PR breakpoints/18303
> 	* cp-namespace.c (cp_lookup_bare_symbol): Change assertion to
> 	look for "::" instead of simply ":".
> 	(cp_search_static_and_baseclasses): Return null_block_symbol for
> 	malformed input.
> 	Remove assertions.
> 	* cp-support.c (cp_find_first_component_aux): Do not return
> 	a prefix length for ':' unless the next character is also ':'.
> 
> gdb/testsuite/ChangeLog
> 2016-01-28  Don Breazeal  <donb@codesourcery.com>
> 
> 	* gdb.cp/scope-err.cc: New test program.
> 	* gdb.cp/scope-err.exp: New test script.
> 	* gdb.linespec/ls-errs.c (myfunction): Expanded to have multiple
> 	lines and "set breakpoint here" comment.
> 	* gdb.linespec/ls-errs.exp: Added C++ testing and new test case.
> 	* gdb.mi/mi-linespec-err-cp.cc: New test program.
> 	* gdb.mi/mi-linespec-err-cp.exp: New test script.
> 
> ---
>  gdb/cp-namespace.c                          |   9 +-
>  gdb/cp-support.c                            |   7 +-
>  gdb/testsuite/gdb.cp/scope-err.cc           |  35 +++
>  gdb/testsuite/gdb.cp/scope-err.exp          |  47 ++++
>  gdb/testsuite/gdb.linespec/ls-errs.c        |  13 +-
>  gdb/testsuite/gdb.linespec/ls-errs.exp      | 384 +++++++++++++++-------------
>  gdb/testsuite/gdb.mi/mi-linespec-err-cp.cc  |  35 +++
>  gdb/testsuite/gdb.mi/mi-linespec-err-cp.exp |  57 +++++
>  8 files changed, 405 insertions(+), 182 deletions(-)
> 
> diff --git a/gdb/cp-namespace.c b/gdb/cp-namespace.c
> index 72002d6..016a42f 100644
> --- a/gdb/cp-namespace.c
> +++ b/gdb/cp-namespace.c
> @@ -170,7 +170,7 @@ cp_lookup_bare_symbol (const struct language_defn *langdef,
>       ':' may be in the args of a template spec.  This isn't intended to be
>       a complete test, just cheap and documentary.  */
>    if (strchr (name, '<') == NULL && strchr (name, '(') == NULL)
> -    gdb_assert (strchr (name, ':') == NULL);
> +    gdb_assert (strstr (name, "::") == NULL);
>  
>    sym = lookup_symbol_in_static_block (name, block, domain);
>    if (sym.symbol != NULL)
> @@ -246,10 +246,9 @@ cp_search_static_and_baseclasses (const char *name,
>    struct block_symbol klass_sym;
>    struct type *klass_type;
>  
> -  /* The test here uses <= instead of < because Fortran also uses this,
> -     and the module.exp testcase will pass "modmany::" for NAME here.  */
> -  gdb_assert (prefix_len + 2 <= strlen (name));
> -  gdb_assert (name[prefix_len + 1] == ':');
> +  /* Check for malformed input.  */
> +  if (prefix_len + 2 > strlen (name) || name[prefix_len + 1] != ':')
> +    return null_block_symbol;
>  
>    /* Find the name of the class and the name of the method, variable, etc.  */
>  
> diff --git a/gdb/cp-support.c b/gdb/cp-support.c
> index df127c4..a71c6ad 100644
> --- a/gdb/cp-support.c
> +++ b/gdb/cp-support.c
> @@ -1037,8 +1037,13 @@ cp_find_first_component_aux (const char *name, int permissive)
>  	      return strlen (name);
>  	    }
>  	case '\0':
> -	case ':':
>  	  return index;
> +	case ':':
> +	  /* ':' marks a component iff the next character is also a ':'.
> +	     Otherwise it is probably malformed input.  */
> +	  if (name[index + 1] == ':')
> +	    return index;
> +	  break;
>  	case 'o':
>  	  /* Operator names can screw up the recursion.  */
>  	  if (operator_possible
> diff --git a/gdb/testsuite/gdb.cp/scope-err.cc b/gdb/testsuite/gdb.cp/scope-err.cc
> new file mode 100644
> index 0000000..19c92d8
> --- /dev/null
> +++ b/gdb/testsuite/gdb.cp/scope-err.cc
> @@ -0,0 +1,35 @@
> +/* This testcase is part of GDB, the GNU debugger.
> +
> +   Copyright 2016 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
> +   the Free Software Foundation; either version 3 of the License, or
> +   (at your option) any later version.
> +
> +   This program is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
> +
> +int
> +myfunction (int aa)
> +{
> +  int i;
> +
> +  i = aa + 42;
> +  return i;    /* set breakpoint here */
> +}
> +
> +int
> +main (void)
> +{
> +  int a;
> +
> +  a = myfunction (a);
> +
> +  return a;
> +}
> diff --git a/gdb/testsuite/gdb.cp/scope-err.exp b/gdb/testsuite/gdb.cp/scope-err.exp
> new file mode 100644
> index 0000000..9d93578
> --- /dev/null
> +++ b/gdb/testsuite/gdb.cp/scope-err.exp
> @@ -0,0 +1,47 @@
> +# Copyright 2012-2016 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
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
> +
> +# Tests for linespec errors with C++.
> +# Derived from gdb.linespec/ls-errs.exp.
> +
> +if { [skip_cplus_tests] } { continue }
> +
> +standard_testfile .cc
> +set exefile $testfile
> +
> +if {[prepare_for_testing $testfile $exefile $srcfile {debug c++}]} {
> +    return -1
> +}
> +
> +if ![runto_main] {
> +    fail "Can't run to main"
> +    return 0
> +}
> +
> +# Run to a location in the file.
> +set bp_location [gdb_get_line_number "set breakpoint here"]
> +
> +gdb_test "break $srcfile:$bp_location" \
> +    "Breakpoint.*at.* file .*$srcfile, line $bp_location\\." \
> +    "breakpoint line number in file"
> +
> +gdb_continue_to_breakpoint "$bp_location"
> +
> +# Try to access a variable using scope that is a non-existent filename
> +# with a Windows-style logical drive in the name.
> +set nonexistent_file C:/does/not/exist.cc
> +gdb_test "print '$nonexistent_file'::var" \
> +	 ".*No symbol \"$nonexistent_file\" in current context.*" \
> +	 "print var from \"$nonexistent_file\""
> diff --git a/gdb/testsuite/gdb.linespec/ls-errs.c b/gdb/testsuite/gdb.linespec/ls-errs.c
> index ca41342..a3a43db 100644
> --- a/gdb/testsuite/gdb.linespec/ls-errs.c
> +++ b/gdb/testsuite/gdb.linespec/ls-errs.c
> @@ -15,14 +15,21 @@
>     You should have received a copy of the GNU General Public License
>     along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
>  
> -int myfunction (void) { return 0; }
> +int
> +myfunction (int aa)
> +{
> +  int i;
> +
> +  i = aa + 42;
> +  return i;    /* set breakpoint here */
> +}
>  
>  int
>  main (void)
> -{  
> +{
>    int a;
>  
> -  a = myfunction ();
> +  a = myfunction (a);
>  
>   here:
>    return a;
> diff --git a/gdb/testsuite/gdb.linespec/ls-errs.exp b/gdb/testsuite/gdb.linespec/ls-errs.exp
> index 16d4574..35f8f78 100644
> --- a/gdb/testsuite/gdb.linespec/ls-errs.exp
> +++ b/gdb/testsuite/gdb.linespec/ls-errs.exp
> @@ -13,209 +13,247 @@
>  # You should have received a copy of the GNU General Public License
>  # along with this program.  If not, see <http://www.gnu.org/licenses/>.
>  
> -# Tests for linespec error conditions
> +# Tests for linespec errors with C and C++.
>  
> -standard_testfile
> -set exefile $testfile
> +# The test proper.  LANG is either C or C++.
>  
> -if {[prepare_for_testing $testfile $exefile $srcfile \
> -	 {debug nowarnings}]} {
> -    return -1
> -}
> +proc do_test {lang} {
> +    global testfile srcfile error_messages compiler_info
>  
> -# Turn off the pending breakpoint queries.
> -gdb_test_no_output "set breakpoint pending off"
> +    standard_testfile
> +    set exefile $testfile
> +    if [info exists compiler_info] {
> +	# Unsetting compiler_info allows us to switch compilers
> +	# used by prepare_for_testing.
> +        unset compiler_info
> +    }
> +    set options {debug}
>  
> -# Turn off completion limiting
> -gdb_test_no_output "set max-completions unlimited"
> +    if { $lang == "C++" } {
> +	if { [skip_cplus_tests] } { return 0 }
> +	# Build ".c" source file with g++.
> +	lappend options "c++"
> +    }
>  
> -# We intentionally do not use gdb_breakpoint for these tests.
> +    if {[prepare_for_testing $testfile $exefile $srcfile $options]} {
> +	return -1
> +    }
>  
> -# 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
> +    # Turn off the pending breakpoint queries.
> +    gdb_test_no_output "set breakpoint pending off"
>  
> -    gdb_test "break $linespec" [string_to_regexp \
> -				[eval format \$error_messages($msg_id) $args]]
> -}
> +    # Turn off completion limiting
> +    gdb_test_no_output "set max-completions unlimited"
>  
> -# Common error message format strings.
> -array set error_messages {
> -    invalid_file "No source file named %s."
> -    invalid_function "Function \"%s\" not defined."
> -    invalid_var_or_func "Undefined convenience variable or function \"%s\" not defined."
> -    invalid_function_f "Function \"%s\" not defined in \"%s\"."
> -    invalid_var_or_func_f \
> -	"Undefined convenience variable or function \"%s\" not defined in \"%s\"."
> -    invalid_label "No label \"%s\" defined in function \"%s\"."
> -    invalid_parm "invalid linespec argument, \"%s\""
> -    invalid_offset "No line %d in the current file."
> -    invalid_offset_f "No line %d in file \"%s\"."
> -    malformed_line_offset "malformed line offset: \"%s\""
> -    source_incomplete \
> -	"Source filename requires function, label, or line offset."
> -    unexpected "malformed linespec error: unexpected %s"
> -    unexpected_opt "malformed linespec error: unexpected %s, \"%s\""
> -    unmatched_quote "unmatched quote"
> -    garbage "Garbage '%s' at end of command"
> -}
> +    if ![runto_main] {
> +	fail "Can't run to main"
> +	return 0
> +    }
>  
> -# Some commonly used whitespace tests around ':'.
> -set spaces [list ":" ": " " :" " : " "\t:  " "  :\t" "\t:\t" " \t:\t " \
> -		"\t  \t:\t  \t  \t"]
> +    # Run to a location in the file.
> +    set bp_location [gdb_get_line_number "set breakpoint here"]
> +
> +    gdb_test "break $srcfile:$bp_location" \
> +    "Breakpoint.*at.* file .*$srcfile, line $bp_location\\." \
> +    "breakpoint line number in file"
> +
> +    gdb_continue_to_breakpoint "$bp_location"
> +
> +    # Common error message format strings.
> +    array set error_messages {
> +	invalid_file "No source file named %s."
> +	invalid_function "Function \"%s\" not defined."
> +	invalid_var_or_func 
> +	    "Undefined convenience variable or function \"%s\" not defined."
> +	invalid_function_f "Function \"%s\" not defined in \"%s\"."
> +	invalid_var_or_func_f \
> +	    "Undefined convenience variable or function \"%s\" not defined in \"%s\"."
> +	invalid_label "No label \"%s\" defined in function \"%s\"."
> +	invalid_parm "invalid linespec argument, \"%s\""
> +	invalid_offset "No line %d in the current file."
> +	invalid_offset_f "No line %d in file \"%s\"."
> +	malformed_line_offset "malformed line offset: \"%s\""
> +	source_incomplete \
> +	    "Source filename requires function, label, or line offset."
> +	unexpected "malformed linespec error: unexpected %s"
> +	unexpected_opt "malformed linespec error: unexpected %s, \"%s\""
> +	unmatched_quote "unmatched quote"
> +	garbage "Garbage '%s' at end of command"
> +    }
>  
> -# A list of invalid offsets.
> -set invalid_offsets [list -100 +500 1000]
> +    # We intentionally do not use gdb_breakpoint for these tests.
>  
> -# Try some simple, invalid linespecs involving spaces.
> -foreach x $spaces {
> -    test_break $x unexpected "colon"
> -}
> +    # 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
>  
> -# Test invalid filespecs starting with offset.  This is done
> -# first so that default offsets are tested.
> -foreach x $invalid_offsets {
> -    set offset $x
> -
> -    # Relative offsets are relative to line 16.  Adjust
> -    # expected offset from error message accordingly.
> -    if {[string index $x 0] == "+" ||
> -	[string index $x 0] == "-"} {
> -	incr offset 16
> +	gdb_test "break $linespec" [string_to_regexp \
> +				    [eval format \$error_messages($msg_id) \
> +				     $args]]
>      }
> -    test_break $x invalid_offset $offset
> -    test_break "-line $x" invalid_offset $offset
> -}
>  
> -# Test offsets with trailing tokens w/ and w/o spaces.
> -foreach x $spaces {
> -    test_break "3$x" unexpected "colon"
> -    test_break "+10$x" unexpected "colon"
> -    test_break "-10$x" unexpected "colon"
> -}
> +    # Some commonly used whitespace tests around ':'.
> +    set spaces [list ":" ": " " :" " : " "\t:  " "  :\t" "\t:\t" \
> +		     " \t:\t " "\t  \t:\t  \t  \t"]
>  
> -foreach x {1 +1 +100 -10} {
> -    test_break "3 $x" unexpected_opt "number" $x
> -    test_break "-line 3 $x" garbage $x
> -    test_break "+10 $x" unexpected_opt "number" $x
> -    test_break "-line +10 $x" garbage $x
> -    test_break "-10 $x" unexpected_opt "number" $x
> -    test_break "-line -10 $x" garbage $x
> -}
> +    # A list of invalid offsets.
> +    set invalid_offsets [list -100 +500 1000]
>  
> -foreach x {3 +10 -10} {
> -    test_break "$x foo" unexpected_opt "string" "foo"
> -    test_break "-line $x foo" garbage "foo"
> -}
> +    # Try some simple, invalid linespecs involving spaces.
> +    foreach x $spaces {
> +	test_break $x unexpected "colon"
> +    }
>  
> -# Test invalid linespecs starting with filename.
> -foreach x [list "this_file_doesn't_exist.c" \
> -	       "this file has spaces.c" \
> -	       "\"file::colons.c\"" \
> -	       "'file::colons.c'" \
> -	       "\"this \"file\" has quotes.c\"" \
> -	       "'this \"file\" has quotes.c'" \
> -	       "'this 'file' has quotes.c'" \
> -	       "\"this 'file' has quotes.c\"" \
> -	       "\"spaces: and :colons.c\"" \
> -	       "'more: :spaces: :and  colons::.c'"] {
> -    # Remove any quoting from FILENAME for the error message.
> -    test_break "$x:3" invalid_file [string trim $x \"']
> -}
> -foreach x [list "this_file_doesn't_exist.c" \
> -	       "file::colons.c" \
> -	       "'file::colons.c'"] {
> -    test_break "-source $x -line 3" \
> -	invalid_file [string trim $x \"']
> -}
> +    # Test invalid filespecs starting with offset.  This is done
> +    # first so that default offsets are tested.
> +    foreach x $invalid_offsets {
> +	set offset $x
> +
> +	# Relative offsets are relative to line 16.  Adjust
> +	# expected offset from error message accordingly.
> +	if {[string index $x 0] == "+" || [string index $x 0] == "-"} {
> + 	    incr offset 24
> +	}
> +	test_break $x invalid_offset $offset
> +	test_break "-line $x" invalid_offset $offset
> +    }
> +
> +    # Test offsets with trailing tokens w/ and w/o spaces.
> +    foreach x $spaces {
> +	test_break "3$x" unexpected "colon"
> +	test_break "+10$x" unexpected "colon"
> +	test_break "-10$x" unexpected "colon"
> +    }
> +
> +    foreach x {1 +1 +100 -10} {
> +	test_break "3 $x" unexpected_opt "number" $x
> +	test_break "-line 3 $x" garbage $x
> +	test_break "+10 $x" unexpected_opt "number" $x
> +	test_break "-line +10 $x" garbage $x
> +	test_break "-10 $x" unexpected_opt "number" $x
> +	test_break "-line -10 $x" garbage $x
> +    }
>  
> -# Test that option lexing stops at whitespace boundaries
> -test_break "-source this file has spaces.c -line 3" \
> -    invalid_file "this"
> +    foreach x {3 +10 -10} {
> +	test_break "$x foo" unexpected_opt "string" "foo"
> +	test_break "-line $x foo" garbage "foo"
> +    }
>  
> -test_break "-function function whitespace" \
> -    invalid_function "function"
> +    # Test invalid linespecs starting with filename.
> +    # It's OK to use the ".c" extension for the C++ test
> +    # since the extension doesn't affect GDB's lookup.
> +    set invalid_files [list "this_file_doesn't_exist.c" \
> +			    "this file has spaces.c" \
> +			    "\"file::colons.c\"" \
> +			    "'file::colons.c'" \
> +			    "\"this \"file\" has quotes.c\"" \
> +			    "'this \"file\" has quotes.c'" \
> +			    "'this 'file' has quotes.c'" \
> +			    "\"this 'file' has quotes.c\"" \
> +			    "\"spaces: and :colons.c\"" \
> +			    "'more: :spaces: :and  colons::.c'" \
> +			    "C:/nonexist-with-windrive.c"]
> +
> +    foreach x $invalid_files {
> +	# Remove any quoting from FILENAME for the error message.
> +	test_break "$x:3" invalid_file [string trim $x \"']
> +    }
> +    foreach x [list "this_file_doesn't_exist.c" \
> +		    "file::colons.c" \
> +		    "'file::colons.c'"] {
> +	test_break "-source $x -line 3" invalid_file [string trim $x \"']
> +    }
>  
> -test_break "-source $srcfile -function function whitespace" \
> -    invalid_function_f "function" $srcfile
> +    # Test that option lexing stops at whitespace boundaries
> +    test_break "-source this file has spaces.c -line 3" invalid_file "this"
> +    test_break "-function function whitespace" invalid_function "function"
> +    test_break "-source $srcfile -function function whitespace" \
> +	       invalid_function_f "function" $srcfile
>  
> -test_break "-function main -label label whitespace" \
> -    invalid_label "label" "main"
> +    test_break "-function main -label label whitespace" \
> +	       invalid_label "label" "main"
>  
> -# Test unmatched quotes.
> -foreach x {"\"src-file.c'" "'src-file.c"} {
> -    test_break "$x:3" unmatched_quote
> -}
> +    # Test unmatched quotes.
> +    foreach x {"\"src-file.c'" "'src-file.c"} {
> +	test_break "$x:3" unmatched_quote
> +    }
>  
> -test_break $srcfile invalid_function $srcfile
> -foreach x {"foo" " foo" " foo "} {
> -    # Trim any leading/trailing whitespace for error messages.
> -    test_break "$srcfile:$x" invalid_function_f [string trim $x] $srcfile
> -    test_break "-source $srcfile -function $x" \
> -	invalid_function_f [string trim $x] $srcfile
> -    test_break "$srcfile:main:$x" invalid_label [string trim $x] "main"
> -    test_break "-source $srcfile -function main -label $x" \
> -	invalid_label [string trim $x] "main"
> -}
> +    test_break $srcfile invalid_function $srcfile
> +    foreach x {"foo" " foo" " foo "} {
> +	# Trim any leading/trailing whitespace for error messages.
> +	test_break "$srcfile:$x" invalid_function_f [string trim $x] $srcfile
> +	test_break "-source $srcfile -function $x" \
> +		   invalid_function_f [string trim $x] $srcfile
> +	test_break "$srcfile:main:$x" invalid_label [string trim $x] "main"
> +	test_break "-source $srcfile -function main -label $x" \
> +		   invalid_label [string trim $x] "main"
> +    }
>  
> -foreach x $spaces {
> -    test_break "$srcfile$x" unexpected "end of input"
> -    test_break "$srcfile:main$x" unexpected "end of input"
> -}
> +    foreach x $spaces {
> +	test_break "$srcfile$x" unexpected "end of input"
> +	test_break "$srcfile:main$x" unexpected "end of input"
> +    }
>  
> -test_break "${srcfile}::" invalid_function "${srcfile}::"
> -test_break "$srcfile:3 1" unexpected_opt "number" "1"
> -test_break "-source $srcfile -line 3 1" garbage "1"
> -test_break "$srcfile:3 +100" unexpected_opt "number" "+100"
> -test_break "-source $srcfile -line 3 +100" garbage "+100"
> -test_break "$srcfile:3 -100" unexpected_opt "number" "-100"
> -test_break "$srcfile:3 foo" unexpected_opt "string" "foo"
> -test_break "-source $srcfile -line 3 foo" garbage "foo"
> -
> -foreach x $invalid_offsets {
> -    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_break "-source $srcfile -line $x" invalid_offset_f $x $srcfile
> -}
> -test_break "-source $srcfile -line -x" malformed_line_offset "-x"
> +    test_break "${srcfile}::" invalid_function "${srcfile}::"
> +    test_break "$srcfile:3 1" unexpected_opt "number" "1"
> +    test_break "-source $srcfile -line 3 1" garbage "1"
> +    test_break "$srcfile:3 +100" unexpected_opt "number" "+100"
> +    test_break "-source $srcfile -line 3 +100" garbage "+100"
> +    test_break "$srcfile:3 -100" unexpected_opt "number" "-100"
> +    test_break "$srcfile:3 foo" unexpected_opt "string" "foo"
> +    test_break "-source $srcfile -line 3 foo" garbage "foo"
> +
> +    foreach x $invalid_offsets {
> +	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_break "-source $srcfile -line $x" invalid_offset_f $x $srcfile
> +    }
> +    test_break "-source $srcfile -line -x" malformed_line_offset "-x"
>  
> -# Test invalid filespecs starting with function.
> -foreach x {"foobar" "foo::bar" "foo.bar" "foo ." "foo bar" "foo 1" \
> +    # 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"} {
> -    test_break $x invalid_function $x
> -    test_break "-function \"$x\"" invalid_function $x
> -}
> +	test_break $x invalid_function $x
> +	test_break "-function \"$x\"" invalid_function $x
> +    }
>  
> -foreach x $spaces {
> -    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"
> -}
> +    foreach x $spaces {
> +	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"
> +    }
>  
> -foreach x {"3" "+100" "-100" "foo"} {
> -    test_break "main 3" invalid_function "main 3"
> -    test_break "-function \"main $x\"" invalid_function "main $x"
> -    test_break "main:here $x" invalid_label "here $x" "main"
> -    test_break "-function main -label \"here $x\"" \
> -	invalid_label "here $x" "main"
> -}
> +    foreach x {"3" "+100" "-100" "foo"} {
> +	test_break "main 3" invalid_function "main 3"
> +	test_break "-function \"main $x\"" invalid_function "main $x"
> +	test_break "main:here $x" invalid_label "here $x" "main"
> +	test_break "-function main -label \"here $x\"" \
> +		   invalid_label "here $x" "main"
> +    }
>  
> -foreach x {"if" "task" "thread"} {
> -    test_break $x invalid_function $x
> -}
> +    foreach x {"if" "task" "thread"} {
> +	test_break $x invalid_function $x
> +    }
>  
> -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_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}
> -test_break $x invalid_var_or_func $x
> -test_break "$srcfile:$x" invalid_var_or_func_f $x $srcfile
> +    # Test undefined convenience variables.
> +    set x {$zippo}
> +    test_break $x invalid_var_or_func $x
> +    test_break "$srcfile:$x" invalid_var_or_func_f $x $srcfile
>  
> -# Explicit linespec-specific tests
> -test_break "-source $srcfile" source_incomplete
> +    # Explicit linespec-specific tests
> +    test_break "-source $srcfile" source_incomplete
> +}
> +
> +foreach_with_prefix lang {"C" "C++"} {
> +    do_test ${lang}
> +}
> diff --git a/gdb/testsuite/gdb.mi/mi-linespec-err-cp.cc b/gdb/testsuite/gdb.mi/mi-linespec-err-cp.cc
> new file mode 100644
> index 0000000..19c92d8
> --- /dev/null
> +++ b/gdb/testsuite/gdb.mi/mi-linespec-err-cp.cc
> @@ -0,0 +1,35 @@
> +/* This testcase is part of GDB, the GNU debugger.
> +
> +   Copyright 2016 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
> +   the Free Software Foundation; either version 3 of the License, or
> +   (at your option) any later version.
> +
> +   This program is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
> +
> +int
> +myfunction (int aa)
> +{
> +  int i;
> +
> +  i = aa + 42;
> +  return i;    /* set breakpoint here */
> +}
> +
> +int
> +main (void)
> +{
> +  int a;
> +
> +  a = myfunction (a);
> +
> +  return a;
> +}
> diff --git a/gdb/testsuite/gdb.mi/mi-linespec-err-cp.exp b/gdb/testsuite/gdb.mi/mi-linespec-err-cp.exp
> new file mode 100644
> index 0000000..1a8682e
> --- /dev/null
> +++ b/gdb/testsuite/gdb.mi/mi-linespec-err-cp.exp
> @@ -0,0 +1,57 @@
> +# Copyright 2016 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
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
> +
> +# Regression test for PR breakpoints/18303.  Tests that the correct
> +# errors is generated when setting a breakpoint in a non-existent
> +# file with a Windows-style logical drive names and C++.
> +
> +if { [skip_cplus_tests] } { continue }
> +
> +load_lib mi-support.exp
> +set MIFLAGS "-i=mi"
> +
> +standard_testfile .cc
> +set exefile $testfile
> +
> +if {[prepare_for_testing $testfile $exefile $srcfile {debug c++}]} {
> +    return -1
> +}
> +
> +gdb_exit
> +if [mi_gdb_start] {
> +    continue
> +}
> +
> +# Turn off the pending breakpoint queries.
> +mi_gdb_test "-interpreter-exec console \"set breakpoint pending off\"" \
> +  {=cmd-param-changed,param=\"breakpoint pending\",.*\^done} \
> +  "-interpreter-exec console \"set breakpoint pending off\""
> +
> +mi_run_to_main
> +
> +# Run to a location in the file.
> +set bp_location [gdb_get_line_number "set breakpoint here"]
> +
> +mi_gdb_test "-break-insert ${srcfile}:${bp_location}" \
> +    {\^done,bkpt=.number="2",type="breakpoint".*\}} "set breakpoint"
> +
> +mi_execute_to "exec-continue" "breakpoint-hit" "myfunction" ".*" ".*" "24" \
> +    { "" "disp=\"keep\"" } "breakpoint hit"
> +
> +# Set a breakpoint in a C++ source file whose name contains a
> +# Windows-style logical drive.
> +mi_gdb_test \
> +    "-break-insert -f \"c:/uu.cpp:13\"" \
> +    ".*No source file named c:/uu.cpp.*"
> 
Ping?
thanks
--Don

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

* Re: [PING] Re: [PATCH v4] PR 18303, Tolerate malformed input for lookup_symbol-called functions (was: [PATCH] Fix problem handling colon in linespec, PR breakpoints/18303)
  2016-02-04 18:37               ` [PING] " Don Breazeal
@ 2016-02-18 18:22                 ` Don Breazeal
  2016-02-25 17:28                   ` [PING] Re: [PATCH v4] PR 18303, Tolerate malformed input for lookup_symbol-called functions Don Breazeal
  0 siblings, 1 reply; 17+ messages in thread
From: Don Breazeal @ 2016-02-18 18:22 UTC (permalink / raw)
  To: gdb-patches

Ping.
Current status of this one:
 - Doug approved (at least in concept) Keith's fix below.
 - Pedro requested that the new C++ linespec error test be consolidated
with the existing C version of the test.  That's done in this version.
Thanks!
--Don

On 2/4/2016 10:37 AM, Don Breazeal wrote:
> On 1/28/2016 2:52 PM, Don Breazeal wrote:
>> Apologies -- the previous version of this patch was missing one file,
>> added here.  Sorry for the noise.
>> --Don
>>
>> ---V3 Comment---
>> This patch differs from v2 in that the new test gdb.linespec/ls-errs-cp.exp
>> is gone, and instead (per Pedro's suggestion) gdb.linespec/ls-errs.exp has
>> been modified to test both C & C++, and to incorporate the small amount of
>> extra testing from ls-errs-cp.exp.
>>
>> It also includes, unchanged from the previous version of the patch:
>>
>>  * an updated version of the alternate fix that Keith proposed for my
>>    patch that addressed PR breakpoints/18303.  Doug has approved (in
>>    concept, at least) the code portion of the patch.
>>
>>  * a couple of other new tests of colons in linespecs.
>>
>> Thanks,
>> --Don
>>
>> -----------
>> lookup_symbol is often called with user input.  Consequently, any
>> function called from lookup_symbol{,_in_language} should attempt to
>> deal with malformed input gracefully.  After all, malformed user
>> input is not a programming/API error.
>>
>> This patch does not attempt to find/correct all instances of this.  It
>> only fixes locations in the code that trigger test suite failures.
>>
>> This patch fixes PR breakpoints/18303, "Assertion: -breakpoint-insert
>> with windows paths of file in non-current directory".
>>
>> The patch includes three new tests related to this.  One is just
>> gdb.linespec/ls-errs.exp copied and converted to use C++ instead of C, and
>> to add a case using a file name containing a Windows-style logical drive
>> specifier.  The others include an MI test to provide a regression test for
>> the specific case reported in PR 18303, and a C++ test for proper error
>> handling of access to a program variable when using a file scope specifier
>> that refers to a non-existent file.
>>
>> Tested on x86_64 native Linux.
>>
>> gdb/ChangeLog
>> 2016-01-28  Keith Seitz  <keiths@redhat.com>
>>
>> 	PR breakpoints/18303
>> 	* cp-namespace.c (cp_lookup_bare_symbol): Change assertion to
>> 	look for "::" instead of simply ":".
>> 	(cp_search_static_and_baseclasses): Return null_block_symbol for
>> 	malformed input.
>> 	Remove assertions.
>> 	* cp-support.c (cp_find_first_component_aux): Do not return
>> 	a prefix length for ':' unless the next character is also ':'.
>>
>> gdb/testsuite/ChangeLog
>> 2016-01-28  Don Breazeal  <donb@codesourcery.com>
>>
>> 	* gdb.cp/scope-err.cc: New test program.
>> 	* gdb.cp/scope-err.exp: New test script.
>> 	* gdb.linespec/ls-errs.c (myfunction): Expanded to have multiple
>> 	lines and "set breakpoint here" comment.
>> 	* gdb.linespec/ls-errs.exp: Added C++ testing and new test case.
>> 	* gdb.mi/mi-linespec-err-cp.cc: New test program.
>> 	* gdb.mi/mi-linespec-err-cp.exp: New test script.
>>
>> ---
>>  gdb/cp-namespace.c                          |   9 +-
>>  gdb/cp-support.c                            |   7 +-
>>  gdb/testsuite/gdb.cp/scope-err.cc           |  35 +++
>>  gdb/testsuite/gdb.cp/scope-err.exp          |  47 ++++
>>  gdb/testsuite/gdb.linespec/ls-errs.c        |  13 +-
>>  gdb/testsuite/gdb.linespec/ls-errs.exp      | 384 +++++++++++++++-------------
>>  gdb/testsuite/gdb.mi/mi-linespec-err-cp.cc  |  35 +++
>>  gdb/testsuite/gdb.mi/mi-linespec-err-cp.exp |  57 +++++
>>  8 files changed, 405 insertions(+), 182 deletions(-)
>>
>> diff --git a/gdb/cp-namespace.c b/gdb/cp-namespace.c
>> index 72002d6..016a42f 100644
>> --- a/gdb/cp-namespace.c
>> +++ b/gdb/cp-namespace.c
>> @@ -170,7 +170,7 @@ cp_lookup_bare_symbol (const struct language_defn *langdef,
>>       ':' may be in the args of a template spec.  This isn't intended to be
>>       a complete test, just cheap and documentary.  */
>>    if (strchr (name, '<') == NULL && strchr (name, '(') == NULL)
>> -    gdb_assert (strchr (name, ':') == NULL);
>> +    gdb_assert (strstr (name, "::") == NULL);
>>  
>>    sym = lookup_symbol_in_static_block (name, block, domain);
>>    if (sym.symbol != NULL)
>> @@ -246,10 +246,9 @@ cp_search_static_and_baseclasses (const char *name,
>>    struct block_symbol klass_sym;
>>    struct type *klass_type;
>>  
>> -  /* The test here uses <= instead of < because Fortran also uses this,
>> -     and the module.exp testcase will pass "modmany::" for NAME here.  */
>> -  gdb_assert (prefix_len + 2 <= strlen (name));
>> -  gdb_assert (name[prefix_len + 1] == ':');
>> +  /* Check for malformed input.  */
>> +  if (prefix_len + 2 > strlen (name) || name[prefix_len + 1] != ':')
>> +    return null_block_symbol;
>>  
>>    /* Find the name of the class and the name of the method, variable, etc.  */
>>  
>> diff --git a/gdb/cp-support.c b/gdb/cp-support.c
>> index df127c4..a71c6ad 100644
>> --- a/gdb/cp-support.c
>> +++ b/gdb/cp-support.c
>> @@ -1037,8 +1037,13 @@ cp_find_first_component_aux (const char *name, int permissive)
>>  	      return strlen (name);
>>  	    }
>>  	case '\0':
>> -	case ':':
>>  	  return index;
>> +	case ':':
>> +	  /* ':' marks a component iff the next character is also a ':'.
>> +	     Otherwise it is probably malformed input.  */
>> +	  if (name[index + 1] == ':')
>> +	    return index;
>> +	  break;
>>  	case 'o':
>>  	  /* Operator names can screw up the recursion.  */
>>  	  if (operator_possible
>> diff --git a/gdb/testsuite/gdb.cp/scope-err.cc b/gdb/testsuite/gdb.cp/scope-err.cc
>> new file mode 100644
>> index 0000000..19c92d8
>> --- /dev/null
>> +++ b/gdb/testsuite/gdb.cp/scope-err.cc
>> @@ -0,0 +1,35 @@
>> +/* This testcase is part of GDB, the GNU debugger.
>> +
>> +   Copyright 2016 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
>> +   the Free Software Foundation; either version 3 of the License, or
>> +   (at your option) any later version.
>> +
>> +   This program is distributed in the hope that it will be useful,
>> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
>> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> +   GNU General Public License for more details.
>> +
>> +   You should have received a copy of the GNU General Public License
>> +   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
>> +
>> +int
>> +myfunction (int aa)
>> +{
>> +  int i;
>> +
>> +  i = aa + 42;
>> +  return i;    /* set breakpoint here */
>> +}
>> +
>> +int
>> +main (void)
>> +{
>> +  int a;
>> +
>> +  a = myfunction (a);
>> +
>> +  return a;
>> +}
>> diff --git a/gdb/testsuite/gdb.cp/scope-err.exp b/gdb/testsuite/gdb.cp/scope-err.exp
>> new file mode 100644
>> index 0000000..9d93578
>> --- /dev/null
>> +++ b/gdb/testsuite/gdb.cp/scope-err.exp
>> @@ -0,0 +1,47 @@
>> +# Copyright 2012-2016 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
>> +# the Free Software Foundation; either version 3 of the License, or
>> +# (at your option) any later version.
>> +#
>> +# This program is distributed in the hope that it will be useful,
>> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
>> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> +# GNU General Public License for more details.
>> +#
>> +# You should have received a copy of the GNU General Public License
>> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
>> +
>> +# Tests for linespec errors with C++.
>> +# Derived from gdb.linespec/ls-errs.exp.
>> +
>> +if { [skip_cplus_tests] } { continue }
>> +
>> +standard_testfile .cc
>> +set exefile $testfile
>> +
>> +if {[prepare_for_testing $testfile $exefile $srcfile {debug c++}]} {
>> +    return -1
>> +}
>> +
>> +if ![runto_main] {
>> +    fail "Can't run to main"
>> +    return 0
>> +}
>> +
>> +# Run to a location in the file.
>> +set bp_location [gdb_get_line_number "set breakpoint here"]
>> +
>> +gdb_test "break $srcfile:$bp_location" \
>> +    "Breakpoint.*at.* file .*$srcfile, line $bp_location\\." \
>> +    "breakpoint line number in file"
>> +
>> +gdb_continue_to_breakpoint "$bp_location"
>> +
>> +# Try to access a variable using scope that is a non-existent filename
>> +# with a Windows-style logical drive in the name.
>> +set nonexistent_file C:/does/not/exist.cc
>> +gdb_test "print '$nonexistent_file'::var" \
>> +	 ".*No symbol \"$nonexistent_file\" in current context.*" \
>> +	 "print var from \"$nonexistent_file\""
>> diff --git a/gdb/testsuite/gdb.linespec/ls-errs.c b/gdb/testsuite/gdb.linespec/ls-errs.c
>> index ca41342..a3a43db 100644
>> --- a/gdb/testsuite/gdb.linespec/ls-errs.c
>> +++ b/gdb/testsuite/gdb.linespec/ls-errs.c
>> @@ -15,14 +15,21 @@
>>     You should have received a copy of the GNU General Public License
>>     along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
>>  
>> -int myfunction (void) { return 0; }
>> +int
>> +myfunction (int aa)
>> +{
>> +  int i;
>> +
>> +  i = aa + 42;
>> +  return i;    /* set breakpoint here */
>> +}
>>  
>>  int
>>  main (void)
>> -{  
>> +{
>>    int a;
>>  
>> -  a = myfunction ();
>> +  a = myfunction (a);
>>  
>>   here:
>>    return a;
>> diff --git a/gdb/testsuite/gdb.linespec/ls-errs.exp b/gdb/testsuite/gdb.linespec/ls-errs.exp
>> index 16d4574..35f8f78 100644
>> --- a/gdb/testsuite/gdb.linespec/ls-errs.exp
>> +++ b/gdb/testsuite/gdb.linespec/ls-errs.exp
>> @@ -13,209 +13,247 @@
>>  # You should have received a copy of the GNU General Public License
>>  # along with this program.  If not, see <http://www.gnu.org/licenses/>.
>>  
>> -# Tests for linespec error conditions
>> +# Tests for linespec errors with C and C++.
>>  
>> -standard_testfile
>> -set exefile $testfile
>> +# The test proper.  LANG is either C or C++.
>>  
>> -if {[prepare_for_testing $testfile $exefile $srcfile \
>> -	 {debug nowarnings}]} {
>> -    return -1
>> -}
>> +proc do_test {lang} {
>> +    global testfile srcfile error_messages compiler_info
>>  
>> -# Turn off the pending breakpoint queries.
>> -gdb_test_no_output "set breakpoint pending off"
>> +    standard_testfile
>> +    set exefile $testfile
>> +    if [info exists compiler_info] {
>> +	# Unsetting compiler_info allows us to switch compilers
>> +	# used by prepare_for_testing.
>> +        unset compiler_info
>> +    }
>> +    set options {debug}
>>  
>> -# Turn off completion limiting
>> -gdb_test_no_output "set max-completions unlimited"
>> +    if { $lang == "C++" } {
>> +	if { [skip_cplus_tests] } { return 0 }
>> +	# Build ".c" source file with g++.
>> +	lappend options "c++"
>> +    }
>>  
>> -# We intentionally do not use gdb_breakpoint for these tests.
>> +    if {[prepare_for_testing $testfile $exefile $srcfile $options]} {
>> +	return -1
>> +    }
>>  
>> -# 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
>> +    # Turn off the pending breakpoint queries.
>> +    gdb_test_no_output "set breakpoint pending off"
>>  
>> -    gdb_test "break $linespec" [string_to_regexp \
>> -				[eval format \$error_messages($msg_id) $args]]
>> -}
>> +    # Turn off completion limiting
>> +    gdb_test_no_output "set max-completions unlimited"
>>  
>> -# Common error message format strings.
>> -array set error_messages {
>> -    invalid_file "No source file named %s."
>> -    invalid_function "Function \"%s\" not defined."
>> -    invalid_var_or_func "Undefined convenience variable or function \"%s\" not defined."
>> -    invalid_function_f "Function \"%s\" not defined in \"%s\"."
>> -    invalid_var_or_func_f \
>> -	"Undefined convenience variable or function \"%s\" not defined in \"%s\"."
>> -    invalid_label "No label \"%s\" defined in function \"%s\"."
>> -    invalid_parm "invalid linespec argument, \"%s\""
>> -    invalid_offset "No line %d in the current file."
>> -    invalid_offset_f "No line %d in file \"%s\"."
>> -    malformed_line_offset "malformed line offset: \"%s\""
>> -    source_incomplete \
>> -	"Source filename requires function, label, or line offset."
>> -    unexpected "malformed linespec error: unexpected %s"
>> -    unexpected_opt "malformed linespec error: unexpected %s, \"%s\""
>> -    unmatched_quote "unmatched quote"
>> -    garbage "Garbage '%s' at end of command"
>> -}
>> +    if ![runto_main] {
>> +	fail "Can't run to main"
>> +	return 0
>> +    }
>>  
>> -# Some commonly used whitespace tests around ':'.
>> -set spaces [list ":" ": " " :" " : " "\t:  " "  :\t" "\t:\t" " \t:\t " \
>> -		"\t  \t:\t  \t  \t"]
>> +    # Run to a location in the file.
>> +    set bp_location [gdb_get_line_number "set breakpoint here"]
>> +
>> +    gdb_test "break $srcfile:$bp_location" \
>> +    "Breakpoint.*at.* file .*$srcfile, line $bp_location\\." \
>> +    "breakpoint line number in file"
>> +
>> +    gdb_continue_to_breakpoint "$bp_location"
>> +
>> +    # Common error message format strings.
>> +    array set error_messages {
>> +	invalid_file "No source file named %s."
>> +	invalid_function "Function \"%s\" not defined."
>> +	invalid_var_or_func 
>> +	    "Undefined convenience variable or function \"%s\" not defined."
>> +	invalid_function_f "Function \"%s\" not defined in \"%s\"."
>> +	invalid_var_or_func_f \
>> +	    "Undefined convenience variable or function \"%s\" not defined in \"%s\"."
>> +	invalid_label "No label \"%s\" defined in function \"%s\"."
>> +	invalid_parm "invalid linespec argument, \"%s\""
>> +	invalid_offset "No line %d in the current file."
>> +	invalid_offset_f "No line %d in file \"%s\"."
>> +	malformed_line_offset "malformed line offset: \"%s\""
>> +	source_incomplete \
>> +	    "Source filename requires function, label, or line offset."
>> +	unexpected "malformed linespec error: unexpected %s"
>> +	unexpected_opt "malformed linespec error: unexpected %s, \"%s\""
>> +	unmatched_quote "unmatched quote"
>> +	garbage "Garbage '%s' at end of command"
>> +    }
>>  
>> -# A list of invalid offsets.
>> -set invalid_offsets [list -100 +500 1000]
>> +    # We intentionally do not use gdb_breakpoint for these tests.
>>  
>> -# Try some simple, invalid linespecs involving spaces.
>> -foreach x $spaces {
>> -    test_break $x unexpected "colon"
>> -}
>> +    # 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
>>  
>> -# Test invalid filespecs starting with offset.  This is done
>> -# first so that default offsets are tested.
>> -foreach x $invalid_offsets {
>> -    set offset $x
>> -
>> -    # Relative offsets are relative to line 16.  Adjust
>> -    # expected offset from error message accordingly.
>> -    if {[string index $x 0] == "+" ||
>> -	[string index $x 0] == "-"} {
>> -	incr offset 16
>> +	gdb_test "break $linespec" [string_to_regexp \
>> +				    [eval format \$error_messages($msg_id) \
>> +				     $args]]
>>      }
>> -    test_break $x invalid_offset $offset
>> -    test_break "-line $x" invalid_offset $offset
>> -}
>>  
>> -# Test offsets with trailing tokens w/ and w/o spaces.
>> -foreach x $spaces {
>> -    test_break "3$x" unexpected "colon"
>> -    test_break "+10$x" unexpected "colon"
>> -    test_break "-10$x" unexpected "colon"
>> -}
>> +    # Some commonly used whitespace tests around ':'.
>> +    set spaces [list ":" ": " " :" " : " "\t:  " "  :\t" "\t:\t" \
>> +		     " \t:\t " "\t  \t:\t  \t  \t"]
>>  
>> -foreach x {1 +1 +100 -10} {
>> -    test_break "3 $x" unexpected_opt "number" $x
>> -    test_break "-line 3 $x" garbage $x
>> -    test_break "+10 $x" unexpected_opt "number" $x
>> -    test_break "-line +10 $x" garbage $x
>> -    test_break "-10 $x" unexpected_opt "number" $x
>> -    test_break "-line -10 $x" garbage $x
>> -}
>> +    # A list of invalid offsets.
>> +    set invalid_offsets [list -100 +500 1000]
>>  
>> -foreach x {3 +10 -10} {
>> -    test_break "$x foo" unexpected_opt "string" "foo"
>> -    test_break "-line $x foo" garbage "foo"
>> -}
>> +    # Try some simple, invalid linespecs involving spaces.
>> +    foreach x $spaces {
>> +	test_break $x unexpected "colon"
>> +    }
>>  
>> -# Test invalid linespecs starting with filename.
>> -foreach x [list "this_file_doesn't_exist.c" \
>> -	       "this file has spaces.c" \
>> -	       "\"file::colons.c\"" \
>> -	       "'file::colons.c'" \
>> -	       "\"this \"file\" has quotes.c\"" \
>> -	       "'this \"file\" has quotes.c'" \
>> -	       "'this 'file' has quotes.c'" \
>> -	       "\"this 'file' has quotes.c\"" \
>> -	       "\"spaces: and :colons.c\"" \
>> -	       "'more: :spaces: :and  colons::.c'"] {
>> -    # Remove any quoting from FILENAME for the error message.
>> -    test_break "$x:3" invalid_file [string trim $x \"']
>> -}
>> -foreach x [list "this_file_doesn't_exist.c" \
>> -	       "file::colons.c" \
>> -	       "'file::colons.c'"] {
>> -    test_break "-source $x -line 3" \
>> -	invalid_file [string trim $x \"']
>> -}
>> +    # Test invalid filespecs starting with offset.  This is done
>> +    # first so that default offsets are tested.
>> +    foreach x $invalid_offsets {
>> +	set offset $x
>> +
>> +	# Relative offsets are relative to line 16.  Adjust
>> +	# expected offset from error message accordingly.
>> +	if {[string index $x 0] == "+" || [string index $x 0] == "-"} {
>> + 	    incr offset 24
>> +	}
>> +	test_break $x invalid_offset $offset
>> +	test_break "-line $x" invalid_offset $offset
>> +    }
>> +
>> +    # Test offsets with trailing tokens w/ and w/o spaces.
>> +    foreach x $spaces {
>> +	test_break "3$x" unexpected "colon"
>> +	test_break "+10$x" unexpected "colon"
>> +	test_break "-10$x" unexpected "colon"
>> +    }
>> +
>> +    foreach x {1 +1 +100 -10} {
>> +	test_break "3 $x" unexpected_opt "number" $x
>> +	test_break "-line 3 $x" garbage $x
>> +	test_break "+10 $x" unexpected_opt "number" $x
>> +	test_break "-line +10 $x" garbage $x
>> +	test_break "-10 $x" unexpected_opt "number" $x
>> +	test_break "-line -10 $x" garbage $x
>> +    }
>>  
>> -# Test that option lexing stops at whitespace boundaries
>> -test_break "-source this file has spaces.c -line 3" \
>> -    invalid_file "this"
>> +    foreach x {3 +10 -10} {
>> +	test_break "$x foo" unexpected_opt "string" "foo"
>> +	test_break "-line $x foo" garbage "foo"
>> +    }
>>  
>> -test_break "-function function whitespace" \
>> -    invalid_function "function"
>> +    # Test invalid linespecs starting with filename.
>> +    # It's OK to use the ".c" extension for the C++ test
>> +    # since the extension doesn't affect GDB's lookup.
>> +    set invalid_files [list "this_file_doesn't_exist.c" \
>> +			    "this file has spaces.c" \
>> +			    "\"file::colons.c\"" \
>> +			    "'file::colons.c'" \
>> +			    "\"this \"file\" has quotes.c\"" \
>> +			    "'this \"file\" has quotes.c'" \
>> +			    "'this 'file' has quotes.c'" \
>> +			    "\"this 'file' has quotes.c\"" \
>> +			    "\"spaces: and :colons.c\"" \
>> +			    "'more: :spaces: :and  colons::.c'" \
>> +			    "C:/nonexist-with-windrive.c"]
>> +
>> +    foreach x $invalid_files {
>> +	# Remove any quoting from FILENAME for the error message.
>> +	test_break "$x:3" invalid_file [string trim $x \"']
>> +    }
>> +    foreach x [list "this_file_doesn't_exist.c" \
>> +		    "file::colons.c" \
>> +		    "'file::colons.c'"] {
>> +	test_break "-source $x -line 3" invalid_file [string trim $x \"']
>> +    }
>>  
>> -test_break "-source $srcfile -function function whitespace" \
>> -    invalid_function_f "function" $srcfile
>> +    # Test that option lexing stops at whitespace boundaries
>> +    test_break "-source this file has spaces.c -line 3" invalid_file "this"
>> +    test_break "-function function whitespace" invalid_function "function"
>> +    test_break "-source $srcfile -function function whitespace" \
>> +	       invalid_function_f "function" $srcfile
>>  
>> -test_break "-function main -label label whitespace" \
>> -    invalid_label "label" "main"
>> +    test_break "-function main -label label whitespace" \
>> +	       invalid_label "label" "main"
>>  
>> -# Test unmatched quotes.
>> -foreach x {"\"src-file.c'" "'src-file.c"} {
>> -    test_break "$x:3" unmatched_quote
>> -}
>> +    # Test unmatched quotes.
>> +    foreach x {"\"src-file.c'" "'src-file.c"} {
>> +	test_break "$x:3" unmatched_quote
>> +    }
>>  
>> -test_break $srcfile invalid_function $srcfile
>> -foreach x {"foo" " foo" " foo "} {
>> -    # Trim any leading/trailing whitespace for error messages.
>> -    test_break "$srcfile:$x" invalid_function_f [string trim $x] $srcfile
>> -    test_break "-source $srcfile -function $x" \
>> -	invalid_function_f [string trim $x] $srcfile
>> -    test_break "$srcfile:main:$x" invalid_label [string trim $x] "main"
>> -    test_break "-source $srcfile -function main -label $x" \
>> -	invalid_label [string trim $x] "main"
>> -}
>> +    test_break $srcfile invalid_function $srcfile
>> +    foreach x {"foo" " foo" " foo "} {
>> +	# Trim any leading/trailing whitespace for error messages.
>> +	test_break "$srcfile:$x" invalid_function_f [string trim $x] $srcfile
>> +	test_break "-source $srcfile -function $x" \
>> +		   invalid_function_f [string trim $x] $srcfile
>> +	test_break "$srcfile:main:$x" invalid_label [string trim $x] "main"
>> +	test_break "-source $srcfile -function main -label $x" \
>> +		   invalid_label [string trim $x] "main"
>> +    }
>>  
>> -foreach x $spaces {
>> -    test_break "$srcfile$x" unexpected "end of input"
>> -    test_break "$srcfile:main$x" unexpected "end of input"
>> -}
>> +    foreach x $spaces {
>> +	test_break "$srcfile$x" unexpected "end of input"
>> +	test_break "$srcfile:main$x" unexpected "end of input"
>> +    }
>>  
>> -test_break "${srcfile}::" invalid_function "${srcfile}::"
>> -test_break "$srcfile:3 1" unexpected_opt "number" "1"
>> -test_break "-source $srcfile -line 3 1" garbage "1"
>> -test_break "$srcfile:3 +100" unexpected_opt "number" "+100"
>> -test_break "-source $srcfile -line 3 +100" garbage "+100"
>> -test_break "$srcfile:3 -100" unexpected_opt "number" "-100"
>> -test_break "$srcfile:3 foo" unexpected_opt "string" "foo"
>> -test_break "-source $srcfile -line 3 foo" garbage "foo"
>> -
>> -foreach x $invalid_offsets {
>> -    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_break "-source $srcfile -line $x" invalid_offset_f $x $srcfile
>> -}
>> -test_break "-source $srcfile -line -x" malformed_line_offset "-x"
>> +    test_break "${srcfile}::" invalid_function "${srcfile}::"
>> +    test_break "$srcfile:3 1" unexpected_opt "number" "1"
>> +    test_break "-source $srcfile -line 3 1" garbage "1"
>> +    test_break "$srcfile:3 +100" unexpected_opt "number" "+100"
>> +    test_break "-source $srcfile -line 3 +100" garbage "+100"
>> +    test_break "$srcfile:3 -100" unexpected_opt "number" "-100"
>> +    test_break "$srcfile:3 foo" unexpected_opt "string" "foo"
>> +    test_break "-source $srcfile -line 3 foo" garbage "foo"
>> +
>> +    foreach x $invalid_offsets {
>> +	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_break "-source $srcfile -line $x" invalid_offset_f $x $srcfile
>> +    }
>> +    test_break "-source $srcfile -line -x" malformed_line_offset "-x"
>>  
>> -# Test invalid filespecs starting with function.
>> -foreach x {"foobar" "foo::bar" "foo.bar" "foo ." "foo bar" "foo 1" \
>> +    # 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"} {
>> -    test_break $x invalid_function $x
>> -    test_break "-function \"$x\"" invalid_function $x
>> -}
>> +	test_break $x invalid_function $x
>> +	test_break "-function \"$x\"" invalid_function $x
>> +    }
>>  
>> -foreach x $spaces {
>> -    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"
>> -}
>> +    foreach x $spaces {
>> +	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"
>> +    }
>>  
>> -foreach x {"3" "+100" "-100" "foo"} {
>> -    test_break "main 3" invalid_function "main 3"
>> -    test_break "-function \"main $x\"" invalid_function "main $x"
>> -    test_break "main:here $x" invalid_label "here $x" "main"
>> -    test_break "-function main -label \"here $x\"" \
>> -	invalid_label "here $x" "main"
>> -}
>> +    foreach x {"3" "+100" "-100" "foo"} {
>> +	test_break "main 3" invalid_function "main 3"
>> +	test_break "-function \"main $x\"" invalid_function "main $x"
>> +	test_break "main:here $x" invalid_label "here $x" "main"
>> +	test_break "-function main -label \"here $x\"" \
>> +		   invalid_label "here $x" "main"
>> +    }
>>  
>> -foreach x {"if" "task" "thread"} {
>> -    test_break $x invalid_function $x
>> -}
>> +    foreach x {"if" "task" "thread"} {
>> +	test_break $x invalid_function $x
>> +    }
>>  
>> -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_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}
>> -test_break $x invalid_var_or_func $x
>> -test_break "$srcfile:$x" invalid_var_or_func_f $x $srcfile
>> +    # Test undefined convenience variables.
>> +    set x {$zippo}
>> +    test_break $x invalid_var_or_func $x
>> +    test_break "$srcfile:$x" invalid_var_or_func_f $x $srcfile
>>  
>> -# Explicit linespec-specific tests
>> -test_break "-source $srcfile" source_incomplete
>> +    # Explicit linespec-specific tests
>> +    test_break "-source $srcfile" source_incomplete
>> +}
>> +
>> +foreach_with_prefix lang {"C" "C++"} {
>> +    do_test ${lang}
>> +}
>> diff --git a/gdb/testsuite/gdb.mi/mi-linespec-err-cp.cc b/gdb/testsuite/gdb.mi/mi-linespec-err-cp.cc
>> new file mode 100644
>> index 0000000..19c92d8
>> --- /dev/null
>> +++ b/gdb/testsuite/gdb.mi/mi-linespec-err-cp.cc
>> @@ -0,0 +1,35 @@
>> +/* This testcase is part of GDB, the GNU debugger.
>> +
>> +   Copyright 2016 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
>> +   the Free Software Foundation; either version 3 of the License, or
>> +   (at your option) any later version.
>> +
>> +   This program is distributed in the hope that it will be useful,
>> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
>> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> +   GNU General Public License for more details.
>> +
>> +   You should have received a copy of the GNU General Public License
>> +   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
>> +
>> +int
>> +myfunction (int aa)
>> +{
>> +  int i;
>> +
>> +  i = aa + 42;
>> +  return i;    /* set breakpoint here */
>> +}
>> +
>> +int
>> +main (void)
>> +{
>> +  int a;
>> +
>> +  a = myfunction (a);
>> +
>> +  return a;
>> +}
>> diff --git a/gdb/testsuite/gdb.mi/mi-linespec-err-cp.exp b/gdb/testsuite/gdb.mi/mi-linespec-err-cp.exp
>> new file mode 100644
>> index 0000000..1a8682e
>> --- /dev/null
>> +++ b/gdb/testsuite/gdb.mi/mi-linespec-err-cp.exp
>> @@ -0,0 +1,57 @@
>> +# Copyright 2016 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
>> +# the Free Software Foundation; either version 3 of the License, or
>> +# (at your option) any later version.
>> +#
>> +# This program is distributed in the hope that it will be useful,
>> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
>> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> +# GNU General Public License for more details.
>> +#
>> +# You should have received a copy of the GNU General Public License
>> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
>> +
>> +# Regression test for PR breakpoints/18303.  Tests that the correct
>> +# errors is generated when setting a breakpoint in a non-existent
>> +# file with a Windows-style logical drive names and C++.
>> +
>> +if { [skip_cplus_tests] } { continue }
>> +
>> +load_lib mi-support.exp
>> +set MIFLAGS "-i=mi"
>> +
>> +standard_testfile .cc
>> +set exefile $testfile
>> +
>> +if {[prepare_for_testing $testfile $exefile $srcfile {debug c++}]} {
>> +    return -1
>> +}
>> +
>> +gdb_exit
>> +if [mi_gdb_start] {
>> +    continue
>> +}
>> +
>> +# Turn off the pending breakpoint queries.
>> +mi_gdb_test "-interpreter-exec console \"set breakpoint pending off\"" \
>> +  {=cmd-param-changed,param=\"breakpoint pending\",.*\^done} \
>> +  "-interpreter-exec console \"set breakpoint pending off\""
>> +
>> +mi_run_to_main
>> +
>> +# Run to a location in the file.
>> +set bp_location [gdb_get_line_number "set breakpoint here"]
>> +
>> +mi_gdb_test "-break-insert ${srcfile}:${bp_location}" \
>> +    {\^done,bkpt=.number="2",type="breakpoint".*\}} "set breakpoint"
>> +
>> +mi_execute_to "exec-continue" "breakpoint-hit" "myfunction" ".*" ".*" "24" \
>> +    { "" "disp=\"keep\"" } "breakpoint hit"
>> +
>> +# Set a breakpoint in a C++ source file whose name contains a
>> +# Windows-style logical drive.
>> +mi_gdb_test \
>> +    "-break-insert -f \"c:/uu.cpp:13\"" \
>> +    ".*No source file named c:/uu.cpp.*"
>>
> Ping?
> thanks
> --Don
> 

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

* Re: [PING] Re: [PATCH v4] PR 18303, Tolerate malformed input for lookup_symbol-called functions
  2016-02-18 18:22                 ` [PING] Re: [PATCH v4] PR 18303, Tolerate malformed input for lookup_symbol-called functions (was: [PATCH] Fix problem handling colon in linespec, PR breakpoints/18303) Don Breazeal
@ 2016-02-25 17:28                   ` Don Breazeal
  2016-03-03 18:19                     ` Don Breazeal
  0 siblings, 1 reply; 17+ messages in thread
From: Don Breazeal @ 2016-02-25 17:28 UTC (permalink / raw)
  To: gdb-patches

Ping
Thanks
--Don

On 2/18/2016 10:13 AM, Don Breazeal wrote:
> Ping.
> Current status of this one:
>  - Doug approved (at least in concept) Keith's fix below.
>  - Pedro requested that the new C++ linespec error test be consolidated
> with the existing C version of the test.  That's done in this version.
> Thanks!
> --Don
> 
> On 2/4/2016 10:37 AM, Don Breazeal wrote:
>> On 1/28/2016 2:52 PM, Don Breazeal wrote:
>>> Apologies -- the previous version of this patch was missing one file,
>>> added here.  Sorry for the noise.
>>> --Don
>>>
>>> ---V3 Comment---
>>> This patch differs from v2 in that the new test gdb.linespec/ls-errs-cp.exp
>>> is gone, and instead (per Pedro's suggestion) gdb.linespec/ls-errs.exp has
>>> been modified to test both C & C++, and to incorporate the small amount of
>>> extra testing from ls-errs-cp.exp.
>>>
>>> It also includes, unchanged from the previous version of the patch:
>>>
>>>  * an updated version of the alternate fix that Keith proposed for my
>>>    patch that addressed PR breakpoints/18303.  Doug has approved (in
>>>    concept, at least) the code portion of the patch.
>>>
>>>  * a couple of other new tests of colons in linespecs.
>>>
>>> Thanks,
>>> --Don
>>>
>>> -----------
>>> lookup_symbol is often called with user input.  Consequently, any
>>> function called from lookup_symbol{,_in_language} should attempt to
>>> deal with malformed input gracefully.  After all, malformed user
>>> input is not a programming/API error.
>>>
>>> This patch does not attempt to find/correct all instances of this.  It
>>> only fixes locations in the code that trigger test suite failures.
>>>
>>> This patch fixes PR breakpoints/18303, "Assertion: -breakpoint-insert
>>> with windows paths of file in non-current directory".
>>>
>>> The patch includes three new tests related to this.  One is just
>>> gdb.linespec/ls-errs.exp copied and converted to use C++ instead of C, and
>>> to add a case using a file name containing a Windows-style logical drive
>>> specifier.  The others include an MI test to provide a regression test for
>>> the specific case reported in PR 18303, and a C++ test for proper error
>>> handling of access to a program variable when using a file scope specifier
>>> that refers to a non-existent file.
>>>
>>> Tested on x86_64 native Linux.
>>>
>>> gdb/ChangeLog
>>> 2016-01-28  Keith Seitz  <keiths@redhat.com>
>>>
>>> 	PR breakpoints/18303
>>> 	* cp-namespace.c (cp_lookup_bare_symbol): Change assertion to
>>> 	look for "::" instead of simply ":".
>>> 	(cp_search_static_and_baseclasses): Return null_block_symbol for
>>> 	malformed input.
>>> 	Remove assertions.
>>> 	* cp-support.c (cp_find_first_component_aux): Do not return
>>> 	a prefix length for ':' unless the next character is also ':'.
>>>
>>> gdb/testsuite/ChangeLog
>>> 2016-01-28  Don Breazeal  <donb@codesourcery.com>
>>>
>>> 	* gdb.cp/scope-err.cc: New test program.
>>> 	* gdb.cp/scope-err.exp: New test script.
>>> 	* gdb.linespec/ls-errs.c (myfunction): Expanded to have multiple
>>> 	lines and "set breakpoint here" comment.
>>> 	* gdb.linespec/ls-errs.exp: Added C++ testing and new test case.
>>> 	* gdb.mi/mi-linespec-err-cp.cc: New test program.
>>> 	* gdb.mi/mi-linespec-err-cp.exp: New test script.
>>>
>>> ---
>>>  gdb/cp-namespace.c                          |   9 +-
>>>  gdb/cp-support.c                            |   7 +-
>>>  gdb/testsuite/gdb.cp/scope-err.cc           |  35 +++
>>>  gdb/testsuite/gdb.cp/scope-err.exp          |  47 ++++
>>>  gdb/testsuite/gdb.linespec/ls-errs.c        |  13 +-
>>>  gdb/testsuite/gdb.linespec/ls-errs.exp      | 384 +++++++++++++++-------------
>>>  gdb/testsuite/gdb.mi/mi-linespec-err-cp.cc  |  35 +++
>>>  gdb/testsuite/gdb.mi/mi-linespec-err-cp.exp |  57 +++++
>>>  8 files changed, 405 insertions(+), 182 deletions(-)
>>>
>>> diff --git a/gdb/cp-namespace.c b/gdb/cp-namespace.c
>>> index 72002d6..016a42f 100644
>>> --- a/gdb/cp-namespace.c
>>> +++ b/gdb/cp-namespace.c
>>> @@ -170,7 +170,7 @@ cp_lookup_bare_symbol (const struct language_defn *langdef,
>>>       ':' may be in the args of a template spec.  This isn't intended to be
>>>       a complete test, just cheap and documentary.  */
>>>    if (strchr (name, '<') == NULL && strchr (name, '(') == NULL)
>>> -    gdb_assert (strchr (name, ':') == NULL);
>>> +    gdb_assert (strstr (name, "::") == NULL);
>>>  
>>>    sym = lookup_symbol_in_static_block (name, block, domain);
>>>    if (sym.symbol != NULL)
>>> @@ -246,10 +246,9 @@ cp_search_static_and_baseclasses (const char *name,
>>>    struct block_symbol klass_sym;
>>>    struct type *klass_type;
>>>  
>>> -  /* The test here uses <= instead of < because Fortran also uses this,
>>> -     and the module.exp testcase will pass "modmany::" for NAME here.  */
>>> -  gdb_assert (prefix_len + 2 <= strlen (name));
>>> -  gdb_assert (name[prefix_len + 1] == ':');
>>> +  /* Check for malformed input.  */
>>> +  if (prefix_len + 2 > strlen (name) || name[prefix_len + 1] != ':')
>>> +    return null_block_symbol;
>>>  
>>>    /* Find the name of the class and the name of the method, variable, etc.  */
>>>  
>>> diff --git a/gdb/cp-support.c b/gdb/cp-support.c
>>> index df127c4..a71c6ad 100644
>>> --- a/gdb/cp-support.c
>>> +++ b/gdb/cp-support.c
>>> @@ -1037,8 +1037,13 @@ cp_find_first_component_aux (const char *name, int permissive)
>>>  	      return strlen (name);
>>>  	    }
>>>  	case '\0':
>>> -	case ':':
>>>  	  return index;
>>> +	case ':':
>>> +	  /* ':' marks a component iff the next character is also a ':'.
>>> +	     Otherwise it is probably malformed input.  */
>>> +	  if (name[index + 1] == ':')
>>> +	    return index;
>>> +	  break;
>>>  	case 'o':
>>>  	  /* Operator names can screw up the recursion.  */
>>>  	  if (operator_possible
>>> diff --git a/gdb/testsuite/gdb.cp/scope-err.cc b/gdb/testsuite/gdb.cp/scope-err.cc
>>> new file mode 100644
>>> index 0000000..19c92d8
>>> --- /dev/null
>>> +++ b/gdb/testsuite/gdb.cp/scope-err.cc
>>> @@ -0,0 +1,35 @@
>>> +/* This testcase is part of GDB, the GNU debugger.
>>> +
>>> +   Copyright 2016 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
>>> +   the Free Software Foundation; either version 3 of the License, or
>>> +   (at your option) any later version.
>>> +
>>> +   This program is distributed in the hope that it will be useful,
>>> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
>>> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>>> +   GNU General Public License for more details.
>>> +
>>> +   You should have received a copy of the GNU General Public License
>>> +   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
>>> +
>>> +int
>>> +myfunction (int aa)
>>> +{
>>> +  int i;
>>> +
>>> +  i = aa + 42;
>>> +  return i;    /* set breakpoint here */
>>> +}
>>> +
>>> +int
>>> +main (void)
>>> +{
>>> +  int a;
>>> +
>>> +  a = myfunction (a);
>>> +
>>> +  return a;
>>> +}
>>> diff --git a/gdb/testsuite/gdb.cp/scope-err.exp b/gdb/testsuite/gdb.cp/scope-err.exp
>>> new file mode 100644
>>> index 0000000..9d93578
>>> --- /dev/null
>>> +++ b/gdb/testsuite/gdb.cp/scope-err.exp
>>> @@ -0,0 +1,47 @@
>>> +# Copyright 2012-2016 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
>>> +# the Free Software Foundation; either version 3 of the License, or
>>> +# (at your option) any later version.
>>> +#
>>> +# This program is distributed in the hope that it will be useful,
>>> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
>>> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>>> +# GNU General Public License for more details.
>>> +#
>>> +# You should have received a copy of the GNU General Public License
>>> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
>>> +
>>> +# Tests for linespec errors with C++.
>>> +# Derived from gdb.linespec/ls-errs.exp.
>>> +
>>> +if { [skip_cplus_tests] } { continue }
>>> +
>>> +standard_testfile .cc
>>> +set exefile $testfile
>>> +
>>> +if {[prepare_for_testing $testfile $exefile $srcfile {debug c++}]} {
>>> +    return -1
>>> +}
>>> +
>>> +if ![runto_main] {
>>> +    fail "Can't run to main"
>>> +    return 0
>>> +}
>>> +
>>> +# Run to a location in the file.
>>> +set bp_location [gdb_get_line_number "set breakpoint here"]
>>> +
>>> +gdb_test "break $srcfile:$bp_location" \
>>> +    "Breakpoint.*at.* file .*$srcfile, line $bp_location\\." \
>>> +    "breakpoint line number in file"
>>> +
>>> +gdb_continue_to_breakpoint "$bp_location"
>>> +
>>> +# Try to access a variable using scope that is a non-existent filename
>>> +# with a Windows-style logical drive in the name.
>>> +set nonexistent_file C:/does/not/exist.cc
>>> +gdb_test "print '$nonexistent_file'::var" \
>>> +	 ".*No symbol \"$nonexistent_file\" in current context.*" \
>>> +	 "print var from \"$nonexistent_file\""
>>> diff --git a/gdb/testsuite/gdb.linespec/ls-errs.c b/gdb/testsuite/gdb.linespec/ls-errs.c
>>> index ca41342..a3a43db 100644
>>> --- a/gdb/testsuite/gdb.linespec/ls-errs.c
>>> +++ b/gdb/testsuite/gdb.linespec/ls-errs.c
>>> @@ -15,14 +15,21 @@
>>>     You should have received a copy of the GNU General Public License
>>>     along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
>>>  
>>> -int myfunction (void) { return 0; }
>>> +int
>>> +myfunction (int aa)
>>> +{
>>> +  int i;
>>> +
>>> +  i = aa + 42;
>>> +  return i;    /* set breakpoint here */
>>> +}
>>>  
>>>  int
>>>  main (void)
>>> -{  
>>> +{
>>>    int a;
>>>  
>>> -  a = myfunction ();
>>> +  a = myfunction (a);
>>>  
>>>   here:
>>>    return a;
>>> diff --git a/gdb/testsuite/gdb.linespec/ls-errs.exp b/gdb/testsuite/gdb.linespec/ls-errs.exp
>>> index 16d4574..35f8f78 100644
>>> --- a/gdb/testsuite/gdb.linespec/ls-errs.exp
>>> +++ b/gdb/testsuite/gdb.linespec/ls-errs.exp
>>> @@ -13,209 +13,247 @@
>>>  # You should have received a copy of the GNU General Public License
>>>  # along with this program.  If not, see <http://www.gnu.org/licenses/>.
>>>  
>>> -# Tests for linespec error conditions
>>> +# Tests for linespec errors with C and C++.
>>>  
>>> -standard_testfile
>>> -set exefile $testfile
>>> +# The test proper.  LANG is either C or C++.
>>>  
>>> -if {[prepare_for_testing $testfile $exefile $srcfile \
>>> -	 {debug nowarnings}]} {
>>> -    return -1
>>> -}
>>> +proc do_test {lang} {
>>> +    global testfile srcfile error_messages compiler_info
>>>  
>>> -# Turn off the pending breakpoint queries.
>>> -gdb_test_no_output "set breakpoint pending off"
>>> +    standard_testfile
>>> +    set exefile $testfile
>>> +    if [info exists compiler_info] {
>>> +	# Unsetting compiler_info allows us to switch compilers
>>> +	# used by prepare_for_testing.
>>> +        unset compiler_info
>>> +    }
>>> +    set options {debug}
>>>  
>>> -# Turn off completion limiting
>>> -gdb_test_no_output "set max-completions unlimited"
>>> +    if { $lang == "C++" } {
>>> +	if { [skip_cplus_tests] } { return 0 }
>>> +	# Build ".c" source file with g++.
>>> +	lappend options "c++"
>>> +    }
>>>  
>>> -# We intentionally do not use gdb_breakpoint for these tests.
>>> +    if {[prepare_for_testing $testfile $exefile $srcfile $options]} {
>>> +	return -1
>>> +    }
>>>  
>>> -# 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
>>> +    # Turn off the pending breakpoint queries.
>>> +    gdb_test_no_output "set breakpoint pending off"
>>>  
>>> -    gdb_test "break $linespec" [string_to_regexp \
>>> -				[eval format \$error_messages($msg_id) $args]]
>>> -}
>>> +    # Turn off completion limiting
>>> +    gdb_test_no_output "set max-completions unlimited"
>>>  
>>> -# Common error message format strings.
>>> -array set error_messages {
>>> -    invalid_file "No source file named %s."
>>> -    invalid_function "Function \"%s\" not defined."
>>> -    invalid_var_or_func "Undefined convenience variable or function \"%s\" not defined."
>>> -    invalid_function_f "Function \"%s\" not defined in \"%s\"."
>>> -    invalid_var_or_func_f \
>>> -	"Undefined convenience variable or function \"%s\" not defined in \"%s\"."
>>> -    invalid_label "No label \"%s\" defined in function \"%s\"."
>>> -    invalid_parm "invalid linespec argument, \"%s\""
>>> -    invalid_offset "No line %d in the current file."
>>> -    invalid_offset_f "No line %d in file \"%s\"."
>>> -    malformed_line_offset "malformed line offset: \"%s\""
>>> -    source_incomplete \
>>> -	"Source filename requires function, label, or line offset."
>>> -    unexpected "malformed linespec error: unexpected %s"
>>> -    unexpected_opt "malformed linespec error: unexpected %s, \"%s\""
>>> -    unmatched_quote "unmatched quote"
>>> -    garbage "Garbage '%s' at end of command"
>>> -}
>>> +    if ![runto_main] {
>>> +	fail "Can't run to main"
>>> +	return 0
>>> +    }
>>>  
>>> -# Some commonly used whitespace tests around ':'.
>>> -set spaces [list ":" ": " " :" " : " "\t:  " "  :\t" "\t:\t" " \t:\t " \
>>> -		"\t  \t:\t  \t  \t"]
>>> +    # Run to a location in the file.
>>> +    set bp_location [gdb_get_line_number "set breakpoint here"]
>>> +
>>> +    gdb_test "break $srcfile:$bp_location" \
>>> +    "Breakpoint.*at.* file .*$srcfile, line $bp_location\\." \
>>> +    "breakpoint line number in file"
>>> +
>>> +    gdb_continue_to_breakpoint "$bp_location"
>>> +
>>> +    # Common error message format strings.
>>> +    array set error_messages {
>>> +	invalid_file "No source file named %s."
>>> +	invalid_function "Function \"%s\" not defined."
>>> +	invalid_var_or_func 
>>> +	    "Undefined convenience variable or function \"%s\" not defined."
>>> +	invalid_function_f "Function \"%s\" not defined in \"%s\"."
>>> +	invalid_var_or_func_f \
>>> +	    "Undefined convenience variable or function \"%s\" not defined in \"%s\"."
>>> +	invalid_label "No label \"%s\" defined in function \"%s\"."
>>> +	invalid_parm "invalid linespec argument, \"%s\""
>>> +	invalid_offset "No line %d in the current file."
>>> +	invalid_offset_f "No line %d in file \"%s\"."
>>> +	malformed_line_offset "malformed line offset: \"%s\""
>>> +	source_incomplete \
>>> +	    "Source filename requires function, label, or line offset."
>>> +	unexpected "malformed linespec error: unexpected %s"
>>> +	unexpected_opt "malformed linespec error: unexpected %s, \"%s\""
>>> +	unmatched_quote "unmatched quote"
>>> +	garbage "Garbage '%s' at end of command"
>>> +    }
>>>  
>>> -# A list of invalid offsets.
>>> -set invalid_offsets [list -100 +500 1000]
>>> +    # We intentionally do not use gdb_breakpoint for these tests.
>>>  
>>> -# Try some simple, invalid linespecs involving spaces.
>>> -foreach x $spaces {
>>> -    test_break $x unexpected "colon"
>>> -}
>>> +    # 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
>>>  
>>> -# Test invalid filespecs starting with offset.  This is done
>>> -# first so that default offsets are tested.
>>> -foreach x $invalid_offsets {
>>> -    set offset $x
>>> -
>>> -    # Relative offsets are relative to line 16.  Adjust
>>> -    # expected offset from error message accordingly.
>>> -    if {[string index $x 0] == "+" ||
>>> -	[string index $x 0] == "-"} {
>>> -	incr offset 16
>>> +	gdb_test "break $linespec" [string_to_regexp \
>>> +				    [eval format \$error_messages($msg_id) \
>>> +				     $args]]
>>>      }
>>> -    test_break $x invalid_offset $offset
>>> -    test_break "-line $x" invalid_offset $offset
>>> -}
>>>  
>>> -# Test offsets with trailing tokens w/ and w/o spaces.
>>> -foreach x $spaces {
>>> -    test_break "3$x" unexpected "colon"
>>> -    test_break "+10$x" unexpected "colon"
>>> -    test_break "-10$x" unexpected "colon"
>>> -}
>>> +    # Some commonly used whitespace tests around ':'.
>>> +    set spaces [list ":" ": " " :" " : " "\t:  " "  :\t" "\t:\t" \
>>> +		     " \t:\t " "\t  \t:\t  \t  \t"]
>>>  
>>> -foreach x {1 +1 +100 -10} {
>>> -    test_break "3 $x" unexpected_opt "number" $x
>>> -    test_break "-line 3 $x" garbage $x
>>> -    test_break "+10 $x" unexpected_opt "number" $x
>>> -    test_break "-line +10 $x" garbage $x
>>> -    test_break "-10 $x" unexpected_opt "number" $x
>>> -    test_break "-line -10 $x" garbage $x
>>> -}
>>> +    # A list of invalid offsets.
>>> +    set invalid_offsets [list -100 +500 1000]
>>>  
>>> -foreach x {3 +10 -10} {
>>> -    test_break "$x foo" unexpected_opt "string" "foo"
>>> -    test_break "-line $x foo" garbage "foo"
>>> -}
>>> +    # Try some simple, invalid linespecs involving spaces.
>>> +    foreach x $spaces {
>>> +	test_break $x unexpected "colon"
>>> +    }
>>>  
>>> -# Test invalid linespecs starting with filename.
>>> -foreach x [list "this_file_doesn't_exist.c" \
>>> -	       "this file has spaces.c" \
>>> -	       "\"file::colons.c\"" \
>>> -	       "'file::colons.c'" \
>>> -	       "\"this \"file\" has quotes.c\"" \
>>> -	       "'this \"file\" has quotes.c'" \
>>> -	       "'this 'file' has quotes.c'" \
>>> -	       "\"this 'file' has quotes.c\"" \
>>> -	       "\"spaces: and :colons.c\"" \
>>> -	       "'more: :spaces: :and  colons::.c'"] {
>>> -    # Remove any quoting from FILENAME for the error message.
>>> -    test_break "$x:3" invalid_file [string trim $x \"']
>>> -}
>>> -foreach x [list "this_file_doesn't_exist.c" \
>>> -	       "file::colons.c" \
>>> -	       "'file::colons.c'"] {
>>> -    test_break "-source $x -line 3" \
>>> -	invalid_file [string trim $x \"']
>>> -}
>>> +    # Test invalid filespecs starting with offset.  This is done
>>> +    # first so that default offsets are tested.
>>> +    foreach x $invalid_offsets {
>>> +	set offset $x
>>> +
>>> +	# Relative offsets are relative to line 16.  Adjust
>>> +	# expected offset from error message accordingly.
>>> +	if {[string index $x 0] == "+" || [string index $x 0] == "-"} {
>>> + 	    incr offset 24
>>> +	}
>>> +	test_break $x invalid_offset $offset
>>> +	test_break "-line $x" invalid_offset $offset
>>> +    }
>>> +
>>> +    # Test offsets with trailing tokens w/ and w/o spaces.
>>> +    foreach x $spaces {
>>> +	test_break "3$x" unexpected "colon"
>>> +	test_break "+10$x" unexpected "colon"
>>> +	test_break "-10$x" unexpected "colon"
>>> +    }
>>> +
>>> +    foreach x {1 +1 +100 -10} {
>>> +	test_break "3 $x" unexpected_opt "number" $x
>>> +	test_break "-line 3 $x" garbage $x
>>> +	test_break "+10 $x" unexpected_opt "number" $x
>>> +	test_break "-line +10 $x" garbage $x
>>> +	test_break "-10 $x" unexpected_opt "number" $x
>>> +	test_break "-line -10 $x" garbage $x
>>> +    }
>>>  
>>> -# Test that option lexing stops at whitespace boundaries
>>> -test_break "-source this file has spaces.c -line 3" \
>>> -    invalid_file "this"
>>> +    foreach x {3 +10 -10} {
>>> +	test_break "$x foo" unexpected_opt "string" "foo"
>>> +	test_break "-line $x foo" garbage "foo"
>>> +    }
>>>  
>>> -test_break "-function function whitespace" \
>>> -    invalid_function "function"
>>> +    # Test invalid linespecs starting with filename.
>>> +    # It's OK to use the ".c" extension for the C++ test
>>> +    # since the extension doesn't affect GDB's lookup.
>>> +    set invalid_files [list "this_file_doesn't_exist.c" \
>>> +			    "this file has spaces.c" \
>>> +			    "\"file::colons.c\"" \
>>> +			    "'file::colons.c'" \
>>> +			    "\"this \"file\" has quotes.c\"" \
>>> +			    "'this \"file\" has quotes.c'" \
>>> +			    "'this 'file' has quotes.c'" \
>>> +			    "\"this 'file' has quotes.c\"" \
>>> +			    "\"spaces: and :colons.c\"" \
>>> +			    "'more: :spaces: :and  colons::.c'" \
>>> +			    "C:/nonexist-with-windrive.c"]
>>> +
>>> +    foreach x $invalid_files {
>>> +	# Remove any quoting from FILENAME for the error message.
>>> +	test_break "$x:3" invalid_file [string trim $x \"']
>>> +    }
>>> +    foreach x [list "this_file_doesn't_exist.c" \
>>> +		    "file::colons.c" \
>>> +		    "'file::colons.c'"] {
>>> +	test_break "-source $x -line 3" invalid_file [string trim $x \"']
>>> +    }
>>>  
>>> -test_break "-source $srcfile -function function whitespace" \
>>> -    invalid_function_f "function" $srcfile
>>> +    # Test that option lexing stops at whitespace boundaries
>>> +    test_break "-source this file has spaces.c -line 3" invalid_file "this"
>>> +    test_break "-function function whitespace" invalid_function "function"
>>> +    test_break "-source $srcfile -function function whitespace" \
>>> +	       invalid_function_f "function" $srcfile
>>>  
>>> -test_break "-function main -label label whitespace" \
>>> -    invalid_label "label" "main"
>>> +    test_break "-function main -label label whitespace" \
>>> +	       invalid_label "label" "main"
>>>  
>>> -# Test unmatched quotes.
>>> -foreach x {"\"src-file.c'" "'src-file.c"} {
>>> -    test_break "$x:3" unmatched_quote
>>> -}
>>> +    # Test unmatched quotes.
>>> +    foreach x {"\"src-file.c'" "'src-file.c"} {
>>> +	test_break "$x:3" unmatched_quote
>>> +    }
>>>  
>>> -test_break $srcfile invalid_function $srcfile
>>> -foreach x {"foo" " foo" " foo "} {
>>> -    # Trim any leading/trailing whitespace for error messages.
>>> -    test_break "$srcfile:$x" invalid_function_f [string trim $x] $srcfile
>>> -    test_break "-source $srcfile -function $x" \
>>> -	invalid_function_f [string trim $x] $srcfile
>>> -    test_break "$srcfile:main:$x" invalid_label [string trim $x] "main"
>>> -    test_break "-source $srcfile -function main -label $x" \
>>> -	invalid_label [string trim $x] "main"
>>> -}
>>> +    test_break $srcfile invalid_function $srcfile
>>> +    foreach x {"foo" " foo" " foo "} {
>>> +	# Trim any leading/trailing whitespace for error messages.
>>> +	test_break "$srcfile:$x" invalid_function_f [string trim $x] $srcfile
>>> +	test_break "-source $srcfile -function $x" \
>>> +		   invalid_function_f [string trim $x] $srcfile
>>> +	test_break "$srcfile:main:$x" invalid_label [string trim $x] "main"
>>> +	test_break "-source $srcfile -function main -label $x" \
>>> +		   invalid_label [string trim $x] "main"
>>> +    }
>>>  
>>> -foreach x $spaces {
>>> -    test_break "$srcfile$x" unexpected "end of input"
>>> -    test_break "$srcfile:main$x" unexpected "end of input"
>>> -}
>>> +    foreach x $spaces {
>>> +	test_break "$srcfile$x" unexpected "end of input"
>>> +	test_break "$srcfile:main$x" unexpected "end of input"
>>> +    }
>>>  
>>> -test_break "${srcfile}::" invalid_function "${srcfile}::"
>>> -test_break "$srcfile:3 1" unexpected_opt "number" "1"
>>> -test_break "-source $srcfile -line 3 1" garbage "1"
>>> -test_break "$srcfile:3 +100" unexpected_opt "number" "+100"
>>> -test_break "-source $srcfile -line 3 +100" garbage "+100"
>>> -test_break "$srcfile:3 -100" unexpected_opt "number" "-100"
>>> -test_break "$srcfile:3 foo" unexpected_opt "string" "foo"
>>> -test_break "-source $srcfile -line 3 foo" garbage "foo"
>>> -
>>> -foreach x $invalid_offsets {
>>> -    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_break "-source $srcfile -line $x" invalid_offset_f $x $srcfile
>>> -}
>>> -test_break "-source $srcfile -line -x" malformed_line_offset "-x"
>>> +    test_break "${srcfile}::" invalid_function "${srcfile}::"
>>> +    test_break "$srcfile:3 1" unexpected_opt "number" "1"
>>> +    test_break "-source $srcfile -line 3 1" garbage "1"
>>> +    test_break "$srcfile:3 +100" unexpected_opt "number" "+100"
>>> +    test_break "-source $srcfile -line 3 +100" garbage "+100"
>>> +    test_break "$srcfile:3 -100" unexpected_opt "number" "-100"
>>> +    test_break "$srcfile:3 foo" unexpected_opt "string" "foo"
>>> +    test_break "-source $srcfile -line 3 foo" garbage "foo"
>>> +
>>> +    foreach x $invalid_offsets {
>>> +	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_break "-source $srcfile -line $x" invalid_offset_f $x $srcfile
>>> +    }
>>> +    test_break "-source $srcfile -line -x" malformed_line_offset "-x"
>>>  
>>> -# Test invalid filespecs starting with function.
>>> -foreach x {"foobar" "foo::bar" "foo.bar" "foo ." "foo bar" "foo 1" \
>>> +    # 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"} {
>>> -    test_break $x invalid_function $x
>>> -    test_break "-function \"$x\"" invalid_function $x
>>> -}
>>> +	test_break $x invalid_function $x
>>> +	test_break "-function \"$x\"" invalid_function $x
>>> +    }
>>>  
>>> -foreach x $spaces {
>>> -    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"
>>> -}
>>> +    foreach x $spaces {
>>> +	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"
>>> +    }
>>>  
>>> -foreach x {"3" "+100" "-100" "foo"} {
>>> -    test_break "main 3" invalid_function "main 3"
>>> -    test_break "-function \"main $x\"" invalid_function "main $x"
>>> -    test_break "main:here $x" invalid_label "here $x" "main"
>>> -    test_break "-function main -label \"here $x\"" \
>>> -	invalid_label "here $x" "main"
>>> -}
>>> +    foreach x {"3" "+100" "-100" "foo"} {
>>> +	test_break "main 3" invalid_function "main 3"
>>> +	test_break "-function \"main $x\"" invalid_function "main $x"
>>> +	test_break "main:here $x" invalid_label "here $x" "main"
>>> +	test_break "-function main -label \"here $x\"" \
>>> +		   invalid_label "here $x" "main"
>>> +    }
>>>  
>>> -foreach x {"if" "task" "thread"} {
>>> -    test_break $x invalid_function $x
>>> -}
>>> +    foreach x {"if" "task" "thread"} {
>>> +	test_break $x invalid_function $x
>>> +    }
>>>  
>>> -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_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}
>>> -test_break $x invalid_var_or_func $x
>>> -test_break "$srcfile:$x" invalid_var_or_func_f $x $srcfile
>>> +    # Test undefined convenience variables.
>>> +    set x {$zippo}
>>> +    test_break $x invalid_var_or_func $x
>>> +    test_break "$srcfile:$x" invalid_var_or_func_f $x $srcfile
>>>  
>>> -# Explicit linespec-specific tests
>>> -test_break "-source $srcfile" source_incomplete
>>> +    # Explicit linespec-specific tests
>>> +    test_break "-source $srcfile" source_incomplete
>>> +}
>>> +
>>> +foreach_with_prefix lang {"C" "C++"} {
>>> +    do_test ${lang}
>>> +}
>>> diff --git a/gdb/testsuite/gdb.mi/mi-linespec-err-cp.cc b/gdb/testsuite/gdb.mi/mi-linespec-err-cp.cc
>>> new file mode 100644
>>> index 0000000..19c92d8
>>> --- /dev/null
>>> +++ b/gdb/testsuite/gdb.mi/mi-linespec-err-cp.cc
>>> @@ -0,0 +1,35 @@
>>> +/* This testcase is part of GDB, the GNU debugger.
>>> +
>>> +   Copyright 2016 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
>>> +   the Free Software Foundation; either version 3 of the License, or
>>> +   (at your option) any later version.
>>> +
>>> +   This program is distributed in the hope that it will be useful,
>>> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
>>> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>>> +   GNU General Public License for more details.
>>> +
>>> +   You should have received a copy of the GNU General Public License
>>> +   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
>>> +
>>> +int
>>> +myfunction (int aa)
>>> +{
>>> +  int i;
>>> +
>>> +  i = aa + 42;
>>> +  return i;    /* set breakpoint here */
>>> +}
>>> +
>>> +int
>>> +main (void)
>>> +{
>>> +  int a;
>>> +
>>> +  a = myfunction (a);
>>> +
>>> +  return a;
>>> +}
>>> diff --git a/gdb/testsuite/gdb.mi/mi-linespec-err-cp.exp b/gdb/testsuite/gdb.mi/mi-linespec-err-cp.exp
>>> new file mode 100644
>>> index 0000000..1a8682e
>>> --- /dev/null
>>> +++ b/gdb/testsuite/gdb.mi/mi-linespec-err-cp.exp
>>> @@ -0,0 +1,57 @@
>>> +# Copyright 2016 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
>>> +# the Free Software Foundation; either version 3 of the License, or
>>> +# (at your option) any later version.
>>> +#
>>> +# This program is distributed in the hope that it will be useful,
>>> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
>>> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>>> +# GNU General Public License for more details.
>>> +#
>>> +# You should have received a copy of the GNU General Public License
>>> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
>>> +
>>> +# Regression test for PR breakpoints/18303.  Tests that the correct
>>> +# errors is generated when setting a breakpoint in a non-existent
>>> +# file with a Windows-style logical drive names and C++.
>>> +
>>> +if { [skip_cplus_tests] } { continue }
>>> +
>>> +load_lib mi-support.exp
>>> +set MIFLAGS "-i=mi"
>>> +
>>> +standard_testfile .cc
>>> +set exefile $testfile
>>> +
>>> +if {[prepare_for_testing $testfile $exefile $srcfile {debug c++}]} {
>>> +    return -1
>>> +}
>>> +
>>> +gdb_exit
>>> +if [mi_gdb_start] {
>>> +    continue
>>> +}
>>> +
>>> +# Turn off the pending breakpoint queries.
>>> +mi_gdb_test "-interpreter-exec console \"set breakpoint pending off\"" \
>>> +  {=cmd-param-changed,param=\"breakpoint pending\",.*\^done} \
>>> +  "-interpreter-exec console \"set breakpoint pending off\""
>>> +
>>> +mi_run_to_main
>>> +
>>> +# Run to a location in the file.
>>> +set bp_location [gdb_get_line_number "set breakpoint here"]
>>> +
>>> +mi_gdb_test "-break-insert ${srcfile}:${bp_location}" \
>>> +    {\^done,bkpt=.number="2",type="breakpoint".*\}} "set breakpoint"
>>> +
>>> +mi_execute_to "exec-continue" "breakpoint-hit" "myfunction" ".*" ".*" "24" \
>>> +    { "" "disp=\"keep\"" } "breakpoint hit"
>>> +
>>> +# Set a breakpoint in a C++ source file whose name contains a
>>> +# Windows-style logical drive.
>>> +mi_gdb_test \
>>> +    "-break-insert -f \"c:/uu.cpp:13\"" \
>>> +    ".*No source file named c:/uu.cpp.*"
>>>
>> Ping?
>> thanks
>> --Don
>>
> 

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

* Re: [PING] Re: [PATCH v4] PR 18303, Tolerate malformed input for lookup_symbol-called functions
  2016-02-25 17:28                   ` [PING] Re: [PATCH v4] PR 18303, Tolerate malformed input for lookup_symbol-called functions Don Breazeal
@ 2016-03-03 18:19                     ` Don Breazeal
  2016-03-14 21:23                       ` [PING] Re: [PATCH v4] PR 18303, Tolerate malformed input for lookup_symbol-called functions (was: Re: [PATCH] Fix problem handling colon in linespec, PR breakpoints/18303) Don Breazeal
  0 siblings, 1 reply; 17+ messages in thread
From: Don Breazeal @ 2016-03-03 18:19 UTC (permalink / raw)
  To: gdb-patches

Ping.
I checked, the patch still applies cleanly to mainline.
Thanks
--Don

On 2/25/2016 9:27 AM, Don Breazeal wrote:
> Ping
> Thanks
> --Don
> 
> On 2/18/2016 10:13 AM, Don Breazeal wrote:
>> Ping.
>> Current status of this one:
>>  - Doug approved (at least in concept) Keith's fix below.
>>  - Pedro requested that the new C++ linespec error test be consolidated
>> with the existing C version of the test.  That's done in this version.
>> Thanks!
>> --Don
>>
>> On 2/4/2016 10:37 AM, Don Breazeal wrote:
>>> On 1/28/2016 2:52 PM, Don Breazeal wrote:
>>>> Apologies -- the previous version of this patch was missing one file,
>>>> added here.  Sorry for the noise.
>>>> --Don
>>>>
>>>> ---V3 Comment---
>>>> This patch differs from v2 in that the new test gdb.linespec/ls-errs-cp.exp
>>>> is gone, and instead (per Pedro's suggestion) gdb.linespec/ls-errs.exp has
>>>> been modified to test both C & C++, and to incorporate the small amount of
>>>> extra testing from ls-errs-cp.exp.
>>>>
>>>> It also includes, unchanged from the previous version of the patch:
>>>>
>>>>  * an updated version of the alternate fix that Keith proposed for my
>>>>    patch that addressed PR breakpoints/18303.  Doug has approved (in
>>>>    concept, at least) the code portion of the patch.
>>>>
>>>>  * a couple of other new tests of colons in linespecs.
>>>>
>>>> Thanks,
>>>> --Don
>>>>
>>>> -----------
>>>> lookup_symbol is often called with user input.  Consequently, any
>>>> function called from lookup_symbol{,_in_language} should attempt to
>>>> deal with malformed input gracefully.  After all, malformed user
>>>> input is not a programming/API error.
>>>>
>>>> This patch does not attempt to find/correct all instances of this.  It
>>>> only fixes locations in the code that trigger test suite failures.
>>>>
>>>> This patch fixes PR breakpoints/18303, "Assertion: -breakpoint-insert
>>>> with windows paths of file in non-current directory".
>>>>
>>>> The patch includes three new tests related to this.  One is just
>>>> gdb.linespec/ls-errs.exp copied and converted to use C++ instead of C, and
>>>> to add a case using a file name containing a Windows-style logical drive
>>>> specifier.  The others include an MI test to provide a regression test for
>>>> the specific case reported in PR 18303, and a C++ test for proper error
>>>> handling of access to a program variable when using a file scope specifier
>>>> that refers to a non-existent file.
>>>>
>>>> Tested on x86_64 native Linux.
>>>>
>>>> gdb/ChangeLog
>>>> 2016-01-28  Keith Seitz  <keiths@redhat.com>
>>>>
>>>> 	PR breakpoints/18303
>>>> 	* cp-namespace.c (cp_lookup_bare_symbol): Change assertion to
>>>> 	look for "::" instead of simply ":".
>>>> 	(cp_search_static_and_baseclasses): Return null_block_symbol for
>>>> 	malformed input.
>>>> 	Remove assertions.
>>>> 	* cp-support.c (cp_find_first_component_aux): Do not return
>>>> 	a prefix length for ':' unless the next character is also ':'.
>>>>
>>>> gdb/testsuite/ChangeLog
>>>> 2016-01-28  Don Breazeal  <donb@codesourcery.com>
>>>>
>>>> 	* gdb.cp/scope-err.cc: New test program.
>>>> 	* gdb.cp/scope-err.exp: New test script.
>>>> 	* gdb.linespec/ls-errs.c (myfunction): Expanded to have multiple
>>>> 	lines and "set breakpoint here" comment.
>>>> 	* gdb.linespec/ls-errs.exp: Added C++ testing and new test case.
>>>> 	* gdb.mi/mi-linespec-err-cp.cc: New test program.
>>>> 	* gdb.mi/mi-linespec-err-cp.exp: New test script.
>>>>
>>>> ---
>>>>  gdb/cp-namespace.c                          |   9 +-
>>>>  gdb/cp-support.c                            |   7 +-
>>>>  gdb/testsuite/gdb.cp/scope-err.cc           |  35 +++
>>>>  gdb/testsuite/gdb.cp/scope-err.exp          |  47 ++++
>>>>  gdb/testsuite/gdb.linespec/ls-errs.c        |  13 +-
>>>>  gdb/testsuite/gdb.linespec/ls-errs.exp      | 384 +++++++++++++++-------------
>>>>  gdb/testsuite/gdb.mi/mi-linespec-err-cp.cc  |  35 +++
>>>>  gdb/testsuite/gdb.mi/mi-linespec-err-cp.exp |  57 +++++
>>>>  8 files changed, 405 insertions(+), 182 deletions(-)
>>>>
>>>> diff --git a/gdb/cp-namespace.c b/gdb/cp-namespace.c
>>>> index 72002d6..016a42f 100644
>>>> --- a/gdb/cp-namespace.c
>>>> +++ b/gdb/cp-namespace.c
>>>> @@ -170,7 +170,7 @@ cp_lookup_bare_symbol (const struct language_defn *langdef,
>>>>       ':' may be in the args of a template spec.  This isn't intended to be
>>>>       a complete test, just cheap and documentary.  */
>>>>    if (strchr (name, '<') == NULL && strchr (name, '(') == NULL)
>>>> -    gdb_assert (strchr (name, ':') == NULL);
>>>> +    gdb_assert (strstr (name, "::") == NULL);
>>>>  
>>>>    sym = lookup_symbol_in_static_block (name, block, domain);
>>>>    if (sym.symbol != NULL)
>>>> @@ -246,10 +246,9 @@ cp_search_static_and_baseclasses (const char *name,
>>>>    struct block_symbol klass_sym;
>>>>    struct type *klass_type;
>>>>  
>>>> -  /* The test here uses <= instead of < because Fortran also uses this,
>>>> -     and the module.exp testcase will pass "modmany::" for NAME here.  */
>>>> -  gdb_assert (prefix_len + 2 <= strlen (name));
>>>> -  gdb_assert (name[prefix_len + 1] == ':');
>>>> +  /* Check for malformed input.  */
>>>> +  if (prefix_len + 2 > strlen (name) || name[prefix_len + 1] != ':')
>>>> +    return null_block_symbol;
>>>>  
>>>>    /* Find the name of the class and the name of the method, variable, etc.  */
>>>>  
>>>> diff --git a/gdb/cp-support.c b/gdb/cp-support.c
>>>> index df127c4..a71c6ad 100644
>>>> --- a/gdb/cp-support.c
>>>> +++ b/gdb/cp-support.c
>>>> @@ -1037,8 +1037,13 @@ cp_find_first_component_aux (const char *name, int permissive)
>>>>  	      return strlen (name);
>>>>  	    }
>>>>  	case '\0':
>>>> -	case ':':
>>>>  	  return index;
>>>> +	case ':':
>>>> +	  /* ':' marks a component iff the next character is also a ':'.
>>>> +	     Otherwise it is probably malformed input.  */
>>>> +	  if (name[index + 1] == ':')
>>>> +	    return index;
>>>> +	  break;
>>>>  	case 'o':
>>>>  	  /* Operator names can screw up the recursion.  */
>>>>  	  if (operator_possible
>>>> diff --git a/gdb/testsuite/gdb.cp/scope-err.cc b/gdb/testsuite/gdb.cp/scope-err.cc
>>>> new file mode 100644
>>>> index 0000000..19c92d8
>>>> --- /dev/null
>>>> +++ b/gdb/testsuite/gdb.cp/scope-err.cc
>>>> @@ -0,0 +1,35 @@
>>>> +/* This testcase is part of GDB, the GNU debugger.
>>>> +
>>>> +   Copyright 2016 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
>>>> +   the Free Software Foundation; either version 3 of the License, or
>>>> +   (at your option) any later version.
>>>> +
>>>> +   This program is distributed in the hope that it will be useful,
>>>> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
>>>> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>>>> +   GNU General Public License for more details.
>>>> +
>>>> +   You should have received a copy of the GNU General Public License
>>>> +   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
>>>> +
>>>> +int
>>>> +myfunction (int aa)
>>>> +{
>>>> +  int i;
>>>> +
>>>> +  i = aa + 42;
>>>> +  return i;    /* set breakpoint here */
>>>> +}
>>>> +
>>>> +int
>>>> +main (void)
>>>> +{
>>>> +  int a;
>>>> +
>>>> +  a = myfunction (a);
>>>> +
>>>> +  return a;
>>>> +}
>>>> diff --git a/gdb/testsuite/gdb.cp/scope-err.exp b/gdb/testsuite/gdb.cp/scope-err.exp
>>>> new file mode 100644
>>>> index 0000000..9d93578
>>>> --- /dev/null
>>>> +++ b/gdb/testsuite/gdb.cp/scope-err.exp
>>>> @@ -0,0 +1,47 @@
>>>> +# Copyright 2012-2016 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
>>>> +# the Free Software Foundation; either version 3 of the License, or
>>>> +# (at your option) any later version.
>>>> +#
>>>> +# This program is distributed in the hope that it will be useful,
>>>> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
>>>> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>>>> +# GNU General Public License for more details.
>>>> +#
>>>> +# You should have received a copy of the GNU General Public License
>>>> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
>>>> +
>>>> +# Tests for linespec errors with C++.
>>>> +# Derived from gdb.linespec/ls-errs.exp.
>>>> +
>>>> +if { [skip_cplus_tests] } { continue }
>>>> +
>>>> +standard_testfile .cc
>>>> +set exefile $testfile
>>>> +
>>>> +if {[prepare_for_testing $testfile $exefile $srcfile {debug c++}]} {
>>>> +    return -1
>>>> +}
>>>> +
>>>> +if ![runto_main] {
>>>> +    fail "Can't run to main"
>>>> +    return 0
>>>> +}
>>>> +
>>>> +# Run to a location in the file.
>>>> +set bp_location [gdb_get_line_number "set breakpoint here"]
>>>> +
>>>> +gdb_test "break $srcfile:$bp_location" \
>>>> +    "Breakpoint.*at.* file .*$srcfile, line $bp_location\\." \
>>>> +    "breakpoint line number in file"
>>>> +
>>>> +gdb_continue_to_breakpoint "$bp_location"
>>>> +
>>>> +# Try to access a variable using scope that is a non-existent filename
>>>> +# with a Windows-style logical drive in the name.
>>>> +set nonexistent_file C:/does/not/exist.cc
>>>> +gdb_test "print '$nonexistent_file'::var" \
>>>> +	 ".*No symbol \"$nonexistent_file\" in current context.*" \
>>>> +	 "print var from \"$nonexistent_file\""
>>>> diff --git a/gdb/testsuite/gdb.linespec/ls-errs.c b/gdb/testsuite/gdb.linespec/ls-errs.c
>>>> index ca41342..a3a43db 100644
>>>> --- a/gdb/testsuite/gdb.linespec/ls-errs.c
>>>> +++ b/gdb/testsuite/gdb.linespec/ls-errs.c
>>>> @@ -15,14 +15,21 @@
>>>>     You should have received a copy of the GNU General Public License
>>>>     along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
>>>>  
>>>> -int myfunction (void) { return 0; }
>>>> +int
>>>> +myfunction (int aa)
>>>> +{
>>>> +  int i;
>>>> +
>>>> +  i = aa + 42;
>>>> +  return i;    /* set breakpoint here */
>>>> +}
>>>>  
>>>>  int
>>>>  main (void)
>>>> -{  
>>>> +{
>>>>    int a;
>>>>  
>>>> -  a = myfunction ();
>>>> +  a = myfunction (a);
>>>>  
>>>>   here:
>>>>    return a;
>>>> diff --git a/gdb/testsuite/gdb.linespec/ls-errs.exp b/gdb/testsuite/gdb.linespec/ls-errs.exp
>>>> index 16d4574..35f8f78 100644
>>>> --- a/gdb/testsuite/gdb.linespec/ls-errs.exp
>>>> +++ b/gdb/testsuite/gdb.linespec/ls-errs.exp
>>>> @@ -13,209 +13,247 @@
>>>>  # You should have received a copy of the GNU General Public License
>>>>  # along with this program.  If not, see <http://www.gnu.org/licenses/>.
>>>>  
>>>> -# Tests for linespec error conditions
>>>> +# Tests for linespec errors with C and C++.
>>>>  
>>>> -standard_testfile
>>>> -set exefile $testfile
>>>> +# The test proper.  LANG is either C or C++.
>>>>  
>>>> -if {[prepare_for_testing $testfile $exefile $srcfile \
>>>> -	 {debug nowarnings}]} {
>>>> -    return -1
>>>> -}
>>>> +proc do_test {lang} {
>>>> +    global testfile srcfile error_messages compiler_info
>>>>  
>>>> -# Turn off the pending breakpoint queries.
>>>> -gdb_test_no_output "set breakpoint pending off"
>>>> +    standard_testfile
>>>> +    set exefile $testfile
>>>> +    if [info exists compiler_info] {
>>>> +	# Unsetting compiler_info allows us to switch compilers
>>>> +	# used by prepare_for_testing.
>>>> +        unset compiler_info
>>>> +    }
>>>> +    set options {debug}
>>>>  
>>>> -# Turn off completion limiting
>>>> -gdb_test_no_output "set max-completions unlimited"
>>>> +    if { $lang == "C++" } {
>>>> +	if { [skip_cplus_tests] } { return 0 }
>>>> +	# Build ".c" source file with g++.
>>>> +	lappend options "c++"
>>>> +    }
>>>>  
>>>> -# We intentionally do not use gdb_breakpoint for these tests.
>>>> +    if {[prepare_for_testing $testfile $exefile $srcfile $options]} {
>>>> +	return -1
>>>> +    }
>>>>  
>>>> -# 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
>>>> +    # Turn off the pending breakpoint queries.
>>>> +    gdb_test_no_output "set breakpoint pending off"
>>>>  
>>>> -    gdb_test "break $linespec" [string_to_regexp \
>>>> -				[eval format \$error_messages($msg_id) $args]]
>>>> -}
>>>> +    # Turn off completion limiting
>>>> +    gdb_test_no_output "set max-completions unlimited"
>>>>  
>>>> -# Common error message format strings.
>>>> -array set error_messages {
>>>> -    invalid_file "No source file named %s."
>>>> -    invalid_function "Function \"%s\" not defined."
>>>> -    invalid_var_or_func "Undefined convenience variable or function \"%s\" not defined."
>>>> -    invalid_function_f "Function \"%s\" not defined in \"%s\"."
>>>> -    invalid_var_or_func_f \
>>>> -	"Undefined convenience variable or function \"%s\" not defined in \"%s\"."
>>>> -    invalid_label "No label \"%s\" defined in function \"%s\"."
>>>> -    invalid_parm "invalid linespec argument, \"%s\""
>>>> -    invalid_offset "No line %d in the current file."
>>>> -    invalid_offset_f "No line %d in file \"%s\"."
>>>> -    malformed_line_offset "malformed line offset: \"%s\""
>>>> -    source_incomplete \
>>>> -	"Source filename requires function, label, or line offset."
>>>> -    unexpected "malformed linespec error: unexpected %s"
>>>> -    unexpected_opt "malformed linespec error: unexpected %s, \"%s\""
>>>> -    unmatched_quote "unmatched quote"
>>>> -    garbage "Garbage '%s' at end of command"
>>>> -}
>>>> +    if ![runto_main] {
>>>> +	fail "Can't run to main"
>>>> +	return 0
>>>> +    }
>>>>  
>>>> -# Some commonly used whitespace tests around ':'.
>>>> -set spaces [list ":" ": " " :" " : " "\t:  " "  :\t" "\t:\t" " \t:\t " \
>>>> -		"\t  \t:\t  \t  \t"]
>>>> +    # Run to a location in the file.
>>>> +    set bp_location [gdb_get_line_number "set breakpoint here"]
>>>> +
>>>> +    gdb_test "break $srcfile:$bp_location" \
>>>> +    "Breakpoint.*at.* file .*$srcfile, line $bp_location\\." \
>>>> +    "breakpoint line number in file"
>>>> +
>>>> +    gdb_continue_to_breakpoint "$bp_location"
>>>> +
>>>> +    # Common error message format strings.
>>>> +    array set error_messages {
>>>> +	invalid_file "No source file named %s."
>>>> +	invalid_function "Function \"%s\" not defined."
>>>> +	invalid_var_or_func 
>>>> +	    "Undefined convenience variable or function \"%s\" not defined."
>>>> +	invalid_function_f "Function \"%s\" not defined in \"%s\"."
>>>> +	invalid_var_or_func_f \
>>>> +	    "Undefined convenience variable or function \"%s\" not defined in \"%s\"."
>>>> +	invalid_label "No label \"%s\" defined in function \"%s\"."
>>>> +	invalid_parm "invalid linespec argument, \"%s\""
>>>> +	invalid_offset "No line %d in the current file."
>>>> +	invalid_offset_f "No line %d in file \"%s\"."
>>>> +	malformed_line_offset "malformed line offset: \"%s\""
>>>> +	source_incomplete \
>>>> +	    "Source filename requires function, label, or line offset."
>>>> +	unexpected "malformed linespec error: unexpected %s"
>>>> +	unexpected_opt "malformed linespec error: unexpected %s, \"%s\""
>>>> +	unmatched_quote "unmatched quote"
>>>> +	garbage "Garbage '%s' at end of command"
>>>> +    }
>>>>  
>>>> -# A list of invalid offsets.
>>>> -set invalid_offsets [list -100 +500 1000]
>>>> +    # We intentionally do not use gdb_breakpoint for these tests.
>>>>  
>>>> -# Try some simple, invalid linespecs involving spaces.
>>>> -foreach x $spaces {
>>>> -    test_break $x unexpected "colon"
>>>> -}
>>>> +    # 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
>>>>  
>>>> -# Test invalid filespecs starting with offset.  This is done
>>>> -# first so that default offsets are tested.
>>>> -foreach x $invalid_offsets {
>>>> -    set offset $x
>>>> -
>>>> -    # Relative offsets are relative to line 16.  Adjust
>>>> -    # expected offset from error message accordingly.
>>>> -    if {[string index $x 0] == "+" ||
>>>> -	[string index $x 0] == "-"} {
>>>> -	incr offset 16
>>>> +	gdb_test "break $linespec" [string_to_regexp \
>>>> +				    [eval format \$error_messages($msg_id) \
>>>> +				     $args]]
>>>>      }
>>>> -    test_break $x invalid_offset $offset
>>>> -    test_break "-line $x" invalid_offset $offset
>>>> -}
>>>>  
>>>> -# Test offsets with trailing tokens w/ and w/o spaces.
>>>> -foreach x $spaces {
>>>> -    test_break "3$x" unexpected "colon"
>>>> -    test_break "+10$x" unexpected "colon"
>>>> -    test_break "-10$x" unexpected "colon"
>>>> -}
>>>> +    # Some commonly used whitespace tests around ':'.
>>>> +    set spaces [list ":" ": " " :" " : " "\t:  " "  :\t" "\t:\t" \
>>>> +		     " \t:\t " "\t  \t:\t  \t  \t"]
>>>>  
>>>> -foreach x {1 +1 +100 -10} {
>>>> -    test_break "3 $x" unexpected_opt "number" $x
>>>> -    test_break "-line 3 $x" garbage $x
>>>> -    test_break "+10 $x" unexpected_opt "number" $x
>>>> -    test_break "-line +10 $x" garbage $x
>>>> -    test_break "-10 $x" unexpected_opt "number" $x
>>>> -    test_break "-line -10 $x" garbage $x
>>>> -}
>>>> +    # A list of invalid offsets.
>>>> +    set invalid_offsets [list -100 +500 1000]
>>>>  
>>>> -foreach x {3 +10 -10} {
>>>> -    test_break "$x foo" unexpected_opt "string" "foo"
>>>> -    test_break "-line $x foo" garbage "foo"
>>>> -}
>>>> +    # Try some simple, invalid linespecs involving spaces.
>>>> +    foreach x $spaces {
>>>> +	test_break $x unexpected "colon"
>>>> +    }
>>>>  
>>>> -# Test invalid linespecs starting with filename.
>>>> -foreach x [list "this_file_doesn't_exist.c" \
>>>> -	       "this file has spaces.c" \
>>>> -	       "\"file::colons.c\"" \
>>>> -	       "'file::colons.c'" \
>>>> -	       "\"this \"file\" has quotes.c\"" \
>>>> -	       "'this \"file\" has quotes.c'" \
>>>> -	       "'this 'file' has quotes.c'" \
>>>> -	       "\"this 'file' has quotes.c\"" \
>>>> -	       "\"spaces: and :colons.c\"" \
>>>> -	       "'more: :spaces: :and  colons::.c'"] {
>>>> -    # Remove any quoting from FILENAME for the error message.
>>>> -    test_break "$x:3" invalid_file [string trim $x \"']
>>>> -}
>>>> -foreach x [list "this_file_doesn't_exist.c" \
>>>> -	       "file::colons.c" \
>>>> -	       "'file::colons.c'"] {
>>>> -    test_break "-source $x -line 3" \
>>>> -	invalid_file [string trim $x \"']
>>>> -}
>>>> +    # Test invalid filespecs starting with offset.  This is done
>>>> +    # first so that default offsets are tested.
>>>> +    foreach x $invalid_offsets {
>>>> +	set offset $x
>>>> +
>>>> +	# Relative offsets are relative to line 16.  Adjust
>>>> +	# expected offset from error message accordingly.
>>>> +	if {[string index $x 0] == "+" || [string index $x 0] == "-"} {
>>>> + 	    incr offset 24
>>>> +	}
>>>> +	test_break $x invalid_offset $offset
>>>> +	test_break "-line $x" invalid_offset $offset
>>>> +    }
>>>> +
>>>> +    # Test offsets with trailing tokens w/ and w/o spaces.
>>>> +    foreach x $spaces {
>>>> +	test_break "3$x" unexpected "colon"
>>>> +	test_break "+10$x" unexpected "colon"
>>>> +	test_break "-10$x" unexpected "colon"
>>>> +    }
>>>> +
>>>> +    foreach x {1 +1 +100 -10} {
>>>> +	test_break "3 $x" unexpected_opt "number" $x
>>>> +	test_break "-line 3 $x" garbage $x
>>>> +	test_break "+10 $x" unexpected_opt "number" $x
>>>> +	test_break "-line +10 $x" garbage $x
>>>> +	test_break "-10 $x" unexpected_opt "number" $x
>>>> +	test_break "-line -10 $x" garbage $x
>>>> +    }
>>>>  
>>>> -# Test that option lexing stops at whitespace boundaries
>>>> -test_break "-source this file has spaces.c -line 3" \
>>>> -    invalid_file "this"
>>>> +    foreach x {3 +10 -10} {
>>>> +	test_break "$x foo" unexpected_opt "string" "foo"
>>>> +	test_break "-line $x foo" garbage "foo"
>>>> +    }
>>>>  
>>>> -test_break "-function function whitespace" \
>>>> -    invalid_function "function"
>>>> +    # Test invalid linespecs starting with filename.
>>>> +    # It's OK to use the ".c" extension for the C++ test
>>>> +    # since the extension doesn't affect GDB's lookup.
>>>> +    set invalid_files [list "this_file_doesn't_exist.c" \
>>>> +			    "this file has spaces.c" \
>>>> +			    "\"file::colons.c\"" \
>>>> +			    "'file::colons.c'" \
>>>> +			    "\"this \"file\" has quotes.c\"" \
>>>> +			    "'this \"file\" has quotes.c'" \
>>>> +			    "'this 'file' has quotes.c'" \
>>>> +			    "\"this 'file' has quotes.c\"" \
>>>> +			    "\"spaces: and :colons.c\"" \
>>>> +			    "'more: :spaces: :and  colons::.c'" \
>>>> +			    "C:/nonexist-with-windrive.c"]
>>>> +
>>>> +    foreach x $invalid_files {
>>>> +	# Remove any quoting from FILENAME for the error message.
>>>> +	test_break "$x:3" invalid_file [string trim $x \"']
>>>> +    }
>>>> +    foreach x [list "this_file_doesn't_exist.c" \
>>>> +		    "file::colons.c" \
>>>> +		    "'file::colons.c'"] {
>>>> +	test_break "-source $x -line 3" invalid_file [string trim $x \"']
>>>> +    }
>>>>  
>>>> -test_break "-source $srcfile -function function whitespace" \
>>>> -    invalid_function_f "function" $srcfile
>>>> +    # Test that option lexing stops at whitespace boundaries
>>>> +    test_break "-source this file has spaces.c -line 3" invalid_file "this"
>>>> +    test_break "-function function whitespace" invalid_function "function"
>>>> +    test_break "-source $srcfile -function function whitespace" \
>>>> +	       invalid_function_f "function" $srcfile
>>>>  
>>>> -test_break "-function main -label label whitespace" \
>>>> -    invalid_label "label" "main"
>>>> +    test_break "-function main -label label whitespace" \
>>>> +	       invalid_label "label" "main"
>>>>  
>>>> -# Test unmatched quotes.
>>>> -foreach x {"\"src-file.c'" "'src-file.c"} {
>>>> -    test_break "$x:3" unmatched_quote
>>>> -}
>>>> +    # Test unmatched quotes.
>>>> +    foreach x {"\"src-file.c'" "'src-file.c"} {
>>>> +	test_break "$x:3" unmatched_quote
>>>> +    }
>>>>  
>>>> -test_break $srcfile invalid_function $srcfile
>>>> -foreach x {"foo" " foo" " foo "} {
>>>> -    # Trim any leading/trailing whitespace for error messages.
>>>> -    test_break "$srcfile:$x" invalid_function_f [string trim $x] $srcfile
>>>> -    test_break "-source $srcfile -function $x" \
>>>> -	invalid_function_f [string trim $x] $srcfile
>>>> -    test_break "$srcfile:main:$x" invalid_label [string trim $x] "main"
>>>> -    test_break "-source $srcfile -function main -label $x" \
>>>> -	invalid_label [string trim $x] "main"
>>>> -}
>>>> +    test_break $srcfile invalid_function $srcfile
>>>> +    foreach x {"foo" " foo" " foo "} {
>>>> +	# Trim any leading/trailing whitespace for error messages.
>>>> +	test_break "$srcfile:$x" invalid_function_f [string trim $x] $srcfile
>>>> +	test_break "-source $srcfile -function $x" \
>>>> +		   invalid_function_f [string trim $x] $srcfile
>>>> +	test_break "$srcfile:main:$x" invalid_label [string trim $x] "main"
>>>> +	test_break "-source $srcfile -function main -label $x" \
>>>> +		   invalid_label [string trim $x] "main"
>>>> +    }
>>>>  
>>>> -foreach x $spaces {
>>>> -    test_break "$srcfile$x" unexpected "end of input"
>>>> -    test_break "$srcfile:main$x" unexpected "end of input"
>>>> -}
>>>> +    foreach x $spaces {
>>>> +	test_break "$srcfile$x" unexpected "end of input"
>>>> +	test_break "$srcfile:main$x" unexpected "end of input"
>>>> +    }
>>>>  
>>>> -test_break "${srcfile}::" invalid_function "${srcfile}::"
>>>> -test_break "$srcfile:3 1" unexpected_opt "number" "1"
>>>> -test_break "-source $srcfile -line 3 1" garbage "1"
>>>> -test_break "$srcfile:3 +100" unexpected_opt "number" "+100"
>>>> -test_break "-source $srcfile -line 3 +100" garbage "+100"
>>>> -test_break "$srcfile:3 -100" unexpected_opt "number" "-100"
>>>> -test_break "$srcfile:3 foo" unexpected_opt "string" "foo"
>>>> -test_break "-source $srcfile -line 3 foo" garbage "foo"
>>>> -
>>>> -foreach x $invalid_offsets {
>>>> -    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_break "-source $srcfile -line $x" invalid_offset_f $x $srcfile
>>>> -}
>>>> -test_break "-source $srcfile -line -x" malformed_line_offset "-x"
>>>> +    test_break "${srcfile}::" invalid_function "${srcfile}::"
>>>> +    test_break "$srcfile:3 1" unexpected_opt "number" "1"
>>>> +    test_break "-source $srcfile -line 3 1" garbage "1"
>>>> +    test_break "$srcfile:3 +100" unexpected_opt "number" "+100"
>>>> +    test_break "-source $srcfile -line 3 +100" garbage "+100"
>>>> +    test_break "$srcfile:3 -100" unexpected_opt "number" "-100"
>>>> +    test_break "$srcfile:3 foo" unexpected_opt "string" "foo"
>>>> +    test_break "-source $srcfile -line 3 foo" garbage "foo"
>>>> +
>>>> +    foreach x $invalid_offsets {
>>>> +	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_break "-source $srcfile -line $x" invalid_offset_f $x $srcfile
>>>> +    }
>>>> +    test_break "-source $srcfile -line -x" malformed_line_offset "-x"
>>>>  
>>>> -# Test invalid filespecs starting with function.
>>>> -foreach x {"foobar" "foo::bar" "foo.bar" "foo ." "foo bar" "foo 1" \
>>>> +    # 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"} {
>>>> -    test_break $x invalid_function $x
>>>> -    test_break "-function \"$x\"" invalid_function $x
>>>> -}
>>>> +	test_break $x invalid_function $x
>>>> +	test_break "-function \"$x\"" invalid_function $x
>>>> +    }
>>>>  
>>>> -foreach x $spaces {
>>>> -    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"
>>>> -}
>>>> +    foreach x $spaces {
>>>> +	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"
>>>> +    }
>>>>  
>>>> -foreach x {"3" "+100" "-100" "foo"} {
>>>> -    test_break "main 3" invalid_function "main 3"
>>>> -    test_break "-function \"main $x\"" invalid_function "main $x"
>>>> -    test_break "main:here $x" invalid_label "here $x" "main"
>>>> -    test_break "-function main -label \"here $x\"" \
>>>> -	invalid_label "here $x" "main"
>>>> -}
>>>> +    foreach x {"3" "+100" "-100" "foo"} {
>>>> +	test_break "main 3" invalid_function "main 3"
>>>> +	test_break "-function \"main $x\"" invalid_function "main $x"
>>>> +	test_break "main:here $x" invalid_label "here $x" "main"
>>>> +	test_break "-function main -label \"here $x\"" \
>>>> +		   invalid_label "here $x" "main"
>>>> +    }
>>>>  
>>>> -foreach x {"if" "task" "thread"} {
>>>> -    test_break $x invalid_function $x
>>>> -}
>>>> +    foreach x {"if" "task" "thread"} {
>>>> +	test_break $x invalid_function $x
>>>> +    }
>>>>  
>>>> -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_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}
>>>> -test_break $x invalid_var_or_func $x
>>>> -test_break "$srcfile:$x" invalid_var_or_func_f $x $srcfile
>>>> +    # Test undefined convenience variables.
>>>> +    set x {$zippo}
>>>> +    test_break $x invalid_var_or_func $x
>>>> +    test_break "$srcfile:$x" invalid_var_or_func_f $x $srcfile
>>>>  
>>>> -# Explicit linespec-specific tests
>>>> -test_break "-source $srcfile" source_incomplete
>>>> +    # Explicit linespec-specific tests
>>>> +    test_break "-source $srcfile" source_incomplete
>>>> +}
>>>> +
>>>> +foreach_with_prefix lang {"C" "C++"} {
>>>> +    do_test ${lang}
>>>> +}
>>>> diff --git a/gdb/testsuite/gdb.mi/mi-linespec-err-cp.cc b/gdb/testsuite/gdb.mi/mi-linespec-err-cp.cc
>>>> new file mode 100644
>>>> index 0000000..19c92d8
>>>> --- /dev/null
>>>> +++ b/gdb/testsuite/gdb.mi/mi-linespec-err-cp.cc
>>>> @@ -0,0 +1,35 @@
>>>> +/* This testcase is part of GDB, the GNU debugger.
>>>> +
>>>> +   Copyright 2016 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
>>>> +   the Free Software Foundation; either version 3 of the License, or
>>>> +   (at your option) any later version.
>>>> +
>>>> +   This program is distributed in the hope that it will be useful,
>>>> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
>>>> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>>>> +   GNU General Public License for more details.
>>>> +
>>>> +   You should have received a copy of the GNU General Public License
>>>> +   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
>>>> +
>>>> +int
>>>> +myfunction (int aa)
>>>> +{
>>>> +  int i;
>>>> +
>>>> +  i = aa + 42;
>>>> +  return i;    /* set breakpoint here */
>>>> +}
>>>> +
>>>> +int
>>>> +main (void)
>>>> +{
>>>> +  int a;
>>>> +
>>>> +  a = myfunction (a);
>>>> +
>>>> +  return a;
>>>> +}
>>>> diff --git a/gdb/testsuite/gdb.mi/mi-linespec-err-cp.exp b/gdb/testsuite/gdb.mi/mi-linespec-err-cp.exp
>>>> new file mode 100644
>>>> index 0000000..1a8682e
>>>> --- /dev/null
>>>> +++ b/gdb/testsuite/gdb.mi/mi-linespec-err-cp.exp
>>>> @@ -0,0 +1,57 @@
>>>> +# Copyright 2016 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
>>>> +# the Free Software Foundation; either version 3 of the License, or
>>>> +# (at your option) any later version.
>>>> +#
>>>> +# This program is distributed in the hope that it will be useful,
>>>> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
>>>> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>>>> +# GNU General Public License for more details.
>>>> +#
>>>> +# You should have received a copy of the GNU General Public License
>>>> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
>>>> +
>>>> +# Regression test for PR breakpoints/18303.  Tests that the correct
>>>> +# errors is generated when setting a breakpoint in a non-existent
>>>> +# file with a Windows-style logical drive names and C++.
>>>> +
>>>> +if { [skip_cplus_tests] } { continue }
>>>> +
>>>> +load_lib mi-support.exp
>>>> +set MIFLAGS "-i=mi"
>>>> +
>>>> +standard_testfile .cc
>>>> +set exefile $testfile
>>>> +
>>>> +if {[prepare_for_testing $testfile $exefile $srcfile {debug c++}]} {
>>>> +    return -1
>>>> +}
>>>> +
>>>> +gdb_exit
>>>> +if [mi_gdb_start] {
>>>> +    continue
>>>> +}
>>>> +
>>>> +# Turn off the pending breakpoint queries.
>>>> +mi_gdb_test "-interpreter-exec console \"set breakpoint pending off\"" \
>>>> +  {=cmd-param-changed,param=\"breakpoint pending\",.*\^done} \
>>>> +  "-interpreter-exec console \"set breakpoint pending off\""
>>>> +
>>>> +mi_run_to_main
>>>> +
>>>> +# Run to a location in the file.
>>>> +set bp_location [gdb_get_line_number "set breakpoint here"]
>>>> +
>>>> +mi_gdb_test "-break-insert ${srcfile}:${bp_location}" \
>>>> +    {\^done,bkpt=.number="2",type="breakpoint".*\}} "set breakpoint"
>>>> +
>>>> +mi_execute_to "exec-continue" "breakpoint-hit" "myfunction" ".*" ".*" "24" \
>>>> +    { "" "disp=\"keep\"" } "breakpoint hit"
>>>> +
>>>> +# Set a breakpoint in a C++ source file whose name contains a
>>>> +# Windows-style logical drive.
>>>> +mi_gdb_test \
>>>> +    "-break-insert -f \"c:/uu.cpp:13\"" \
>>>> +    ".*No source file named c:/uu.cpp.*"
>>>>
>>> Ping?
>>> thanks
>>> --Don
>>>
>>
> 

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

* Re: [PING] Re: [PATCH v4] PR 18303, Tolerate malformed input for lookup_symbol-called functions (was: Re: [PATCH] Fix problem handling colon in linespec, PR breakpoints/18303)
  2016-03-03 18:19                     ` Don Breazeal
@ 2016-03-14 21:23                       ` Don Breazeal
  2016-03-15 15:55                         ` [PING] Re: [PATCH v4] PR 18303, Tolerate malformed input for lookup_symbol-called functions Pedro Alves
  0 siblings, 1 reply; 17+ messages in thread
From: Don Breazeal @ 2016-03-14 21:23 UTC (permalink / raw)
  To: gdb-patches, Pedro Alves

On 1/28/2016 4:06 AM, Pedro Alves wrote:
> On 01/28/2016 01:21 AM, Don Breazeal wrote:
>> The patch includes three new tests related to this.  One is just
>> gdb.linespec/ls-errs.exp copied and converted to use C++ instead of
C, and
>> to add a case using a file name containing a Windows-style logical drive
>> specifier.
> ....
>> gdb/testsuite/gdb.linespec/ls-errs-cp.cc    |  36 +++++
>> gdb/testsuite/gdb.linespec/ls-errs-cp.exp   | 240
++++++++++++++++++++++++++++
> ...
>
> Can't we somehow reuse the existing test?  Say, either:
>
>  - move the main body of ls-errs.exp a procedure, and call it twice,
>    once for each language, or,
>
>  - make gdb.linespec/ls-errs-cp.exp set some $language var and then
>    source gdb.linespec/ls-errs.exp, like gdb.base/checkpoint-ns.exp.
>
> Thanks,
> Pedro Alves
>

Hi Pedro,
I used your first suggestion above.  Did you have any other comments
for this one, or is it good to go?
Thanks,
--Don

On 3/3/2016 10:19 AM, Don Breazeal wrote:
> Ping.
> I checked, the patch still applies cleanly to mainline.
> Thanks
> --Don
> 
> On 2/25/2016 9:27 AM, Don Breazeal wrote:
>> Ping
>> Thanks
>> --Don
>>
>> On 2/18/2016 10:13 AM, Don Breazeal wrote:
>>> Ping.
>>> Current status of this one:
>>>  - Doug approved (at least in concept) Keith's fix below.
>>>  - Pedro requested that the new C++ linespec error test be consolidated
>>> with the existing C version of the test.  That's done in this version.
>>> Thanks!
>>> --Don
>>>
>>> On 2/4/2016 10:37 AM, Don Breazeal wrote:
>>>> Ping?
>>>> thanks
>>>> --Don
>>>>
>>>> On 1/28/2016 2:52 PM, Don Breazeal wrote:
>>>>> Apologies -- the previous version of this patch was missing one file,
>>>>> added here.  Sorry for the noise.
>>>>> --Don
>>>>>
>>>>> ---V3 Comment---
>>>>> This patch differs from v2 in that the new test gdb.linespec/ls-errs-cp.exp
>>>>> is gone, and instead (per Pedro's suggestion) gdb.linespec/ls-errs.exp has
>>>>> been modified to test both C & C++, and to incorporate the small amount of
>>>>> extra testing from ls-errs-cp.exp.
>>>>>
>>>>> It also includes, unchanged from the previous version of the patch:
>>>>>
>>>>>  * an updated version of the alternate fix that Keith proposed for my
>>>>>    patch that addressed PR breakpoints/18303.  Doug has approved (in
>>>>>    concept, at least) the code portion of the patch.
>>>>>
>>>>>  * a couple of other new tests of colons in linespecs.
>>>>>
>>>>> Thanks,
>>>>> --Don
>>>>>
>>>>> -----------
>>>>> lookup_symbol is often called with user input.  Consequently, any
>>>>> function called from lookup_symbol{,_in_language} should attempt to
>>>>> deal with malformed input gracefully.  After all, malformed user
>>>>> input is not a programming/API error.
>>>>>
>>>>> This patch does not attempt to find/correct all instances of this.  It
>>>>> only fixes locations in the code that trigger test suite failures.
>>>>>
>>>>> This patch fixes PR breakpoints/18303, "Assertion: -breakpoint-insert
>>>>> with windows paths of file in non-current directory".
>>>>>
>>>>> The patch includes three new tests related to this.  One is just
>>>>> gdb.linespec/ls-errs.exp copied and converted to use C++ instead of C, and
>>>>> to add a case using a file name containing a Windows-style logical drive
>>>>> specifier.  The others include an MI test to provide a regression test for
>>>>> the specific case reported in PR 18303, and a C++ test for proper error
>>>>> handling of access to a program variable when using a file scope specifier
>>>>> that refers to a non-existent file.
>>>>>
>>>>> Tested on x86_64 native Linux.
>>>>>
>>>>> gdb/ChangeLog
>>>>> 2016-01-28  Keith Seitz  <keiths@redhat.com>
>>>>>
>>>>> 	PR breakpoints/18303
>>>>> 	* cp-namespace.c (cp_lookup_bare_symbol): Change assertion to
>>>>> 	look for "::" instead of simply ":".
>>>>> 	(cp_search_static_and_baseclasses): Return null_block_symbol for
>>>>> 	malformed input.
>>>>> 	Remove assertions.
>>>>> 	* cp-support.c (cp_find_first_component_aux): Do not return
>>>>> 	a prefix length for ':' unless the next character is also ':'.
>>>>>
>>>>> gdb/testsuite/ChangeLog
>>>>> 2016-01-28  Don Breazeal  <donb@codesourcery.com>
>>>>>
>>>>> 	* gdb.cp/scope-err.cc: New test program.
>>>>> 	* gdb.cp/scope-err.exp: New test script.
>>>>> 	* gdb.linespec/ls-errs.c (myfunction): Expanded to have multiple
>>>>> 	lines and "set breakpoint here" comment.
>>>>> 	* gdb.linespec/ls-errs.exp: Added C++ testing and new test case.
>>>>> 	* gdb.mi/mi-linespec-err-cp.cc: New test program.
>>>>> 	* gdb.mi/mi-linespec-err-cp.exp: New test script.
>>>>>
>>>>> ---
>>>>>  gdb/cp-namespace.c                          |   9 +-
>>>>>  gdb/cp-support.c                            |   7 +-
>>>>>  gdb/testsuite/gdb.cp/scope-err.cc           |  35 +++
>>>>>  gdb/testsuite/gdb.cp/scope-err.exp          |  47 ++++
>>>>>  gdb/testsuite/gdb.linespec/ls-errs.c        |  13 +-
>>>>>  gdb/testsuite/gdb.linespec/ls-errs.exp      | 384 +++++++++++++++-------------
>>>>>  gdb/testsuite/gdb.mi/mi-linespec-err-cp.cc  |  35 +++
>>>>>  gdb/testsuite/gdb.mi/mi-linespec-err-cp.exp |  57 +++++
>>>>>  8 files changed, 405 insertions(+), 182 deletions(-)
>>>>>
>>>>> diff --git a/gdb/cp-namespace.c b/gdb/cp-namespace.c
>>>>> index 72002d6..016a42f 100644
>>>>> --- a/gdb/cp-namespace.c
>>>>> +++ b/gdb/cp-namespace.c
>>>>> @@ -170,7 +170,7 @@ cp_lookup_bare_symbol (const struct language_defn *langdef,
>>>>>       ':' may be in the args of a template spec.  This isn't intended to be
>>>>>       a complete test, just cheap and documentary.  */
>>>>>    if (strchr (name, '<') == NULL && strchr (name, '(') == NULL)
>>>>> -    gdb_assert (strchr (name, ':') == NULL);
>>>>> +    gdb_assert (strstr (name, "::") == NULL);
>>>>>  
>>>>>    sym = lookup_symbol_in_static_block (name, block, domain);
>>>>>    if (sym.symbol != NULL)
>>>>> @@ -246,10 +246,9 @@ cp_search_static_and_baseclasses (const char *name,
>>>>>    struct block_symbol klass_sym;
>>>>>    struct type *klass_type;
>>>>>  
>>>>> -  /* The test here uses <= instead of < because Fortran also uses this,
>>>>> -     and the module.exp testcase will pass "modmany::" for NAME here.  */
>>>>> -  gdb_assert (prefix_len + 2 <= strlen (name));
>>>>> -  gdb_assert (name[prefix_len + 1] == ':');
>>>>> +  /* Check for malformed input.  */
>>>>> +  if (prefix_len + 2 > strlen (name) || name[prefix_len + 1] != ':')
>>>>> +    return null_block_symbol;
>>>>>  
>>>>>    /* Find the name of the class and the name of the method, variable, etc.  */
>>>>>  
>>>>> diff --git a/gdb/cp-support.c b/gdb/cp-support.c
>>>>> index df127c4..a71c6ad 100644
>>>>> --- a/gdb/cp-support.c
>>>>> +++ b/gdb/cp-support.c
>>>>> @@ -1037,8 +1037,13 @@ cp_find_first_component_aux (const char *name, int permissive)
>>>>>  	      return strlen (name);
>>>>>  	    }
>>>>>  	case '\0':
>>>>> -	case ':':
>>>>>  	  return index;
>>>>> +	case ':':
>>>>> +	  /* ':' marks a component iff the next character is also a ':'.
>>>>> +	     Otherwise it is probably malformed input.  */
>>>>> +	  if (name[index + 1] == ':')
>>>>> +	    return index;
>>>>> +	  break;
>>>>>  	case 'o':
>>>>>  	  /* Operator names can screw up the recursion.  */
>>>>>  	  if (operator_possible
>>>>> diff --git a/gdb/testsuite/gdb.cp/scope-err.cc b/gdb/testsuite/gdb.cp/scope-err.cc
>>>>> new file mode 100644
>>>>> index 0000000..19c92d8
>>>>> --- /dev/null
>>>>> +++ b/gdb/testsuite/gdb.cp/scope-err.cc
>>>>> @@ -0,0 +1,35 @@
>>>>> +/* This testcase is part of GDB, the GNU debugger.
>>>>> +
>>>>> +   Copyright 2016 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
>>>>> +   the Free Software Foundation; either version 3 of the License, or
>>>>> +   (at your option) any later version.
>>>>> +
>>>>> +   This program is distributed in the hope that it will be useful,
>>>>> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
>>>>> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>>>>> +   GNU General Public License for more details.
>>>>> +
>>>>> +   You should have received a copy of the GNU General Public License
>>>>> +   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
>>>>> +
>>>>> +int
>>>>> +myfunction (int aa)
>>>>> +{
>>>>> +  int i;
>>>>> +
>>>>> +  i = aa + 42;
>>>>> +  return i;    /* set breakpoint here */
>>>>> +}
>>>>> +
>>>>> +int
>>>>> +main (void)
>>>>> +{
>>>>> +  int a;
>>>>> +
>>>>> +  a = myfunction (a);
>>>>> +
>>>>> +  return a;
>>>>> +}
>>>>> diff --git a/gdb/testsuite/gdb.cp/scope-err.exp b/gdb/testsuite/gdb.cp/scope-err.exp
>>>>> new file mode 100644
>>>>> index 0000000..9d93578
>>>>> --- /dev/null
>>>>> +++ b/gdb/testsuite/gdb.cp/scope-err.exp
>>>>> @@ -0,0 +1,47 @@
>>>>> +# Copyright 2012-2016 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
>>>>> +# the Free Software Foundation; either version 3 of the License, or
>>>>> +# (at your option) any later version.
>>>>> +#
>>>>> +# This program is distributed in the hope that it will be useful,
>>>>> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
>>>>> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>>>>> +# GNU General Public License for more details.
>>>>> +#
>>>>> +# You should have received a copy of the GNU General Public License
>>>>> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
>>>>> +
>>>>> +# Tests for linespec errors with C++.
>>>>> +# Derived from gdb.linespec/ls-errs.exp.
>>>>> +
>>>>> +if { [skip_cplus_tests] } { continue }
>>>>> +
>>>>> +standard_testfile .cc
>>>>> +set exefile $testfile
>>>>> +
>>>>> +if {[prepare_for_testing $testfile $exefile $srcfile {debug c++}]} {
>>>>> +    return -1
>>>>> +}
>>>>> +
>>>>> +if ![runto_main] {
>>>>> +    fail "Can't run to main"
>>>>> +    return 0
>>>>> +}
>>>>> +
>>>>> +# Run to a location in the file.
>>>>> +set bp_location [gdb_get_line_number "set breakpoint here"]
>>>>> +
>>>>> +gdb_test "break $srcfile:$bp_location" \
>>>>> +    "Breakpoint.*at.* file .*$srcfile, line $bp_location\\." \
>>>>> +    "breakpoint line number in file"
>>>>> +
>>>>> +gdb_continue_to_breakpoint "$bp_location"
>>>>> +
>>>>> +# Try to access a variable using scope that is a non-existent filename
>>>>> +# with a Windows-style logical drive in the name.
>>>>> +set nonexistent_file C:/does/not/exist.cc
>>>>> +gdb_test "print '$nonexistent_file'::var" \
>>>>> +	 ".*No symbol \"$nonexistent_file\" in current context.*" \
>>>>> +	 "print var from \"$nonexistent_file\""
>>>>> diff --git a/gdb/testsuite/gdb.linespec/ls-errs.c b/gdb/testsuite/gdb.linespec/ls-errs.c
>>>>> index ca41342..a3a43db 100644
>>>>> --- a/gdb/testsuite/gdb.linespec/ls-errs.c
>>>>> +++ b/gdb/testsuite/gdb.linespec/ls-errs.c
>>>>> @@ -15,14 +15,21 @@
>>>>>     You should have received a copy of the GNU General Public License
>>>>>     along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
>>>>>  
>>>>> -int myfunction (void) { return 0; }
>>>>> +int
>>>>> +myfunction (int aa)
>>>>> +{
>>>>> +  int i;
>>>>> +
>>>>> +  i = aa + 42;
>>>>> +  return i;    /* set breakpoint here */
>>>>> +}
>>>>>  
>>>>>  int
>>>>>  main (void)
>>>>> -{  
>>>>> +{
>>>>>    int a;
>>>>>  
>>>>> -  a = myfunction ();
>>>>> +  a = myfunction (a);
>>>>>  
>>>>>   here:
>>>>>    return a;
>>>>> diff --git a/gdb/testsuite/gdb.linespec/ls-errs.exp b/gdb/testsuite/gdb.linespec/ls-errs.exp
>>>>> index 16d4574..35f8f78 100644
>>>>> --- a/gdb/testsuite/gdb.linespec/ls-errs.exp
>>>>> +++ b/gdb/testsuite/gdb.linespec/ls-errs.exp
>>>>> @@ -13,209 +13,247 @@
>>>>>  # You should have received a copy of the GNU General Public License
>>>>>  # along with this program.  If not, see <http://www.gnu.org/licenses/>.
>>>>>  
>>>>> -# Tests for linespec error conditions
>>>>> +# Tests for linespec errors with C and C++.
>>>>>  
>>>>> -standard_testfile
>>>>> -set exefile $testfile
>>>>> +# The test proper.  LANG is either C or C++.
>>>>>  
>>>>> -if {[prepare_for_testing $testfile $exefile $srcfile \
>>>>> -	 {debug nowarnings}]} {
>>>>> -    return -1
>>>>> -}
>>>>> +proc do_test {lang} {
>>>>> +    global testfile srcfile error_messages compiler_info
>>>>>  
>>>>> -# Turn off the pending breakpoint queries.
>>>>> -gdb_test_no_output "set breakpoint pending off"
>>>>> +    standard_testfile
>>>>> +    set exefile $testfile
>>>>> +    if [info exists compiler_info] {
>>>>> +	# Unsetting compiler_info allows us to switch compilers
>>>>> +	# used by prepare_for_testing.
>>>>> +        unset compiler_info
>>>>> +    }
>>>>> +    set options {debug}
>>>>>  
>>>>> -# Turn off completion limiting
>>>>> -gdb_test_no_output "set max-completions unlimited"
>>>>> +    if { $lang == "C++" } {
>>>>> +	if { [skip_cplus_tests] } { return 0 }
>>>>> +	# Build ".c" source file with g++.
>>>>> +	lappend options "c++"
>>>>> +    }
>>>>>  
>>>>> -# We intentionally do not use gdb_breakpoint for these tests.
>>>>> +    if {[prepare_for_testing $testfile $exefile $srcfile $options]} {
>>>>> +	return -1
>>>>> +    }
>>>>>  
>>>>> -# 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
>>>>> +    # Turn off the pending breakpoint queries.
>>>>> +    gdb_test_no_output "set breakpoint pending off"
>>>>>  
>>>>> -    gdb_test "break $linespec" [string_to_regexp \
>>>>> -				[eval format \$error_messages($msg_id) $args]]
>>>>> -}
>>>>> +    # Turn off completion limiting
>>>>> +    gdb_test_no_output "set max-completions unlimited"
>>>>>  
>>>>> -# Common error message format strings.
>>>>> -array set error_messages {
>>>>> -    invalid_file "No source file named %s."
>>>>> -    invalid_function "Function \"%s\" not defined."
>>>>> -    invalid_var_or_func "Undefined convenience variable or function \"%s\" not defined."
>>>>> -    invalid_function_f "Function \"%s\" not defined in \"%s\"."
>>>>> -    invalid_var_or_func_f \
>>>>> -	"Undefined convenience variable or function \"%s\" not defined in \"%s\"."
>>>>> -    invalid_label "No label \"%s\" defined in function \"%s\"."
>>>>> -    invalid_parm "invalid linespec argument, \"%s\""
>>>>> -    invalid_offset "No line %d in the current file."
>>>>> -    invalid_offset_f "No line %d in file \"%s\"."
>>>>> -    malformed_line_offset "malformed line offset: \"%s\""
>>>>> -    source_incomplete \
>>>>> -	"Source filename requires function, label, or line offset."
>>>>> -    unexpected "malformed linespec error: unexpected %s"
>>>>> -    unexpected_opt "malformed linespec error: unexpected %s, \"%s\""
>>>>> -    unmatched_quote "unmatched quote"
>>>>> -    garbage "Garbage '%s' at end of command"
>>>>> -}
>>>>> +    if ![runto_main] {
>>>>> +	fail "Can't run to main"
>>>>> +	return 0
>>>>> +    }
>>>>>  
>>>>> -# Some commonly used whitespace tests around ':'.
>>>>> -set spaces [list ":" ": " " :" " : " "\t:  " "  :\t" "\t:\t" " \t:\t " \
>>>>> -		"\t  \t:\t  \t  \t"]
>>>>> +    # Run to a location in the file.
>>>>> +    set bp_location [gdb_get_line_number "set breakpoint here"]
>>>>> +
>>>>> +    gdb_test "break $srcfile:$bp_location" \
>>>>> +    "Breakpoint.*at.* file .*$srcfile, line $bp_location\\." \
>>>>> +    "breakpoint line number in file"
>>>>> +
>>>>> +    gdb_continue_to_breakpoint "$bp_location"
>>>>> +
>>>>> +    # Common error message format strings.
>>>>> +    array set error_messages {
>>>>> +	invalid_file "No source file named %s."
>>>>> +	invalid_function "Function \"%s\" not defined."
>>>>> +	invalid_var_or_func 
>>>>> +	    "Undefined convenience variable or function \"%s\" not defined."
>>>>> +	invalid_function_f "Function \"%s\" not defined in \"%s\"."
>>>>> +	invalid_var_or_func_f \
>>>>> +	    "Undefined convenience variable or function \"%s\" not defined in \"%s\"."
>>>>> +	invalid_label "No label \"%s\" defined in function \"%s\"."
>>>>> +	invalid_parm "invalid linespec argument, \"%s\""
>>>>> +	invalid_offset "No line %d in the current file."
>>>>> +	invalid_offset_f "No line %d in file \"%s\"."
>>>>> +	malformed_line_offset "malformed line offset: \"%s\""
>>>>> +	source_incomplete \
>>>>> +	    "Source filename requires function, label, or line offset."
>>>>> +	unexpected "malformed linespec error: unexpected %s"
>>>>> +	unexpected_opt "malformed linespec error: unexpected %s, \"%s\""
>>>>> +	unmatched_quote "unmatched quote"
>>>>> +	garbage "Garbage '%s' at end of command"
>>>>> +    }
>>>>>  
>>>>> -# A list of invalid offsets.
>>>>> -set invalid_offsets [list -100 +500 1000]
>>>>> +    # We intentionally do not use gdb_breakpoint for these tests.
>>>>>  
>>>>> -# Try some simple, invalid linespecs involving spaces.
>>>>> -foreach x $spaces {
>>>>> -    test_break $x unexpected "colon"
>>>>> -}
>>>>> +    # 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
>>>>>  
>>>>> -# Test invalid filespecs starting with offset.  This is done
>>>>> -# first so that default offsets are tested.
>>>>> -foreach x $invalid_offsets {
>>>>> -    set offset $x
>>>>> -
>>>>> -    # Relative offsets are relative to line 16.  Adjust
>>>>> -    # expected offset from error message accordingly.
>>>>> -    if {[string index $x 0] == "+" ||
>>>>> -	[string index $x 0] == "-"} {
>>>>> -	incr offset 16
>>>>> +	gdb_test "break $linespec" [string_to_regexp \
>>>>> +				    [eval format \$error_messages($msg_id) \
>>>>> +				     $args]]
>>>>>      }
>>>>> -    test_break $x invalid_offset $offset
>>>>> -    test_break "-line $x" invalid_offset $offset
>>>>> -}
>>>>>  
>>>>> -# Test offsets with trailing tokens w/ and w/o spaces.
>>>>> -foreach x $spaces {
>>>>> -    test_break "3$x" unexpected "colon"
>>>>> -    test_break "+10$x" unexpected "colon"
>>>>> -    test_break "-10$x" unexpected "colon"
>>>>> -}
>>>>> +    # Some commonly used whitespace tests around ':'.
>>>>> +    set spaces [list ":" ": " " :" " : " "\t:  " "  :\t" "\t:\t" \
>>>>> +		     " \t:\t " "\t  \t:\t  \t  \t"]
>>>>>  
>>>>> -foreach x {1 +1 +100 -10} {
>>>>> -    test_break "3 $x" unexpected_opt "number" $x
>>>>> -    test_break "-line 3 $x" garbage $x
>>>>> -    test_break "+10 $x" unexpected_opt "number" $x
>>>>> -    test_break "-line +10 $x" garbage $x
>>>>> -    test_break "-10 $x" unexpected_opt "number" $x
>>>>> -    test_break "-line -10 $x" garbage $x
>>>>> -}
>>>>> +    # A list of invalid offsets.
>>>>> +    set invalid_offsets [list -100 +500 1000]
>>>>>  
>>>>> -foreach x {3 +10 -10} {
>>>>> -    test_break "$x foo" unexpected_opt "string" "foo"
>>>>> -    test_break "-line $x foo" garbage "foo"
>>>>> -}
>>>>> +    # Try some simple, invalid linespecs involving spaces.
>>>>> +    foreach x $spaces {
>>>>> +	test_break $x unexpected "colon"
>>>>> +    }
>>>>>  
>>>>> -# Test invalid linespecs starting with filename.
>>>>> -foreach x [list "this_file_doesn't_exist.c" \
>>>>> -	       "this file has spaces.c" \
>>>>> -	       "\"file::colons.c\"" \
>>>>> -	       "'file::colons.c'" \
>>>>> -	       "\"this \"file\" has quotes.c\"" \
>>>>> -	       "'this \"file\" has quotes.c'" \
>>>>> -	       "'this 'file' has quotes.c'" \
>>>>> -	       "\"this 'file' has quotes.c\"" \
>>>>> -	       "\"spaces: and :colons.c\"" \
>>>>> -	       "'more: :spaces: :and  colons::.c'"] {
>>>>> -    # Remove any quoting from FILENAME for the error message.
>>>>> -    test_break "$x:3" invalid_file [string trim $x \"']
>>>>> -}
>>>>> -foreach x [list "this_file_doesn't_exist.c" \
>>>>> -	       "file::colons.c" \
>>>>> -	       "'file::colons.c'"] {
>>>>> -    test_break "-source $x -line 3" \
>>>>> -	invalid_file [string trim $x \"']
>>>>> -}
>>>>> +    # Test invalid filespecs starting with offset.  This is done
>>>>> +    # first so that default offsets are tested.
>>>>> +    foreach x $invalid_offsets {
>>>>> +	set offset $x
>>>>> +
>>>>> +	# Relative offsets are relative to line 16.  Adjust
>>>>> +	# expected offset from error message accordingly.
>>>>> +	if {[string index $x 0] == "+" || [string index $x 0] == "-"} {
>>>>> + 	    incr offset 24
>>>>> +	}
>>>>> +	test_break $x invalid_offset $offset
>>>>> +	test_break "-line $x" invalid_offset $offset
>>>>> +    }
>>>>> +
>>>>> +    # Test offsets with trailing tokens w/ and w/o spaces.
>>>>> +    foreach x $spaces {
>>>>> +	test_break "3$x" unexpected "colon"
>>>>> +	test_break "+10$x" unexpected "colon"
>>>>> +	test_break "-10$x" unexpected "colon"
>>>>> +    }
>>>>> +
>>>>> +    foreach x {1 +1 +100 -10} {
>>>>> +	test_break "3 $x" unexpected_opt "number" $x
>>>>> +	test_break "-line 3 $x" garbage $x
>>>>> +	test_break "+10 $x" unexpected_opt "number" $x
>>>>> +	test_break "-line +10 $x" garbage $x
>>>>> +	test_break "-10 $x" unexpected_opt "number" $x
>>>>> +	test_break "-line -10 $x" garbage $x
>>>>> +    }
>>>>>  
>>>>> -# Test that option lexing stops at whitespace boundaries
>>>>> -test_break "-source this file has spaces.c -line 3" \
>>>>> -    invalid_file "this"
>>>>> +    foreach x {3 +10 -10} {
>>>>> +	test_break "$x foo" unexpected_opt "string" "foo"
>>>>> +	test_break "-line $x foo" garbage "foo"
>>>>> +    }
>>>>>  
>>>>> -test_break "-function function whitespace" \
>>>>> -    invalid_function "function"
>>>>> +    # Test invalid linespecs starting with filename.
>>>>> +    # It's OK to use the ".c" extension for the C++ test
>>>>> +    # since the extension doesn't affect GDB's lookup.
>>>>> +    set invalid_files [list "this_file_doesn't_exist.c" \
>>>>> +			    "this file has spaces.c" \
>>>>> +			    "\"file::colons.c\"" \
>>>>> +			    "'file::colons.c'" \
>>>>> +			    "\"this \"file\" has quotes.c\"" \
>>>>> +			    "'this \"file\" has quotes.c'" \
>>>>> +			    "'this 'file' has quotes.c'" \
>>>>> +			    "\"this 'file' has quotes.c\"" \
>>>>> +			    "\"spaces: and :colons.c\"" \
>>>>> +			    "'more: :spaces: :and  colons::.c'" \
>>>>> +			    "C:/nonexist-with-windrive.c"]
>>>>> +
>>>>> +    foreach x $invalid_files {
>>>>> +	# Remove any quoting from FILENAME for the error message.
>>>>> +	test_break "$x:3" invalid_file [string trim $x \"']
>>>>> +    }
>>>>> +    foreach x [list "this_file_doesn't_exist.c" \
>>>>> +		    "file::colons.c" \
>>>>> +		    "'file::colons.c'"] {
>>>>> +	test_break "-source $x -line 3" invalid_file [string trim $x \"']
>>>>> +    }
>>>>>  
>>>>> -test_break "-source $srcfile -function function whitespace" \
>>>>> -    invalid_function_f "function" $srcfile
>>>>> +    # Test that option lexing stops at whitespace boundaries
>>>>> +    test_break "-source this file has spaces.c -line 3" invalid_file "this"
>>>>> +    test_break "-function function whitespace" invalid_function "function"
>>>>> +    test_break "-source $srcfile -function function whitespace" \
>>>>> +	       invalid_function_f "function" $srcfile
>>>>>  
>>>>> -test_break "-function main -label label whitespace" \
>>>>> -    invalid_label "label" "main"
>>>>> +    test_break "-function main -label label whitespace" \
>>>>> +	       invalid_label "label" "main"
>>>>>  
>>>>> -# Test unmatched quotes.
>>>>> -foreach x {"\"src-file.c'" "'src-file.c"} {
>>>>> -    test_break "$x:3" unmatched_quote
>>>>> -}
>>>>> +    # Test unmatched quotes.
>>>>> +    foreach x {"\"src-file.c'" "'src-file.c"} {
>>>>> +	test_break "$x:3" unmatched_quote
>>>>> +    }
>>>>>  
>>>>> -test_break $srcfile invalid_function $srcfile
>>>>> -foreach x {"foo" " foo" " foo "} {
>>>>> -    # Trim any leading/trailing whitespace for error messages.
>>>>> -    test_break "$srcfile:$x" invalid_function_f [string trim $x] $srcfile
>>>>> -    test_break "-source $srcfile -function $x" \
>>>>> -	invalid_function_f [string trim $x] $srcfile
>>>>> -    test_break "$srcfile:main:$x" invalid_label [string trim $x] "main"
>>>>> -    test_break "-source $srcfile -function main -label $x" \
>>>>> -	invalid_label [string trim $x] "main"
>>>>> -}
>>>>> +    test_break $srcfile invalid_function $srcfile
>>>>> +    foreach x {"foo" " foo" " foo "} {
>>>>> +	# Trim any leading/trailing whitespace for error messages.
>>>>> +	test_break "$srcfile:$x" invalid_function_f [string trim $x] $srcfile
>>>>> +	test_break "-source $srcfile -function $x" \
>>>>> +		   invalid_function_f [string trim $x] $srcfile
>>>>> +	test_break "$srcfile:main:$x" invalid_label [string trim $x] "main"
>>>>> +	test_break "-source $srcfile -function main -label $x" \
>>>>> +		   invalid_label [string trim $x] "main"
>>>>> +    }
>>>>>  
>>>>> -foreach x $spaces {
>>>>> -    test_break "$srcfile$x" unexpected "end of input"
>>>>> -    test_break "$srcfile:main$x" unexpected "end of input"
>>>>> -}
>>>>> +    foreach x $spaces {
>>>>> +	test_break "$srcfile$x" unexpected "end of input"
>>>>> +	test_break "$srcfile:main$x" unexpected "end of input"
>>>>> +    }
>>>>>  
>>>>> -test_break "${srcfile}::" invalid_function "${srcfile}::"
>>>>> -test_break "$srcfile:3 1" unexpected_opt "number" "1"
>>>>> -test_break "-source $srcfile -line 3 1" garbage "1"
>>>>> -test_break "$srcfile:3 +100" unexpected_opt "number" "+100"
>>>>> -test_break "-source $srcfile -line 3 +100" garbage "+100"
>>>>> -test_break "$srcfile:3 -100" unexpected_opt "number" "-100"
>>>>> -test_break "$srcfile:3 foo" unexpected_opt "string" "foo"
>>>>> -test_break "-source $srcfile -line 3 foo" garbage "foo"
>>>>> -
>>>>> -foreach x $invalid_offsets {
>>>>> -    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_break "-source $srcfile -line $x" invalid_offset_f $x $srcfile
>>>>> -}
>>>>> -test_break "-source $srcfile -line -x" malformed_line_offset "-x"
>>>>> +    test_break "${srcfile}::" invalid_function "${srcfile}::"
>>>>> +    test_break "$srcfile:3 1" unexpected_opt "number" "1"
>>>>> +    test_break "-source $srcfile -line 3 1" garbage "1"
>>>>> +    test_break "$srcfile:3 +100" unexpected_opt "number" "+100"
>>>>> +    test_break "-source $srcfile -line 3 +100" garbage "+100"
>>>>> +    test_break "$srcfile:3 -100" unexpected_opt "number" "-100"
>>>>> +    test_break "$srcfile:3 foo" unexpected_opt "string" "foo"
>>>>> +    test_break "-source $srcfile -line 3 foo" garbage "foo"
>>>>> +
>>>>> +    foreach x $invalid_offsets {
>>>>> +	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_break "-source $srcfile -line $x" invalid_offset_f $x $srcfile
>>>>> +    }
>>>>> +    test_break "-source $srcfile -line -x" malformed_line_offset "-x"
>>>>>  
>>>>> -# Test invalid filespecs starting with function.
>>>>> -foreach x {"foobar" "foo::bar" "foo.bar" "foo ." "foo bar" "foo 1" \
>>>>> +    # 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"} {
>>>>> -    test_break $x invalid_function $x
>>>>> -    test_break "-function \"$x\"" invalid_function $x
>>>>> -}
>>>>> +	test_break $x invalid_function $x
>>>>> +	test_break "-function \"$x\"" invalid_function $x
>>>>> +    }
>>>>>  
>>>>> -foreach x $spaces {
>>>>> -    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"
>>>>> -}
>>>>> +    foreach x $spaces {
>>>>> +	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"
>>>>> +    }
>>>>>  
>>>>> -foreach x {"3" "+100" "-100" "foo"} {
>>>>> -    test_break "main 3" invalid_function "main 3"
>>>>> -    test_break "-function \"main $x\"" invalid_function "main $x"
>>>>> -    test_break "main:here $x" invalid_label "here $x" "main"
>>>>> -    test_break "-function main -label \"here $x\"" \
>>>>> -	invalid_label "here $x" "main"
>>>>> -}
>>>>> +    foreach x {"3" "+100" "-100" "foo"} {
>>>>> +	test_break "main 3" invalid_function "main 3"
>>>>> +	test_break "-function \"main $x\"" invalid_function "main $x"
>>>>> +	test_break "main:here $x" invalid_label "here $x" "main"
>>>>> +	test_break "-function main -label \"here $x\"" \
>>>>> +		   invalid_label "here $x" "main"
>>>>> +    }
>>>>>  
>>>>> -foreach x {"if" "task" "thread"} {
>>>>> -    test_break $x invalid_function $x
>>>>> -}
>>>>> +    foreach x {"if" "task" "thread"} {
>>>>> +	test_break $x invalid_function $x
>>>>> +    }
>>>>>  
>>>>> -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_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}
>>>>> -test_break $x invalid_var_or_func $x
>>>>> -test_break "$srcfile:$x" invalid_var_or_func_f $x $srcfile
>>>>> +    # Test undefined convenience variables.
>>>>> +    set x {$zippo}
>>>>> +    test_break $x invalid_var_or_func $x
>>>>> +    test_break "$srcfile:$x" invalid_var_or_func_f $x $srcfile
>>>>>  
>>>>> -# Explicit linespec-specific tests
>>>>> -test_break "-source $srcfile" source_incomplete
>>>>> +    # Explicit linespec-specific tests
>>>>> +    test_break "-source $srcfile" source_incomplete
>>>>> +}
>>>>> +
>>>>> +foreach_with_prefix lang {"C" "C++"} {
>>>>> +    do_test ${lang}
>>>>> +}
>>>>> diff --git a/gdb/testsuite/gdb.mi/mi-linespec-err-cp.cc b/gdb/testsuite/gdb.mi/mi-linespec-err-cp.cc
>>>>> new file mode 100644
>>>>> index 0000000..19c92d8
>>>>> --- /dev/null
>>>>> +++ b/gdb/testsuite/gdb.mi/mi-linespec-err-cp.cc
>>>>> @@ -0,0 +1,35 @@
>>>>> +/* This testcase is part of GDB, the GNU debugger.
>>>>> +
>>>>> +   Copyright 2016 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
>>>>> +   the Free Software Foundation; either version 3 of the License, or
>>>>> +   (at your option) any later version.
>>>>> +
>>>>> +   This program is distributed in the hope that it will be useful,
>>>>> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
>>>>> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>>>>> +   GNU General Public License for more details.
>>>>> +
>>>>> +   You should have received a copy of the GNU General Public License
>>>>> +   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
>>>>> +
>>>>> +int
>>>>> +myfunction (int aa)
>>>>> +{
>>>>> +  int i;
>>>>> +
>>>>> +  i = aa + 42;
>>>>> +  return i;    /* set breakpoint here */
>>>>> +}
>>>>> +
>>>>> +int
>>>>> +main (void)
>>>>> +{
>>>>> +  int a;
>>>>> +
>>>>> +  a = myfunction (a);
>>>>> +
>>>>> +  return a;
>>>>> +}
>>>>> diff --git a/gdb/testsuite/gdb.mi/mi-linespec-err-cp.exp b/gdb/testsuite/gdb.mi/mi-linespec-err-cp.exp
>>>>> new file mode 100644
>>>>> index 0000000..1a8682e
>>>>> --- /dev/null
>>>>> +++ b/gdb/testsuite/gdb.mi/mi-linespec-err-cp.exp
>>>>> @@ -0,0 +1,57 @@
>>>>> +# Copyright 2016 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
>>>>> +# the Free Software Foundation; either version 3 of the License, or
>>>>> +# (at your option) any later version.
>>>>> +#
>>>>> +# This program is distributed in the hope that it will be useful,
>>>>> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
>>>>> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>>>>> +# GNU General Public License for more details.
>>>>> +#
>>>>> +# You should have received a copy of the GNU General Public License
>>>>> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
>>>>> +
>>>>> +# Regression test for PR breakpoints/18303.  Tests that the correct
>>>>> +# errors is generated when setting a breakpoint in a non-existent
>>>>> +# file with a Windows-style logical drive names and C++.
>>>>> +
>>>>> +if { [skip_cplus_tests] } { continue }
>>>>> +
>>>>> +load_lib mi-support.exp
>>>>> +set MIFLAGS "-i=mi"
>>>>> +
>>>>> +standard_testfile .cc
>>>>> +set exefile $testfile
>>>>> +
>>>>> +if {[prepare_for_testing $testfile $exefile $srcfile {debug c++}]} {
>>>>> +    return -1
>>>>> +}
>>>>> +
>>>>> +gdb_exit
>>>>> +if [mi_gdb_start] {
>>>>> +    continue
>>>>> +}
>>>>> +
>>>>> +# Turn off the pending breakpoint queries.
>>>>> +mi_gdb_test "-interpreter-exec console \"set breakpoint pending off\"" \
>>>>> +  {=cmd-param-changed,param=\"breakpoint pending\",.*\^done} \
>>>>> +  "-interpreter-exec console \"set breakpoint pending off\""
>>>>> +
>>>>> +mi_run_to_main
>>>>> +
>>>>> +# Run to a location in the file.
>>>>> +set bp_location [gdb_get_line_number "set breakpoint here"]
>>>>> +
>>>>> +mi_gdb_test "-break-insert ${srcfile}:${bp_location}" \
>>>>> +    {\^done,bkpt=.number="2",type="breakpoint".*\}} "set breakpoint"
>>>>> +
>>>>> +mi_execute_to "exec-continue" "breakpoint-hit" "myfunction" ".*" ".*" "24" \
>>>>> +    { "" "disp=\"keep\"" } "breakpoint hit"
>>>>> +
>>>>> +# Set a breakpoint in a C++ source file whose name contains a
>>>>> +# Windows-style logical drive.
>>>>> +mi_gdb_test \
>>>>> +    "-break-insert -f \"c:/uu.cpp:13\"" \
>>>>> +    ".*No source file named c:/uu.cpp.*"
>>>>>
>>>>
>>>
>>
> 

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

* Re: [PING] Re: [PATCH v4] PR 18303, Tolerate malformed input for lookup_symbol-called functions
  2016-03-14 21:23                       ` [PING] Re: [PATCH v4] PR 18303, Tolerate malformed input for lookup_symbol-called functions (was: Re: [PATCH] Fix problem handling colon in linespec, PR breakpoints/18303) Don Breazeal
@ 2016-03-15 15:55                         ` Pedro Alves
  0 siblings, 0 replies; 17+ messages in thread
From: Pedro Alves @ 2016-03-15 15:55 UTC (permalink / raw)
  To: Don Breazeal, Keith Seitz, Doug Evans; +Cc: gdb-patches

On 03/14/2016 09:23 PM, Don Breazeal wrote:
> On 1/28/2016 4:06 AM, Pedro Alves wrote:
>> On 01/28/2016 01:21 AM, Don Breazeal wrote:
>>> The patch includes three new tests related to this.  One is just
>>> gdb.linespec/ls-errs.exp copied and converted to use C++ instead of
> C, and
>>> to add a case using a file name containing a Windows-style logical drive
>>> specifier.
>> ....
>>> gdb/testsuite/gdb.linespec/ls-errs-cp.cc    |  36 +++++
>>> gdb/testsuite/gdb.linespec/ls-errs-cp.exp   | 240
> ++++++++++++++++++++++++++++
>> ...
>>
>> Can't we somehow reuse the existing test?  Say, either:
>>
>>   - move the main body of ls-errs.exp a procedure, and call it twice,
>>     once for each language, or,
>>
>>   - make gdb.linespec/ls-errs-cp.exp set some $language var and then
>>     source gdb.linespec/ls-errs.exp, like gdb.base/checkpoint-ns.exp.
>>
>> Thanks,
>> Pedro Alves
>>
>
> Hi Pedro,
> I used your first suggestion above.  Did you have any other comments
> for this one, or is it good to go?

Sorry, that was really only a generic passer-by comment.  I'd
rather leave it to Doug or Keith to approve.

On the testcase part, it may help to post a "git diff -w" patch,
so we can see the real differences without all the reindentation
churn.

Thanks,
Pedro Alves

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

* Re: [PATCH v4] PR 18303, Tolerate malformed input for lookup_symbol-called functions.
  2016-03-15 20:30 Doug Evans
@ 2016-03-15 22:31 ` Don Breazeal
  0 siblings, 0 replies; 17+ messages in thread
From: Don Breazeal @ 2016-03-15 22:31 UTC (permalink / raw)
  To: Doug Evans, Keith Seitz; +Cc: gdb-patches

On 3/15/2016 1:30 PM, Doug Evans wrote:
> Don Breazeal writes:
>   > Apologies -- the previous version of this patch was missing one file,
>   > added here.  Sorry for the noise.
>   > --Don
>   >
>   > ---V3 Comment---
>   > This patch differs from v2 in that the new test  
> gdb.linespec/ls-errs-cp.exp
>   > is gone, and instead (per Pedro's suggestion) gdb.linespec/ls-errs.exp  
> has
>   > been modified to test both C & C++, and to incorporate the small amount  
> of
>   > extra testing from ls-errs-cp.exp.
>   >
>   > It also includes, unchanged from the previous version of the patch:
>   >
>   >  * an updated version of the alternate fix that Keith proposed for my
>   >    patch that addressed PR breakpoints/18303.  Doug has approved (in
>   >    concept, at least) the code portion of the patch.
>   >
>   >  * a couple of other new tests of colons in linespecs.
>   >
>   > Thanks,
>   > --Don
>   >
>   > -----------
>   > lookup_symbol is often called with user input.  Consequently, any
>   > function called from lookup_symbol{,_in_language} should attempt to
>   > deal with malformed input gracefully.  After all, malformed user
>   > input is not a programming/API error.
>   >
>   > This patch does not attempt to find/correct all instances of this.  It
>   > only fixes locations in the code that trigger test suite failures.
>   >
>   > This patch fixes PR breakpoints/18303, "Assertion: -breakpoint-insert
>   > with windows paths of file in non-current directory".
>   >
>   > The patch includes three new tests related to this.  One is just
>   > gdb.linespec/ls-errs.exp copied and converted to use C++ instead of C,  
> and
>   > to add a case using a file name containing a Windows-style logical drive
>   > specifier.  The others include an MI test to provide a regression test  
> for
>   > the specific case reported in PR 18303, and a C++ test for proper error
>   > handling of access to a program variable when using a file scope  
> specifier
>   > that refers to a non-existent file.
>   >
>   > Tested on x86_64 native Linux.
>   >
>   > gdb/ChangeLog
>   > 2016-01-28  Keith Seitz  <keiths@redhat.com>
>   >
>   > 	PR breakpoints/18303
>   > 	* cp-namespace.c (cp_lookup_bare_symbol): Change assertion to
>   > 	look for "::" instead of simply ":".
>   > 	(cp_search_static_and_baseclasses): Return null_block_symbol for
>   > 	malformed input.
>   > 	Remove assertions.
>   > 	* cp-support.c (cp_find_first_component_aux): Do not return
>   > 	a prefix length for ':' unless the next character is also ':'.
>   >
>   > gdb/testsuite/ChangeLog
>   > 2016-01-28  Don Breazeal  <donb@codesourcery.com>
>   >
>   > 	* gdb.cp/scope-err.cc: New test program.
>   > 	* gdb.cp/scope-err.exp: New test script.
>   > 	* gdb.linespec/ls-errs.c (myfunction): Expanded to have multiple
>   > 	lines and "set breakpoint here" comment.
>   > 	* gdb.linespec/ls-errs.exp: Added C++ testing and new test case.
>   > 	* gdb.mi/mi-linespec-err-cp.cc: New test program.
>   > 	* gdb.mi/mi-linespec-err-cp.exp: New test script.
>   >...
> 
> Hi.
> LGTM
> 
This is now pushed.  Keith FYI I made some whitespace/format changes per
your comments.
Thanks,
--Don

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

* Re: [PATCH v4] PR 18303, Tolerate malformed input for lookup_symbol-called functions.
@ 2016-03-15 20:30 Doug Evans
  2016-03-15 22:31 ` Don Breazeal
  0 siblings, 1 reply; 17+ messages in thread
From: Doug Evans @ 2016-03-15 20:30 UTC (permalink / raw)
  To: Don Breazeal; +Cc: gdb-patches

Don Breazeal writes:
  > Apologies -- the previous version of this patch was missing one file,
  > added here.  Sorry for the noise.
  > --Don
  >
  > ---V3 Comment---
  > This patch differs from v2 in that the new test  
gdb.linespec/ls-errs-cp.exp
  > is gone, and instead (per Pedro's suggestion) gdb.linespec/ls-errs.exp  
has
  > been modified to test both C & C++, and to incorporate the small amount  
of
  > extra testing from ls-errs-cp.exp.
  >
  > It also includes, unchanged from the previous version of the patch:
  >
  >  * an updated version of the alternate fix that Keith proposed for my
  >    patch that addressed PR breakpoints/18303.  Doug has approved (in
  >    concept, at least) the code portion of the patch.
  >
  >  * a couple of other new tests of colons in linespecs.
  >
  > Thanks,
  > --Don
  >
  > -----------
  > lookup_symbol is often called with user input.  Consequently, any
  > function called from lookup_symbol{,_in_language} should attempt to
  > deal with malformed input gracefully.  After all, malformed user
  > input is not a programming/API error.
  >
  > This patch does not attempt to find/correct all instances of this.  It
  > only fixes locations in the code that trigger test suite failures.
  >
  > This patch fixes PR breakpoints/18303, "Assertion: -breakpoint-insert
  > with windows paths of file in non-current directory".
  >
  > The patch includes three new tests related to this.  One is just
  > gdb.linespec/ls-errs.exp copied and converted to use C++ instead of C,  
and
  > to add a case using a file name containing a Windows-style logical drive
  > specifier.  The others include an MI test to provide a regression test  
for
  > the specific case reported in PR 18303, and a C++ test for proper error
  > handling of access to a program variable when using a file scope  
specifier
  > that refers to a non-existent file.
  >
  > Tested on x86_64 native Linux.
  >
  > gdb/ChangeLog
  > 2016-01-28  Keith Seitz  <keiths@redhat.com>
  >
  > 	PR breakpoints/18303
  > 	* cp-namespace.c (cp_lookup_bare_symbol): Change assertion to
  > 	look for "::" instead of simply ":".
  > 	(cp_search_static_and_baseclasses): Return null_block_symbol for
  > 	malformed input.
  > 	Remove assertions.
  > 	* cp-support.c (cp_find_first_component_aux): Do not return
  > 	a prefix length for ':' unless the next character is also ':'.
  >
  > gdb/testsuite/ChangeLog
  > 2016-01-28  Don Breazeal  <donb@codesourcery.com>
  >
  > 	* gdb.cp/scope-err.cc: New test program.
  > 	* gdb.cp/scope-err.exp: New test script.
  > 	* gdb.linespec/ls-errs.c (myfunction): Expanded to have multiple
  > 	lines and "set breakpoint here" comment.
  > 	* gdb.linespec/ls-errs.exp: Added C++ testing and new test case.
  > 	* gdb.mi/mi-linespec-err-cp.cc: New test program.
  > 	* gdb.mi/mi-linespec-err-cp.exp: New test script.
  >...

Hi.
LGTM

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

end of thread, other threads:[~2016-03-15 22:31 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-01-11 22:34 [PATCH] Fix problem handling colon in linespec, PR breakpoints/18303 Doug Evans
2016-01-12  0:17 ` Don Breazeal
2016-01-12 18:26 ` Keith Seitz
2016-01-25 17:26   ` [PING] " Don Breazeal
2016-01-26 16:56     ` Doug Evans
2016-01-28  1:21       ` [PATCH v2] PR 18303, Tolerate malformed input for lookup_symbol-called functions Don Breazeal
2016-01-28 12:06         ` Pedro Alves
2016-01-28 22:43           ` [PATCH v3] " Don Breazeal
2016-01-28 22:52             ` [PATCH v4] " Don Breazeal
2016-02-04 18:37               ` [PING] " Don Breazeal
2016-02-18 18:22                 ` [PING] Re: [PATCH v4] PR 18303, Tolerate malformed input for lookup_symbol-called functions (was: [PATCH] Fix problem handling colon in linespec, PR breakpoints/18303) Don Breazeal
2016-02-25 17:28                   ` [PING] Re: [PATCH v4] PR 18303, Tolerate malformed input for lookup_symbol-called functions Don Breazeal
2016-03-03 18:19                     ` Don Breazeal
2016-03-14 21:23                       ` [PING] Re: [PATCH v4] PR 18303, Tolerate malformed input for lookup_symbol-called functions (was: Re: [PATCH] Fix problem handling colon in linespec, PR breakpoints/18303) Don Breazeal
2016-03-15 15:55                         ` [PING] Re: [PATCH v4] PR 18303, Tolerate malformed input for lookup_symbol-called functions Pedro Alves
2016-03-15 20:30 Doug Evans
2016-03-15 22:31 ` Don Breazeal

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).