For patch submissions and discussion

Hui Zhu | 22 Jul 2012 08:30

Picon

[PATCH Bug breakpoints/14381] Fix linespec to parse file name that begin with decimal numbers

Hi,

This issue is because function linespec_lexer_lex_one handle the
string that begin with decimal numbers decimal numbers directly and
use linespec_lexer_lex_number handle it directly.
So when there are a file name that begin with decimal numbers, will a
lot of error around it.

I post a patch make linespec_lexer_lex_number if number followed by
non-space string, use linespec_lexer_lex_string handle linespec as a
string.

Joel, this issue affect 7.5.  Does this patch can add to 7.5 branch?

Thanks,
Hui

2012-07-22  Hui Zhu  <hui_zhu <at> mentor.com>

	* linespec.c (linespec_lexer_lex_number): Call
	linespec_lexer_lex_string if the number is followed by
	non-space string.

---
 linespec.c |    8 ++++++++
 1 file changed, 8 insertions(+)

--- a/linespec.c
+++ b/linespec.c
 <at>  <at>  -368,6 +368,8  <at>  <at>  static const char *const linespec_quote_

(Continue reading)

Jan Kratochvil | 22 Jul 2012 09:10

Picon

Re: [PATCH Bug breakpoints/14381] Fix linespec to parse file name that begin with decimal numbers

On Sun, 22 Jul 2012 08:30:02 +0200, Hui Zhu wrote:
> This issue is because function linespec_lexer_lex_one handle the
> string that begin with decimal numbers decimal numbers directly and
> use linespec_lexer_lex_number handle it directly.

There should be a testcase for it.  (Not a review yet.)

Thanks,
Jan

Hui Zhu | 22 Jul 2012 10:55

Picon

Re: [PATCH Bug breakpoints/14381] Fix linespec to parse file name that begin with decimal numbers

On Sun, Jul 22, 2012 at 3:10 PM, Jan Kratochvil
<jan.kratochvil <at> redhat.com> wrote:
> On Sun, 22 Jul 2012 08:30:02 +0200, Hui Zhu wrote:
>> This issue is because function linespec_lexer_lex_one handle the
>> string that begin with decimal numbers decimal numbers directly and
>> use linespec_lexer_lex_number handle it directly.
>
> There should be a testcase for it.  (Not a review yet.)
>
>
> Thanks,
> Jan

Agree.  I will add it later.

Thanks,
Hui

Hui Zhu | 22 Jul 2012 12:10

Picon

Re: [PATCH Bug breakpoints/14381] Fix linespec to parse file name that begin with decimal numbers

On Sun, Jul 22, 2012 at 4:55 PM, Hui Zhu <teawater <at> gmail.com> wrote:
> On Sun, Jul 22, 2012 at 3:10 PM, Jan Kratochvil
> <jan.kratochvil <at> redhat.com> wrote:
>> On Sun, 22 Jul 2012 08:30:02 +0200, Hui Zhu wrote:
>>> This issue is because function linespec_lexer_lex_one handle the
>>> string that begin with decimal numbers decimal numbers directly and
>>> use linespec_lexer_lex_number handle it directly.
>>
>> There should be a testcase for it.  (Not a review yet.)
>>
>>
>> Thanks,
>> Jan
>
> Agree.  I will add it later.
>
> Thanks,
> Hui

Hi,

I got a trouble with test.

This issue must build test code without dir, for example: "1.c".
But "gdb_compile" or "prepare_for_testing" will build the test code
with dir, for example:"src/gdb/testsuite/gdb.base/1.c".

Could you tell me how to handle it?

Thanks,

(Continue reading)

Jan Kratochvil | 22 Jul 2012 12:51

Picon

Re: [PATCH Bug breakpoints/14381] Fix linespec to parse file name that begin with decimal numbers

On Sun, 22 Jul 2012 12:10:45 +0200, Hui Zhu wrote:
> This issue must build test code without dir, for example: "1.c".
> But "gdb_compile" or "prepare_for_testing" will build the test code
> with dir, for example:"src/gdb/testsuite/gdb.base/1.c".
> 
> Could you tell me how to handle it?

Oops, OK, I did not expect difficulties.

The easier should be just to use TCL commands 'cd' (and restore it to 'pwd'),
and then do not pass $srcdir/$subdir to gdb_compile (you cannot use
build_executable and you also cannot use prepare_for_testing).

The more safe way against compiler output changes would be to generate DWARF.
This case should be the part
	compdir_absolute_ldir_missing__file_basename
from not yet committed patch/testcase:
	http://sourceware.org/ml/gdb-patches/2012-04/msg00106.html
so one could simplify that testcase; the testcase is a bit complex, though.

Thanks,
Jan

Tom Tromey | 23 Jul 2012 16:26

Picon

Re: [PATCH Bug breakpoints/14381] Fix linespec to parse file name that begin with decimal numbers

>>>>> ">" == Hui Zhu <teawater <at> gmail.com> writes:

>> This issue must build test code without dir, for example: "1.c".

Why is that?
It seems to me that the problem is in the linespec lexer, and that
"break 1.c:5" or whatever ought to work ok.

Tom

Keith Seitz | 23 Jul 2012 20:04

Picon

Re: [PATCH Bug breakpoints/14381] Fix linespec to parse file name that begin with decimal numbers

On 07/23/2012 07:26 AM, Tom Tromey wrote:
>>>>>> ">" == Hui Zhu <teawater <at> gmail.com> writes:
>
>>> This issue must build test code without dir, for example: "1.c".
>
> Why is that?
> It seems to me that the problem is in the linespec lexer, and that
> "break 1.c:5" or whatever ought to work ok.

It should also be pretty trivial to add a test for this case, too:

--- a/gdb/testsuite/gdb.linespec/ls-errs.exp
+++ b/gdb/testsuite/gdb.linespec/ls-errs.exp
 <at>  <at>  -124,7 +124,8  <at>  <at>  foreach x [list "this_file_doesn't_exist.c" \
                "'this 'file' has quotes.c'" \
                "\"this 'file' has quotes.c\"" \
                "\"spaces: and :colons.c\"" \
-              "'more: :spaces: :and  colons::.c'"] {
+              "'more: :spaces: :and  colons::.c'" \
+              "1.c"] {
      # Remove any quoting from FILENAME for the error message.
      add the_tests "$x:3" invalid_file [string trim $x \"']
  }

Keith

Keith Seitz | 22 Jul 2012 16:11

Picon

Re: [PATCH Bug breakpoints/14381] Fix linespec to parse file name that begin with decimal numbers

On 07/21/2012 11:30 PM, Hui Zhu wrote:
>  <at>  <at>  -390,6 +392,12  <at>  <at>  linespec_lexer_lex_number (linespec_pars
>         ++(PARSER_STREAM (parser));
>       }
>
> +  if (*PARSER_STREAM (parser) != '\0' && !isspace(*PARSER_STREAM (parser)))
> +    {
> +      PARSER_STREAM (parser) = LS_TOKEN_STOKEN (token).ptr;
> +      return linespec_lexer_lex_string (parser);
> +    }
> +
>     return token;
>   }
>

This obfuscates the meaning of linespec_lexer_lex_number. A better place 
to deal with this is in linespec_lexer_lex_one directly where the 
decision to call linespec_lexer_lex_number is made.

That can either be done by inspecting the input stream directly in 
*_lex_one or by having *_lex_number error (add a new error token type) 
and then having linespec_lexer_lex_one fallback to string lexing.

Keith

Hui Zhu | 22 Jul 2012 16:47

Picon

Re: [PATCH Bug breakpoints/14381] Fix linespec to parse file name that begin with decimal numbers

On Sun, Jul 22, 2012 at 10:11 PM, Keith Seitz <keiths <at> redhat.com> wrote:
> On 07/21/2012 11:30 PM, Hui Zhu wrote:
>>
>>  <at>  <at>  -390,6 +392,12  <at>  <at>  linespec_lexer_lex_number (linespec_pars
>>         ++(PARSER_STREAM (parser));
>>       }
>>
>> +  if (*PARSER_STREAM (parser) != '\0' && !isspace(*PARSER_STREAM
>> (parser)))
>> +    {
>> +      PARSER_STREAM (parser) = LS_TOKEN_STOKEN (token).ptr;
>> +      return linespec_lexer_lex_string (parser);
>> +    }
>> +
>>     return token;
>>   }
>>
>
> This obfuscates the meaning of linespec_lexer_lex_number. A better place to
> deal with this is in linespec_lexer_lex_one directly where the decision to
> call linespec_lexer_lex_number is made.
>
> That can either be done by inspecting the input stream directly in *_lex_one
> or by having *_lex_number error (add a new error token type) and then having
> linespec_lexer_lex_one fallback to string lexing.
>
> Keith

The string begin with a number is not a error, why handle it as a
error?  I think that will really make this part obfuscates.

(Continue reading)

Tom Tromey | 23 Jul 2012 16:28

Picon

Re: [PATCH Bug breakpoints/14381] Fix linespec to parse file name that begin with decimal numbers

>>>>> ">" == Hui Zhu <teawater <at> gmail.com> writes:

>> The string begin with a number is not a error, why handle it as a
>> error?  I think that will really make this part obfuscates.
>> What I thought is change the function name to
>> linespec_lexer_lex_number_string to make it clear.

How about changing linespec_lexer_lex_number to return a boolean
indicating whether it succeeded?  This is basically the same idea, but
without introducing a new token.

Tom

Keith Seitz | 23 Jul 2012 19:16

Picon

Re: [PATCH Bug breakpoints/14381] Fix linespec to parse file name that begin with decimal numbers

On 07/23/2012 07:28 AM, Tom Tromey wrote:
> How about changing linespec_lexer_lex_number to return a boolean
> indicating whether it succeeded?  This is basically the same idea, but
> without introducing a new token.

That's sounds even better than the sleep-deprived solution I came up with.

Keith

Hui Zhu | 24 Jul 2012 05:47

Picon

Re: [PATCH Bug breakpoints/14381] Fix linespec to parse file name that begin with decimal numbers

On Mon, Jul 23, 2012 at 10:28 PM, Tom Tromey <tromey <at> redhat.com> wrote:
>>>>>> ">" == Hui Zhu <teawater <at> gmail.com> writes:
>
>>> The string begin with a number is not a error, why handle it as a
>>> error?  I think that will really make this part obfuscates.
>>> What I thought is change the function name to
>>> linespec_lexer_lex_number_string to make it clear.
>
> How about changing linespec_lexer_lex_number to return a boolean
> indicating whether it succeeded?  This is basically the same idea, but
> without introducing a new token.
>
> Tom

Agree with you.  Post a new patch according to your comments.

Thanks,
Hui

2012-07-24  Hui Zhu  <hui_zhu <at> mentor.com>

	* linespec.c (linespec_lexer_lex_number): Update comments,
	change the return and add check to make sure the input is
	the decimal numbers.
	(linespec_lexer_lex_one): If linespec_lexer_lex_number return
	false, call linespec_lexer_lex_string.

---
 linespec.c |   28 ++++++++++++++++++----------
 1 file changed, 18 insertions(+), 10 deletions(-)

(Continue reading)

Tom Tromey | 24 Jul 2012 21:35

Picon

Re: [PATCH Bug breakpoints/14381] Fix linespec to parse file name that begin with decimal numbers

>>>>> ">" == Hui Zhu <teawater <at> gmail.com> writes:

>> 2012-07-24  Hui Zhu  <hui_zhu <at> mentor.com>
>> 	* linespec.c (linespec_lexer_lex_number): Update comments,
>> 	change the return and add check to make sure the input is
>> 	the decimal numbers.
>> 	(linespec_lexer_lex_one): If linespec_lexer_lex_number return
>> 	false, call linespec_lexer_lex_string.

>> +static int
>> +linespec_lexer_lex_number (linespec_parser *parser, linespec_token *tokenp)
>>  {
>>    linespec_token token;

After this change, I think 'token' is unused.

The patch is ok if you update it to remove this.

thanks,
Tom

Hui Zhu | 25 Jul 2012 14:56

Picon

Re: [PATCH Bug breakpoints/14381] Fix linespec to parse file name that begin with decimal numbers

On Wed, Jul 25, 2012 at 3:35 AM, Tom Tromey <tromey <at> redhat.com> wrote:
>>>>>> ">" == Hui Zhu <teawater <at> gmail.com> writes:
>
>>> 2012-07-24  Hui Zhu  <hui_zhu <at> mentor.com>
>>>      * linespec.c (linespec_lexer_lex_number): Update comments,
>>>      change the return and add check to make sure the input is
>>>      the decimal numbers.
>>>      (linespec_lexer_lex_one): If linespec_lexer_lex_number return
>>>      false, call linespec_lexer_lex_string.
>
>>> +static int
>>> +linespec_lexer_lex_number (linespec_parser *parser, linespec_token *tokenp)
>>>  {
>>>    linespec_token token;
>
> After this change, I think 'token' is unused.

Oops.

>
> The patch is ok if you update it to remove this.
>
> thanks,
> Tom

I fixed it and committed this patch to trunk and 7.5.

Thanks,
Hui

(Continue reading)

Keith Seitz | 25 Jul 2012 23:20

Picon

Re: [PATCH Bug breakpoints/14381] Fix linespec to parse file name that begin with decimal numbers

On 07/25/2012 05:56 AM, Hui Zhu wrote:

>>>> 2012-07-24  Hui Zhu  <hui_zhu <at> mentor.com>
>>>>       * linespec.c (linespec_lexer_lex_number): Update comments,
>>>>       change the return and add check to make sure the input is
>>>>       the decimal numbers.
>>>>       (linespec_lexer_lex_one): If linespec_lexer_lex_number return
>>>>       false, call linespec_lexer_lex_string.

This patch caused several regressions:

FAIL: gdb.base/dprintf.exp: dprintf 26,"arg=%d, g=%d\n", arg, g (got 
interactive prompt)
FAIL: gdb.base/dprintf.exp: 1st dprintf, gdb
FAIL: gdb.base/dprintf.exp: 2nd dprintf, gdb
FAIL: gdb.base/dprintf.exp: 1st dprintf, call
FAIL: gdb.base/dprintf.exp: 2nd dprintf, call
FAIL: gdb.base/dprintf.exp: 1st dprintf, fprintf
FAIL: gdb.base/dprintf.exp: 2nd dprintf, fprintf
FAIL: gdb.base/list.exp: list range; filename:line1,filename:line2
FAIL: gdb.base/list.exp: list range; line1,line2
FAIL: gdb.base/list.exp: list range; upper bound past EOF
FAIL: gdb.base/list.exp: list range; both bounds past EOF
FAIL: gdb.base/list.exp: list range, must be same files
FAIL: gdb.linespec/ls-errs.exp: break 3:
FAIL: gdb.linespec/ls-errs.exp: break +10:
FAIL: gdb.linespec/ls-errs.exp: break -10:
FAIL: gdb.linespec/ls-errs.exp: break 3:
FAIL: gdb.linespec/ls-errs.exp: break +10:
FAIL: gdb.linespec/ls-errs.exp: break -10:

(Continue reading)

Tom Tromey | 26 Jul 2012 15:52

Picon

Re: [PATCH Bug breakpoints/14381] Fix linespec to parse file name that begin with decimal numbers

>>>>> "Keith" == Keith Seitz <keiths <at> redhat.com> writes:

Keith> Ok for HEAD and 7.5?

One nit...

Keith> +      && !isspace(*PARSER_STREAM (parser)) && *PARSER_STREAM (parser) != ','

Space after "isspace".

Ok with that fix, thanks.

Tom

Keith Seitz | 26 Jul 2012 18:35

Picon

Re: [PATCH Bug breakpoints/14381] Fix linespec to parse file name that begin with decimal numbers

On 07/26/2012 06:52 AM, Tom Tromey wrote:
>>>>>> "Keith" == Keith Seitz <keiths <at> redhat.com> writes:
>
> Keith> Ok for HEAD and 7.5?
>
> One nit...
>
> Keith> +      && !isspace(*PARSER_STREAM (parser)) && *PARSER_STREAM (parser) != ','
>
> Space after "isspace"

Ha. I didn't even notice that had been committed.

> Ok with that fix, thanks.
>

Committed to HEAD and 7.5. Thank you for the review.

Keith

Return

Return to gmane.comp.gdb.patches.

Search Archive

Language

Change language

Options

Current view: Threads only / Showing only 20 lines / Not hiding cited text.
Change to All messages, whole messages, or hide cited text.

Post a message
NNTP Newsgroup
Classic Gmane web interface
XML

XML

RSS Feed
List Information

About Gmane