* [RFA 4/4] Constify parse_linesepc @ 2013-09-30 18:57 Keith Seitz 2013-10-01 4:15 ` Sergio Durigan Junior 2013-10-01 20:16 ` Tom Tromey 0 siblings, 2 replies; 17+ messages in thread From: Keith Seitz @ 2013-09-30 18:57 UTC (permalink / raw) To: gdb-patches@sourceware.org ml [-- Attachment #1: Type: text/plain, Size: 662 bytes --] Hi, This last patch const-ifies a fair bit of linespec.c around parse_linespec. Tested as in all previous patches. Keith ChangeLog 2013-09-24 Keith Seitz <keiths@redhat.com> * linespec.c (struct ls_parser): Make 'saved_arg' const. (parse_linespec): Make 'argptr' const. Remove temporary cast of 'argptr' to const char **. (decode_line_full): Pass const pointer to parse_linespec. (decode_line_1): Likewise. (decode_objc): Make local variable 'new_argptr' const. (find_function_symbols): Remove temporary cast to char * to find_imps. * objc-lang.c (find_imps): Make argument 'method' const. Return const. * objc-lang.h (find_imps): Likewise. [-- Attachment #2: constify-parse_linespec.patch --] [-- Type: text/x-patch, Size: 4843 bytes --] diff --git a/gdb/linespec.c b/gdb/linespec.c index 96f1d07..9468f26 100644 --- a/gdb/linespec.c +++ b/gdb/linespec.c @@ -278,7 +278,7 @@ struct ls_parser struct { /* Save head of input stream. */ - char *saved_arg; + const char *saved_arg; /* Head of the input stream. */ const char **stream; @@ -320,7 +320,7 @@ static CORE_ADDR linespec_expression_to_pc (const char **exp_ptr); static struct symtabs_and_lines decode_objc (struct linespec_state *self, linespec_p ls, - char **argptr); + const char **argptr); static VEC (symtab_ptr) *symtabs_from_filename (const char *); @@ -2144,7 +2144,7 @@ convert_linespec_to_sals (struct linespec_state *state, linespec_p ls) /* Parse the linespec in ARGPTR. */ static struct symtabs_and_lines -parse_linespec (linespec_parser *parser, char **argptr) +parse_linespec (linespec_parser *parser, const char **argptr) { linespec_token token; struct symtabs_and_lines values; @@ -2175,7 +2175,7 @@ parse_linespec (linespec_parser *parser, char **argptr) parser->keyword_ok = 0; parser->lexer.saved_arg = *argptr; - parser->lexer.stream = (const char **) argptr; + parser->lexer.stream = argptr; file_exception.reason = 0; /* Initialize the default symtab and line offset. */ @@ -2426,6 +2426,7 @@ decode_line_full (char **argptr, int flags, VEC (const_char_ptr) *filters = NULL; linespec_parser parser; struct linespec_state *state; + const char *copy, *orig; gdb_assert (canonical != NULL); /* The filter only makes sense for 'all'. */ @@ -2441,7 +2442,9 @@ decode_line_full (char **argptr, int flags, cleanups = make_cleanup (linespec_parser_delete, &parser); save_current_program_space (); - result = parse_linespec (&parser, argptr); + orig = copy = *argptr; + result = parse_linespec (&parser, ©); + *argptr += copy - orig; state = PARSER_STATE (&parser); gdb_assert (result.nelts == 1 || canonical->pre_expanded); @@ -2496,13 +2499,16 @@ decode_line_1 (char **argptr, int flags, struct symtabs_and_lines result; linespec_parser parser; struct cleanup *cleanups; + const char *copy, *orig; linespec_parser_new (&parser, flags, current_language, default_symtab, default_line, NULL); cleanups = make_cleanup (linespec_parser_delete, &parser); save_current_program_space (); - result = parse_linespec (&parser, argptr); + orig = copy = *argptr; + result = parse_linespec (&parser, ©); + *argptr += copy - orig; do_cleanups (cleanups); return result; @@ -2602,12 +2608,12 @@ linespec_expression_to_pc (const char **exp_ptr) the existing C++ code to let the user choose one. */ static struct symtabs_and_lines -decode_objc (struct linespec_state *self, linespec_p ls, char **argptr) +decode_objc (struct linespec_state *self, linespec_p ls, const char **argptr) { struct collect_info info; VEC (const_char_ptr) *symbol_names = NULL; struct symtabs_and_lines values; - char *new_argptr; + const char *new_argptr; struct cleanup *cleanup = make_cleanup (VEC_cleanup (const_char_ptr), &symbol_names); @@ -3053,7 +3059,7 @@ find_function_symbols (struct linespec_state *state, info.file_symtabs = file_symtabs; /* Try NAME as an Objective-C selector. */ - find_imps ((char *) name, &symbol_names); + find_imps (name, &symbol_names); if (!VEC_empty (const_char_ptr, symbol_names)) add_all_symbol_names_from_pspace (&info, NULL, symbol_names); else diff --git a/gdb/objc-lang.c b/gdb/objc-lang.c index cf99a0f..bcce435 100644 --- a/gdb/objc-lang.c +++ b/gdb/objc-lang.c @@ -1072,7 +1072,7 @@ uniquify_strings (VEC (const_char_ptr) **strings) } /* - * Function: find_imps (char *selector, struct symbol **sym_arr) + * Function: find_imps (const char *selector, struct symbol **sym_arr) * * Input: a string representing a selector * a pointer to an array of symbol pointers @@ -1101,8 +1101,8 @@ uniquify_strings (VEC (const_char_ptr) **strings) * be the index of the first non-debuggable one). */ -char * -find_imps (char *method, VEC (const_char_ptr) **symbol_names) +const char * +find_imps (const char *method, VEC (const_char_ptr) **symbol_names) { char type = '\0'; char *class = NULL; diff --git a/gdb/objc-lang.h b/gdb/objc-lang.h index 23fac1b..2409363 100644 --- a/gdb/objc-lang.h +++ b/gdb/objc-lang.h @@ -36,7 +36,8 @@ extern char *objc_demangle (const char *mangled, int options); extern int find_objc_msgcall (CORE_ADDR pc, CORE_ADDR *new_pc); -extern char *find_imps (char *method, VEC (const_char_ptr) **symbol_names); +extern const char * + find_imps (const char *method, VEC (const_char_ptr) **symbol_names); extern struct value *value_nsstring (struct gdbarch *gdbarch, char *ptr, int len); ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [RFA 4/4] Constify parse_linesepc 2013-09-30 18:57 [RFA 4/4] Constify parse_linesepc Keith Seitz @ 2013-10-01 4:15 ` Sergio Durigan Junior 2013-10-01 20:16 ` Tom Tromey 1 sibling, 0 replies; 17+ messages in thread From: Sergio Durigan Junior @ 2013-10-01 4:15 UTC (permalink / raw) To: Keith Seitz; +Cc: gdb-patches@sourceware.org ml On Monday, September 30 2013, Keith Seitz wrote: > Hi, > > This last patch const-ifies a fair bit of linespec.c around parse_linespec. Looks good too, but I can't approve it. Thanks again! > Tested as in all previous patches. > > Keith > > ChangeLog > 2013-09-24 Keith Seitz <keiths@redhat.com> > > * linespec.c (struct ls_parser): Make 'saved_arg' const. > (parse_linespec): Make 'argptr' const. > Remove temporary cast of 'argptr' to const char **. > (decode_line_full): Pass const pointer to parse_linespec. > (decode_line_1): Likewise. > (decode_objc): Make local variable 'new_argptr' const. > (find_function_symbols): Remove temporary cast to char * > to find_imps. > * objc-lang.c (find_imps): Make argument 'method' const. > Return const. > * objc-lang.h (find_imps): Likewise. > diff --git a/gdb/linespec.c b/gdb/linespec.c > index 96f1d07..9468f26 100644 > --- a/gdb/linespec.c > +++ b/gdb/linespec.c > @@ -278,7 +278,7 @@ struct ls_parser > struct > { > /* Save head of input stream. */ > - char *saved_arg; > + const char *saved_arg; > > /* Head of the input stream. */ > const char **stream; > @@ -320,7 +320,7 @@ static CORE_ADDR linespec_expression_to_pc (const char **exp_ptr); > > static struct symtabs_and_lines decode_objc (struct linespec_state *self, > linespec_p ls, > - char **argptr); > + const char **argptr); > > static VEC (symtab_ptr) *symtabs_from_filename (const char *); > > @@ -2144,7 +2144,7 @@ convert_linespec_to_sals (struct linespec_state *state, linespec_p ls) > /* Parse the linespec in ARGPTR. */ > > static struct symtabs_and_lines > -parse_linespec (linespec_parser *parser, char **argptr) > +parse_linespec (linespec_parser *parser, const char **argptr) > { > linespec_token token; > struct symtabs_and_lines values; > @@ -2175,7 +2175,7 @@ parse_linespec (linespec_parser *parser, char **argptr) > parser->keyword_ok = 0; > > parser->lexer.saved_arg = *argptr; > - parser->lexer.stream = (const char **) argptr; > + parser->lexer.stream = argptr; > file_exception.reason = 0; > > /* Initialize the default symtab and line offset. */ > @@ -2426,6 +2426,7 @@ decode_line_full (char **argptr, int flags, > VEC (const_char_ptr) *filters = NULL; > linespec_parser parser; > struct linespec_state *state; > + const char *copy, *orig; > > gdb_assert (canonical != NULL); > /* The filter only makes sense for 'all'. */ > @@ -2441,7 +2442,9 @@ decode_line_full (char **argptr, int flags, > cleanups = make_cleanup (linespec_parser_delete, &parser); > save_current_program_space (); > > - result = parse_linespec (&parser, argptr); > + orig = copy = *argptr; > + result = parse_linespec (&parser, ©); > + *argptr += copy - orig; > state = PARSER_STATE (&parser); > > gdb_assert (result.nelts == 1 || canonical->pre_expanded); > @@ -2496,13 +2499,16 @@ decode_line_1 (char **argptr, int flags, > struct symtabs_and_lines result; > linespec_parser parser; > struct cleanup *cleanups; > + const char *copy, *orig; > > linespec_parser_new (&parser, flags, current_language, default_symtab, > default_line, NULL); > cleanups = make_cleanup (linespec_parser_delete, &parser); > save_current_program_space (); > > - result = parse_linespec (&parser, argptr); > + orig = copy = *argptr; > + result = parse_linespec (&parser, ©); > + *argptr += copy - orig; > > do_cleanups (cleanups); > return result; > @@ -2602,12 +2608,12 @@ linespec_expression_to_pc (const char **exp_ptr) > the existing C++ code to let the user choose one. */ > > static struct symtabs_and_lines > -decode_objc (struct linespec_state *self, linespec_p ls, char **argptr) > +decode_objc (struct linespec_state *self, linespec_p ls, const char **argptr) > { > struct collect_info info; > VEC (const_char_ptr) *symbol_names = NULL; > struct symtabs_and_lines values; > - char *new_argptr; > + const char *new_argptr; > struct cleanup *cleanup = make_cleanup (VEC_cleanup (const_char_ptr), > &symbol_names); > > @@ -3053,7 +3059,7 @@ find_function_symbols (struct linespec_state *state, > info.file_symtabs = file_symtabs; > > /* Try NAME as an Objective-C selector. */ > - find_imps ((char *) name, &symbol_names); > + find_imps (name, &symbol_names); > if (!VEC_empty (const_char_ptr, symbol_names)) > add_all_symbol_names_from_pspace (&info, NULL, symbol_names); > else > diff --git a/gdb/objc-lang.c b/gdb/objc-lang.c > index cf99a0f..bcce435 100644 > --- a/gdb/objc-lang.c > +++ b/gdb/objc-lang.c > @@ -1072,7 +1072,7 @@ uniquify_strings (VEC (const_char_ptr) **strings) > } > > /* > - * Function: find_imps (char *selector, struct symbol **sym_arr) > + * Function: find_imps (const char *selector, struct symbol **sym_arr) > * > * Input: a string representing a selector > * a pointer to an array of symbol pointers > @@ -1101,8 +1101,8 @@ uniquify_strings (VEC (const_char_ptr) **strings) > * be the index of the first non-debuggable one). > */ > > -char * > -find_imps (char *method, VEC (const_char_ptr) **symbol_names) > +const char * > +find_imps (const char *method, VEC (const_char_ptr) **symbol_names) > { > char type = '\0'; > char *class = NULL; > diff --git a/gdb/objc-lang.h b/gdb/objc-lang.h > index 23fac1b..2409363 100644 > --- a/gdb/objc-lang.h > +++ b/gdb/objc-lang.h > @@ -36,7 +36,8 @@ extern char *objc_demangle (const char *mangled, int options); > > extern int find_objc_msgcall (CORE_ADDR pc, CORE_ADDR *new_pc); > > -extern char *find_imps (char *method, VEC (const_char_ptr) **symbol_names); > +extern const char * > + find_imps (const char *method, VEC (const_char_ptr) **symbol_names); > > extern struct value *value_nsstring (struct gdbarch *gdbarch, > char *ptr, int len); -- Sergio ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [RFA 4/4] Constify parse_linesepc 2013-09-30 18:57 [RFA 4/4] Constify parse_linesepc Keith Seitz 2013-10-01 4:15 ` Sergio Durigan Junior @ 2013-10-01 20:16 ` Tom Tromey 2013-10-02 4:38 ` Keith Seitz 1 sibling, 1 reply; 17+ messages in thread From: Tom Tromey @ 2013-10-01 20:16 UTC (permalink / raw) To: Keith Seitz; +Cc: gdb-patches@sourceware.org ml >>>>> "Keith" == Keith Seitz <keiths@redhat.com> writes: Keith> This last patch const-ifies a fair bit of linespec.c around Keith> parse_linespec. Thanks. This is ok. Keith> - parser->lexer.stream = (const char **) argptr; Keith> + parser->lexer.stream = argptr; Aha, it is fixed here :) Super. Tom ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [RFA 4/4] Constify parse_linesepc 2013-10-01 20:16 ` Tom Tromey @ 2013-10-02 4:38 ` Keith Seitz 2013-10-16 9:57 ` Regression for gdb.pascal/* [Re: [RFA 4/4] Constify parse_linesepc] Jan Kratochvil 0 siblings, 1 reply; 17+ messages in thread From: Keith Seitz @ 2013-10-02 4:38 UTC (permalink / raw) To: gdb-patches@sourceware.org ml On 10/01/2013 01:16 PM, Tom Tromey wrote: >>>>>> "Keith" == Keith Seitz <keiths@redhat.com> writes: > Keith> - parser->lexer.stream = (const char **) argptr; > Keith> + parser->lexer.stream = argptr; > > Aha, it is fixed here :) Super. Yep, there it is. I told you I'd get to it by the patchset's end. :-) All committed. Thank you Sergio and Tom for looking at this! Keith ^ permalink raw reply [flat|nested] 17+ messages in thread
* Regression for gdb.pascal/* [Re: [RFA 4/4] Constify parse_linesepc] 2013-10-02 4:38 ` Keith Seitz @ 2013-10-16 9:57 ` Jan Kratochvil 2013-10-16 22:07 ` Sergio Durigan Junior 0 siblings, 1 reply; 17+ messages in thread From: Jan Kratochvil @ 2013-10-16 9:57 UTC (permalink / raw) To: Keith Seitz; +Cc: gdb-patches@sourceware.org ml On Wed, 02 Oct 2013 06:38:28 +0200, Keith Seitz wrote: > All committed. Thank you Sergio and Tom for looking at this! c85cddc51d5d9e4423509a2dc7cf3d9809451b49 is the first bad commit commit c85cddc51d5d9e4423509a2dc7cf3d9809451b49 Author: Keith Seitz <keiths@redhat.com> Date: Wed Oct 2 00:46:06 2013 +0000 Constification of parse_linespec and fallout: https://sourceware.org/ml/gdb-patches/2013-09/msg01017.html https://sourceware.org/ml/gdb-patches/2013-09/msg01018.html https://sourceware.org/ml/gdb-patches/2013-09/msg01019.html https://sourceware.org/ml/gdb-patches/2013-09/msg01020.html You need fpc installed. -PASS: gdb.pascal/floats.exp: print 2 * r -PASS: gdb.pascal/floats.exp: print 2.0 * r +FAIL: gdb.pascal/floats.exp: print 2 * r +FAIL: gdb.pascal/floats.exp: print 2.0 * r -PASS: gdb.pascal/floats.exp: print 35 / 2 +FAIL: gdb.pascal/floats.exp: print 35 / 2 -PASS: gdb.pascal/integers.exp: print i + 1 = j +FAIL: gdb.pascal/integers.exp: print i + 1 = j -PASS: gdb.pascal/integers.exp: print i + 1 < j -PASS: gdb.pascal/integers.exp: print i + 1 <= j -PASS: gdb.pascal/integers.exp: print i + 1 > j -PASS: gdb.pascal/integers.exp: print i + 1 >= j -PASS: gdb.pascal/integers.exp: print 2 * i +FAIL: gdb.pascal/integers.exp: print i + 1 < j +FAIL: gdb.pascal/integers.exp: print i + 1 <= j +FAIL: gdb.pascal/integers.exp: print i + 1 > j +FAIL: gdb.pascal/integers.exp: print i + 1 >= j +FAIL: gdb.pascal/integers.exp: print 2 * i -PASS: gdb.pascal/integers.exp: print 35 div 2 -PASS: gdb.pascal/integers.exp: print 35 mod 2 -PASS: gdb.pascal/integers.exp: print i+10*j+100*k -PASS: gdb.pascal/integers.exp: print (i + 5) * (j + 7) +FAIL: gdb.pascal/integers.exp: print 35 div 2 +FAIL: gdb.pascal/integers.exp: print 35 mod 2 +FAIL: gdb.pascal/integers.exp: print i+10*j+100*k +FAIL: gdb.pascal/integers.exp: print (i + 5) * (j + 7) Jan ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: Regression for gdb.pascal/* [Re: [RFA 4/4] Constify parse_linesepc] 2013-10-16 9:57 ` Regression for gdb.pascal/* [Re: [RFA 4/4] Constify parse_linesepc] Jan Kratochvil @ 2013-10-16 22:07 ` Sergio Durigan Junior 2013-10-16 23:40 ` Sergio Durigan Junior 0 siblings, 1 reply; 17+ messages in thread From: Sergio Durigan Junior @ 2013-10-16 22:07 UTC (permalink / raw) To: Jan Kratochvil; +Cc: Keith Seitz, gdb-patches@sourceware.org ml On Wednesday, October 16 2013, Jan Kratochvil wrote: > On Wed, 02 Oct 2013 06:38:28 +0200, Keith Seitz wrote: >> All committed. Thank you Sergio and Tom for looking at this! > > c85cddc51d5d9e4423509a2dc7cf3d9809451b49 is the first bad commit > commit c85cddc51d5d9e4423509a2dc7cf3d9809451b49 > Author: Keith Seitz <keiths@redhat.com> > Date: Wed Oct 2 00:46:06 2013 +0000 > > Constification of parse_linespec and fallout: > https://sourceware.org/ml/gdb-patches/2013-09/msg01017.html > https://sourceware.org/ml/gdb-patches/2013-09/msg01018.html > https://sourceware.org/ml/gdb-patches/2013-09/msg01019.html > https://sourceware.org/ml/gdb-patches/2013-09/msg01020.html Thanks for the report. The problem happens because when yylex is going to parse a number, it starts by saving the pointer to the input string in a variable "p", then it advances this variable depending on what it finds, and finally it updates "lexptr" by using "p". However, "p" is "tokstart" at the beginning of the function, which used to be the same pointer as "lexptr", but now it's allocated using "alloca". I chose to fix this by advancing "p" and "lexptr" by the same amounts every time, so that they do not get out of sync (which also makes it unnecessary to update "lexptr" in the end). I ran the failing tests and they pass now. I am running a regression test now, but it will take a long time to complete, so I am sending this patch for appreciation first. OK to apply? -- Sergio gdb/ 2013-10-16 Sergio Durigan Junior <sergiodj@redhat.com> Fix regression by Keith's patch (Constification of parse_linesepc and fallout). * p-exp.y (yylex): Advance "lexptr" along with "p" when parsing a number. Do not update "lexptr" using "p" in the end. diff --git a/gdb/p-exp.y b/gdb/p-exp.y index de14cbb..41628ea 100644 --- a/gdb/p-exp.y +++ b/gdb/p-exp.y @@ -1264,22 +1264,27 @@ yylex (void) { /* It's a number. */ int got_dot = 0, got_e = 0, toktype; + /* p and lexptr point to the same string, but not to the same + location. They should be both advanced at the same time + so that they do not get out of sync. */ char *p = tokstart; int hex = input_radix > 10; if (c == '0' && (p[1] == 'x' || p[1] == 'X')) { p += 2; + lexptr += 2; hex = 1; } else if (c == '0' && (p[1]=='t' || p[1]=='T' || p[1]=='d' || p[1]=='D')) { p += 2; + lexptr += 2; hex = 0; } - for (;; ++p) + for (;; ++p, ++lexptr) { /* This test includes !hex because 'e' is a valid hex digit and thus does not indicate a floating point number when @@ -1312,7 +1317,6 @@ yylex (void) err_copy[p - tokstart] = 0; error (_("Invalid number \"%s\"."), err_copy); } - lexptr = p; return toktype; } ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: Regression for gdb.pascal/* [Re: [RFA 4/4] Constify parse_linesepc] 2013-10-16 22:07 ` Sergio Durigan Junior @ 2013-10-16 23:40 ` Sergio Durigan Junior 2013-10-17 18:18 ` Keith Seitz 0 siblings, 1 reply; 17+ messages in thread From: Sergio Durigan Junior @ 2013-10-16 23:40 UTC (permalink / raw) To: Jan Kratochvil; +Cc: Keith Seitz, gdb-patches@sourceware.org ml On Wednesday, October 16 2013, I wrote: > On Wednesday, October 16 2013, Jan Kratochvil wrote: > >> On Wed, 02 Oct 2013 06:38:28 +0200, Keith Seitz wrote: >>> All committed. Thank you Sergio and Tom for looking at this! >> >> c85cddc51d5d9e4423509a2dc7cf3d9809451b49 is the first bad commit >> commit c85cddc51d5d9e4423509a2dc7cf3d9809451b49 >> Author: Keith Seitz <keiths@redhat.com> >> Date: Wed Oct 2 00:46:06 2013 +0000 >> >> Constification of parse_linespec and fallout: >> https://sourceware.org/ml/gdb-patches/2013-09/msg01017.html >> https://sourceware.org/ml/gdb-patches/2013-09/msg01018.html >> https://sourceware.org/ml/gdb-patches/2013-09/msg01019.html >> https://sourceware.org/ml/gdb-patches/2013-09/msg01020.html > > Thanks for the report. > > The problem happens because when yylex is going to parse a number, it > starts by saving the pointer to the input string in a variable "p", then > it advances this variable depending on what it finds, and finally it > updates "lexptr" by using "p". However, "p" is "tokstart" at the > beginning of the function, which used to be the same pointer as > "lexptr", but now it's allocated using "alloca". > > I chose to fix this by advancing "p" and "lexptr" by the same amounts > every time, so that they do not get out of sync (which also makes it > unnecessary to update "lexptr" in the end). > > I ran the failing tests and they pass now. I am running a regression > test now, but it will take a long time to complete, so I am sending this > patch for appreciation first. > > OK to apply? I had a brief chat with Keith, who updated me on this. According to him, there are many more places that need to be updated. He's already talking to Pierre about this, so probably my patch is not complete/correct. Thus, feel free to drop it if it is the case. Thanks, -- Sergio ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: Regression for gdb.pascal/* [Re: [RFA 4/4] Constify parse_linesepc] 2013-10-16 23:40 ` Sergio Durigan Junior @ 2013-10-17 18:18 ` Keith Seitz 2013-10-17 20:52 ` Tom Tromey 2013-10-18 19:34 ` Regression for gdb.pascal/* [Re: [RFA 4/4] Constify parse_linesepc] Jan Kratochvil 0 siblings, 2 replies; 17+ messages in thread From: Keith Seitz @ 2013-10-17 18:18 UTC (permalink / raw) To: gdb-patches@sourceware.org ml; +Cc: Sergio Durigan Junior, Jan Kratochvil [-- Attachment #1: Type: text/plain, Size: 2367 bytes --] On 10/16/2013 04:40 PM, Sergio Durigan Junior wrote: > I had a brief chat with Keith, who updated me on this. According to > him, there are many more places that need to be updated. He's already > talking to Pierre about this, so probably my patch is not > complete/correct. Thus, feel free to drop it if it is the case. Yeah, my apologies. I *thought* I had a pascal compiler installed, but I didn't. [Maybe I was thinking of Fortran?] In any case, my previous patch to pascal_lex does some really nasty things, like assigning to lexptr/stoken from an alloca'd block. Boo! This patch fixes the regressions by doing what I should have done the first time around: it changes pascal_lex to really take const input. There are two little sections of code, though, which violate const-ness of the input, and I've removed those, since they don't seem necessary. [This is the two loops that deal with changing the case of `tokstart' -- which can easily be removed because we already have a temporary buffer that is used for this.] I could not think of any reasons why pascal_lex would need to change the input. The only thing that came to mind was completion, and the behavior for that is, as far as I can tell, identical to how 7.0, 7.3, 7.4, 7.5, and 7.6 behave. [both case-sensitive 'on' and 'off'] [Perhaps Pierre knows? It was added here: commit 458d424149528912ee72f6b9b9a2afd578a17715 Author: Pierre Muller <muller@ics.u-strasbg.fr> Date: Fri Nov 9 09:46:40 2001 +0000 2001-11-06 Pierre Muller <muller@ics.u-strasbg.fr> * p-exp.y (yylex): Only change case of expression if symbol is found. Also check for GPC standard name form. ] In any case, IMO parsers should not be changing their input. If this turns out to be necessary, we should extend the API to support it. Comments? Keith ChangeLog 2013-10-17 Keith Seitz <keiths@redhat.com> * p-exp.y (uptok): Make first parameter const. (yylex): Make `tokstart' and `tokptr' const. Don't copy the lexer input to a temporary buffer. Make `p' const. Remove const workaround ofr parse_escape. Create a temporary buffer for a convenience variable instead of doing in-place modification of the input. If a match is found with a different case from the input, do not change the input at all. Use `tmp' to construct the resultant stoken instead of `tokstart'. [-- Attachment #2: p-exp_y-regressions.patch --] [-- Type: text/x-patch, Size: 3864 bytes --] diff --git a/gdb/p-exp.y b/gdb/p-exp.y index de14cbb..8cb98c0 100644 --- a/gdb/p-exp.y +++ b/gdb/p-exp.y @@ -125,7 +125,7 @@ static int yylex (void); void yyerror (char *); -static char * uptok (char *, int); +static char *uptok (const char *, int); %} /* Although the yacc "value" of an expression is not used, @@ -1105,7 +1105,7 @@ static const struct token tokentab2[] = /* Allocate uppercased var: */ /* make an uppercased copy of tokstart. */ static char * -uptok (char *tokstart, int namelen) +uptok (const char *tokstart, int namelen) { int i; char *uptokstart = (char *)malloc(namelen+1); @@ -1133,9 +1133,9 @@ yylex (void) int c; int namelen; unsigned int i; - char *tokstart; + const char *tokstart; char *uptokstart; - char *tokptr; + const char *tokptr; int explen, tempbufindex; static char *tempbuf; static int tempbufsize; @@ -1146,9 +1146,8 @@ yylex (void) prev_lexptr = lexptr; + tokstart = lexptr; explen = strlen (lexptr); - tokstart = alloca (explen + 1); - memcpy (tokstart, lexptr, explen + 1); /* See if it is a special token of length 3. */ if (explen > 2) @@ -1264,7 +1263,7 @@ yylex (void) { /* It's a number. */ int got_dot = 0, got_e = 0, toktype; - char *p = tokstart; + const char *p = tokstart; int hex = input_radix > 10; if (c == '0' && (p[1] == 'x' || p[1] == 'X')) @@ -1369,11 +1368,8 @@ yylex (void) break; case '\\': { - const char *s, *o; - - o = s = ++tokptr; - c = parse_escape (parse_gdbarch, &s); - *tokptr += s - o; + ++tokptr; + c = parse_escape (parse_gdbarch, &tokptr); if (c == -1) { continue; @@ -1511,17 +1507,17 @@ yylex (void) if (*tokstart == '$') { - char c; + char *tmp; + /* $ is the normal prefix for pascal hexadecimal values but this conflicts with the GDB use for debugger variables so in expression to enter hexadecimal values we still need to use C syntax with 0xff */ write_dollar_variable (yylval.sval); - c = tokstart[namelen]; - tokstart[namelen] = 0; - intvar = lookup_only_internalvar (++tokstart); - --tokstart; - tokstart[namelen] = c; + tmp = alloca (namelen + 1); + memcpy (tmp, tokstart, namelen); + tmp[namelen] = '\0'; + intvar = lookup_only_internalvar (tmp + 1); free (uptokstart); return VARIABLE; } @@ -1561,12 +1557,6 @@ yylex (void) else sym = lookup_symbol (tmp, expression_context_block, VAR_DOMAIN, &is_a_field_of_this); - if (sym || is_a_field_of_this.type != NULL || is_a_field) - for (i = 0; i <= namelen; i++) - { - if ((tokstart[i] >= 'a' && tokstart[i] <= 'z')) - tokstart[i] -= ('a'-'A'); - } } /* Third chance Capitalized (as GPC does). */ if (!sym && is_a_field_of_this.type == NULL && !is_a_field) @@ -1589,24 +1579,13 @@ yylex (void) else sym = lookup_symbol (tmp, expression_context_block, VAR_DOMAIN, &is_a_field_of_this); - if (sym || is_a_field_of_this.type != NULL || is_a_field) - for (i = 0; i <= namelen; i++) - { - if (i == 0) - { - if ((tokstart[i] >= 'a' && tokstart[i] <= 'z')) - tokstart[i] -= ('a'-'A'); - } - else - if ((tokstart[i] >= 'A' && tokstart[i] <= 'Z')) - tokstart[i] -= ('A'-'a'); - } } if (is_a_field) { tempbuf = (char *) realloc (tempbuf, namelen + 1); - strncpy (tempbuf, tokstart, namelen); tempbuf [namelen] = 0; + strncpy (tempbuf, tmp, namelen); + tempbuf [namelen] = 0; yylval.sval.ptr = tempbuf; yylval.sval.length = namelen; free (uptokstart); ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: Regression for gdb.pascal/* [Re: [RFA 4/4] Constify parse_linesepc] 2013-10-17 18:18 ` Keith Seitz @ 2013-10-17 20:52 ` Tom Tromey 2013-10-18 17:20 ` Jan Kratochvil 2013-10-18 19:34 ` Regression for gdb.pascal/* [Re: [RFA 4/4] Constify parse_linesepc] Jan Kratochvil 1 sibling, 1 reply; 17+ messages in thread From: Tom Tromey @ 2013-10-17 20:52 UTC (permalink / raw) To: Keith Seitz Cc: gdb-patches@sourceware.org ml, Sergio Durigan Junior, Jan Kratochvil >>>>> "Keith" == Keith Seitz <keiths@redhat.com> writes: Keith> There are two little sections of code, though, which violate Keith> const-ness of the input, and I've removed those, since they don't Keith> seem necessary. [This is the two loops that deal with changing Keith> the case of `tokstart' -- which can easily be removed because we Keith> already have a temporary buffer that is used for this.] I think it's somewhat wrong to do this stuff in the parser anyhow. Ideally the symbol table ought to know that pascal is case-insensitive. I think we faced this with Fortran as well. Keith> [Perhaps Pierre knows? It was added here: Definitely need a reply from Pierre... Keith> - for (i = 0; i <= namelen; i++) Keith> - { Keith> - if ((tokstart[i] >= 'a' && tokstart[i] <= 'z')) Keith> - tokstart[i] -= ('a'-'A'); Keith> - } Not your problem, Keith, but there's other code in that file that ought to be using the ctypes macros instead of this stuff. Tom ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: Regression for gdb.pascal/* [Re: [RFA 4/4] Constify parse_linesepc] 2013-10-17 20:52 ` Tom Tromey @ 2013-10-18 17:20 ` Jan Kratochvil 2013-10-18 19:09 ` [pascal patch] Use case_sensitive_off [Re: Regression for gdb.pascal/* [Re: [RFA 4/4] Constify parse_linesepc]] Jan Kratochvil 0 siblings, 1 reply; 17+ messages in thread From: Jan Kratochvil @ 2013-10-18 17:20 UTC (permalink / raw) To: Pierre Muller Cc: Keith Seitz, gdb-patches@sourceware.org ml, Sergio Durigan Junior, Tom Tromey On Thu, 17 Oct 2013 22:51:55 +0200, Tom Tromey wrote: > >>>>> "Keith" == Keith Seitz <keiths@redhat.com> writes: > > Keith> There are two little sections of code, though, which violate > Keith> const-ness of the input, and I've removed those, since they don't > Keith> seem necessary. [This is the two loops that deal with changing > Keith> the case of `tokstart' -- which can easily be removed because we > Keith> already have a temporary buffer that is used for this.] > > I think it's somewhat wrong to do this stuff in the parser anyhow. > Ideally the symbol table ought to know that pascal is case-insensitive. > I think we faced this with Fortran as well. Case insensitive symbols should be supported (originally written for Fortran) since: commit 5b7743a275e4610fe6ea57f0c61e317490ee6854 Date: Wed Apr 27 20:03:03 2011 +0000 Case insensitive lookups implementation. (+about one fallout later) While f_language_defn has case_sensitive_off I see that pascal_language_defn has case_sensitive_on. At https://en.wikipedia.org/wiki/Pascal_%28programming_language%29#Hello_world I read Letter case is ignored in Pascal source. so I think pascal_language_defn really should use case_sensitive_off. Jan ^ permalink raw reply [flat|nested] 17+ messages in thread
* [pascal patch] Use case_sensitive_off [Re: Regression for gdb.pascal/* [Re: [RFA 4/4] Constify parse_linesepc]] 2013-10-18 17:20 ` Jan Kratochvil @ 2013-10-18 19:09 ` Jan Kratochvil 0 siblings, 0 replies; 17+ messages in thread From: Jan Kratochvil @ 2013-10-18 19:09 UTC (permalink / raw) To: Pierre Muller Cc: Keith Seitz, gdb-patches@sourceware.org ml, Sergio Durigan Junior, Tom Tromey On Fri, 18 Oct 2013 19:20:32 +0200, Jan Kratochvil wrote: > Case insensitive symbols should be supported (originally written for Fortran) > since: > commit 5b7743a275e4610fe6ea57f0c61e317490ee6854 > Date: Wed Apr 27 20:03:03 2011 +0000 > Case insensitive lookups implementation. > (+about one fallout later) > > While f_language_defn has case_sensitive_off > I see that pascal_language_defn has case_sensitive_on. p-exp.y currently contains: sym = lookup_symbol (tmp, expression_context_block, /* second chance uppercased (as Free Pascal does). */ sym = lookup_symbol (tmp, expression_context_block, /* Third chance Capitalized (as GPC does). */ sym = lookup_symbol (tmp, expression_context_block, which works for fpc -g or fpc -gw2 capitalizing the symbols. But it no longer works for -gw3 or -gw4 where fpc keeps the original case: <1><83>: Abbrev Number: 2 (DW_TAG_variable) <84> DW_AT_name : st with "set case-sensitive off" it works fine: (gdb) p st $1 = 0x626700 #13'Hello, world!' (gdb) p sT No symbol "sT" in current context. (gdb) set case-sensitive off warning: the current case sensitivity setting does not match the language. (gdb) p sT $2 = 0x626700 #13'Hello, world!' I do not know much about Pascal in GDB but do you think this is OK? The patch is on top of Keith's one. Thanks, Jan gdb/ 2013-10-18 Jan Kratochvil <jan.kratochvil@redhat.com> * p-exp.y (uptok): Remove variable i, remove the uppercasing. Update function comment. (yylex): Replace strcmp calls for strcasecmp. Remove uppercasing and capitalizing of symbols. * p-lang.c (pascal_language_defn): Use case_sensitive_off. diff --git a/gdb/p-exp.y b/gdb/p-exp.y index 8cb98c0..af338e1 100644 --- a/gdb/p-exp.y +++ b/gdb/p-exp.y @@ -1102,20 +1102,14 @@ static const struct token tokentab2[] = {":=", ASSIGN, BINOP_END}, {"::", COLONCOLON, BINOP_END} }; -/* Allocate uppercased var: */ -/* make an uppercased copy of tokstart. */ +/* Allocate a duplicate of NAMELEN length. */ + static char * uptok (const char *tokstart, int namelen) { - int i; char *uptokstart = (char *)malloc(namelen+1); - for (i = 0;i <= namelen;i++) - { - if ((tokstart[i]>='a' && tokstart[i]<='z')) - uptokstart[i] = tokstart[i]-('a'-'A'); - else - uptokstart[i] = tokstart[i]; - } + + memcpy (uptokstart, tokstart, namelen); uptokstart[namelen]='\0'; return uptokstart; } @@ -1448,29 +1442,29 @@ yylex (void) switch (namelen) { case 6: - if (strcmp (uptokstart, "OBJECT") == 0) + if (strcasecmp (uptokstart, "OBJECT") == 0) { free (uptokstart); return CLASS; } - if (strcmp (uptokstart, "RECORD") == 0) + if (strcasecmp (uptokstart, "RECORD") == 0) { free (uptokstart); return STRUCT; } - if (strcmp (uptokstart, "SIZEOF") == 0) + if (strcasecmp (uptokstart, "SIZEOF") == 0) { free (uptokstart); return SIZEOF; } break; case 5: - if (strcmp (uptokstart, "CLASS") == 0) + if (strcasecmp (uptokstart, "CLASS") == 0) { free (uptokstart); return CLASS; } - if (strcmp (uptokstart, "FALSE") == 0) + if (strcasecmp (uptokstart, "FALSE") == 0) { yylval.lval = 0; free (uptokstart); @@ -1478,13 +1472,13 @@ yylex (void) } break; case 4: - if (strcmp (uptokstart, "TRUE") == 0) + if (strcasecmp (uptokstart, "TRUE") == 0) { yylval.lval = 1; free (uptokstart); return TRUEKEYWORD; } - if (strcmp (uptokstart, "SELF") == 0) + if (strcasecmp (uptokstart, "SELF") == 0) { /* Here we search for 'this' like inserted in FPC stabs debug info. */ @@ -1542,44 +1536,6 @@ yylex (void) else sym = lookup_symbol (tmp, expression_context_block, VAR_DOMAIN, &is_a_field_of_this); - /* second chance uppercased (as Free Pascal does). */ - if (!sym && is_a_field_of_this.type == NULL && !is_a_field) - { - for (i = 0; i <= namelen; i++) - { - if ((tmp[i] >= 'a' && tmp[i] <= 'z')) - tmp[i] -= ('a'-'A'); - } - if (search_field && current_type) - is_a_field = (lookup_struct_elt_type (current_type, tmp, 1) != NULL); - if (is_a_field || parse_completion) - sym = NULL; - else - sym = lookup_symbol (tmp, expression_context_block, - VAR_DOMAIN, &is_a_field_of_this); - } - /* Third chance Capitalized (as GPC does). */ - if (!sym && is_a_field_of_this.type == NULL && !is_a_field) - { - for (i = 0; i <= namelen; i++) - { - if (i == 0) - { - if ((tmp[i] >= 'a' && tmp[i] <= 'z')) - tmp[i] -= ('a'-'A'); - } - else - if ((tmp[i] >= 'A' && tmp[i] <= 'Z')) - tmp[i] -= ('A'-'a'); - } - if (search_field && current_type) - is_a_field = (lookup_struct_elt_type (current_type, tmp, 1) != NULL); - if (is_a_field || parse_completion) - sym = NULL; - else - sym = lookup_symbol (tmp, expression_context_block, - VAR_DOMAIN, &is_a_field_of_this); - } if (is_a_field) { diff --git a/gdb/p-lang.c b/gdb/p-lang.c index 07006cd..a26c0c4 100644 --- a/gdb/p-lang.c +++ b/gdb/p-lang.c @@ -417,7 +417,7 @@ const struct language_defn pascal_language_defn = "pascal", /* Language name */ language_pascal, range_check_on, - case_sensitive_on, + case_sensitive_off, array_row_major, macro_expansion_no, &exp_descriptor_standard, ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: Regression for gdb.pascal/* [Re: [RFA 4/4] Constify parse_linesepc] 2013-10-17 18:18 ` Keith Seitz 2013-10-17 20:52 ` Tom Tromey @ 2013-10-18 19:34 ` Jan Kratochvil 2013-10-20 13:17 ` Pierre Muller 2013-10-29 16:39 ` Tom Tromey 1 sibling, 2 replies; 17+ messages in thread From: Jan Kratochvil @ 2013-10-18 19:34 UTC (permalink / raw) To: Keith Seitz; +Cc: gdb-patches@sourceware.org ml, Sergio Durigan Junior On Thu, 17 Oct 2013 20:18:48 +0200, Keith Seitz wrote: > There are two little sections of code, though, which violate > const-ness of the input, and I've removed those, since they don't > seem necessary. [This is the two loops that deal with changing the > case of `tokstart' -- which can easily be removed because we already > have a temporary buffer that is used for this.] > > I could not think of any reasons why pascal_lex would need to change > the input. The only thing that came to mind was completion, and the > behavior for that is, as far as I can tell, identical to how 7.0, > 7.3, 7.4, 7.5, and 7.6 behave. [both case-sensitive 'on' and 'off'] Maybe we could juse use [pascal patch] Use case_sensitive_off [Re: Regression for gdb.pascal/* [Re: [RFA 4/4] Constify parse_linesepc]] https://sourceware.org/ml/gdb-patches/2013-10/msg00581.html and forget about all the case changes then. > @@ -1369,11 +1368,8 @@ yylex (void) > break; > case '\\': > { > - const char *s, *o; > - > - o = s = ++tokptr; > - c = parse_escape (parse_gdbarch, &s); > - *tokptr += s - o; > + ++tokptr; > + c = parse_escape (parse_gdbarch, &tokptr); > if (c == -1) > { > continue; coding/patch style: This change is a bit unfortunate that together with your previous patch it just reformats the code, moreover not simplifying it. diff --git a/gdb/p-exp.y b/gdb/p-exp.y index da8d5f7..8cb98c0 100644 --- a/gdb/p-exp.y +++ b/gdb/p-exp.y @@ -1361,13 +1367,15 @@ yylex (void) /* Do nothing, loop will terminate. */ break; case '\\': - tokptr++; - c = parse_escape (parse_gdbarch, &tokptr); - if (c == -1) - { - continue; - } - tempbuf[tempbufindex++] = c; + { + ++tokptr; + c = parse_escape (parse_gdbarch, &tokptr); + if (c == -1) + { + continue; + } + tempbuf[tempbufindex++] = c; + } break; default: tempbuf[tempbufindex++] = *tokptr++; Pierre's reply would be great to check in the case changes removal with the case_sensitive_off patch. Otherwise it is not clear to me it is safe. Thanks, Jan ^ permalink raw reply [flat|nested] 17+ messages in thread
* RE: Regression for gdb.pascal/* [Re: [RFA 4/4] Constify parse_linesepc] 2013-10-18 19:34 ` Regression for gdb.pascal/* [Re: [RFA 4/4] Constify parse_linesepc] Jan Kratochvil @ 2013-10-20 13:17 ` Pierre Muller 2013-10-20 13:27 ` Jan Kratochvil 2013-10-29 16:39 ` Tom Tromey 1 sibling, 1 reply; 17+ messages in thread From: Pierre Muller @ 2013-10-20 13:17 UTC (permalink / raw) To: 'Jan Kratochvil', 'Keith Seitz' Cc: gdb-patches, 'Sergio Durigan Junior' > -----Message d'origine----- > De : gdb-patches-owner@sourceware.org [mailto:gdb-patches- > owner@sourceware.org] De la part de Jan Kratochvil > Envoyé : vendredi 18 octobre 2013 21:35 > À : Keith Seitz > Cc : gdb-patches@sourceware.org ml; Sergio Durigan Junior > Objet : Re: Regression for gdb.pascal/* [Re: [RFA 4/4] Constify > parse_linesepc] > > On Thu, 17 Oct 2013 20:18:48 +0200, Keith Seitz wrote: > > There are two little sections of code, though, which violate > > const-ness of the input, and I've removed those, since they don't > > seem necessary. [This is the two loops that deal with changing the > > case of `tokstart' -- which can easily be removed because we already > > have a temporary buffer that is used for this.] > > > > I could not think of any reasons why pascal_lex would need to change > > the input. The only thing that came to mind was completion, and the > > behavior for that is, as far as I can tell, identical to how 7.0, > > 7.3, 7.4, 7.5, and 7.6 behave. [both case-sensitive 'on' and 'off'] > > Maybe we could juse use > [pascal patch] Use case_sensitive_off [Re: Regression for > gdb.pascal/* [Re: [RFA 4/4] Constify parse_linesepc]] > https://sourceware.org/ml/gdb-patches/2013-10/msg00581.html > > and forget about all the case changes then. > > > > @@ -1369,11 +1368,8 @@ yylex (void) > > break; > > case '\\': > > { > > - const char *s, *o; > > - > > - o = s = ++tokptr; > > - c = parse_escape (parse_gdbarch, &s); > > - *tokptr += s - o; > > + ++tokptr; > > + c = parse_escape (parse_gdbarch, &tokptr); > > if (c == -1) > > { > > continue; > > coding/patch style: > This change is a bit unfortunate that together with your previous patch it > just reformats the code, moreover not simplifying it. > > > diff --git a/gdb/p-exp.y b/gdb/p-exp.y > index da8d5f7..8cb98c0 100644 > --- a/gdb/p-exp.y > +++ b/gdb/p-exp.y > @@ -1361,13 +1367,15 @@ yylex (void) > /* Do nothing, loop will terminate. */ > break; > case '\\': > - tokptr++; > - c = parse_escape (parse_gdbarch, &tokptr); > - if (c == -1) > - { > - continue; > - } > - tempbuf[tempbufindex++] = c; > + { > + ++tokptr; > + c = parse_escape (parse_gdbarch, &tokptr); > + if (c == -1) > + { > + continue; > + } > + tempbuf[tempbufindex++] = c; > + } > break; > default: > tempbuf[tempbufindex++] = *tokptr++; > > > Pierre's reply would be great to check in the case changes removal with the > case_sensitive_off patch. Otherwise it is not clear to me it is safe. Just a quick reply to tell you that I did read this thread, and that I am quite lost in the parser/completion coupling ... I did see indeed problems related to access of alloca'ted memory outside the function in current trunk. But I don't understand anymore how completion works in context of language expression parser. The completion doesn't seem to work at all anymore for pascal language, but it does work correctly for C language, and I did not yet understand why it behaves differently... I also found some internal error generated by the pascal expression parser triggered if you try to print out a non-existing field of a record (struct in C language) or object/class... I at least managed to get completion to work partially again with the patch below... I hope I will be able to really understand the whole problem... Pierre Index: p-exp.y =================================================================== RCS file: /cvs/src/src/gdb/p-exp.y,v retrieving revision 1.69 diff -u -p -r1.69 p-exp.y --- p-exp.y 2 Oct 2013 00:46:06 -0000 1.69 +++ p-exp.y 20 Oct 2013 11:34:34 -0000 @@ -125,7 +125,7 @@ static int yylex (void); void yyerror (char *); -static char * uptok (char *, int); +static char *uptok (const char *, int); %} /* Although the yacc "value" of an expression is not used, @@ -316,8 +316,7 @@ exp : field_exp FIELDNAME exp : field_exp name - { mark_struct_expression (); - write_exp_elt_opcode (STRUCTOP_STRUCT); + { write_exp_elt_opcode (STRUCTOP_STRUCT); write_exp_string ($2); write_exp_elt_opcode (STRUCTOP_STRUCT); search_field = 0; @@ -332,7 +331,12 @@ exp : field_exp name } } ; - +exp : field_exp name COMPLETE + { mark_struct_expression (); + write_exp_elt_opcode (STRUCTOP_STRUCT); + write_exp_string ($2); + write_exp_elt_opcode (STRUCTOP_STRUCT); } + ; exp : field_exp COMPLETE { struct stoken s; mark_struct_expression (); @@ -1105,7 +1109,7 @@ static const struct token tokentab2[] = /* Allocate uppercased var: */ /* make an uppercased copy of tokstart. */ static char * -uptok (char *tokstart, int namelen) +uptok (const char *tokstart, int namelen) { int i; char *uptokstart = (char *)malloc(namelen+1); @@ -1120,11 +1124,6 @@ uptok (char *tokstart, int namelen) return uptokstart; } -/* This is set if the previously-returned token was a structure - operator '.'. This is used only when parsing to - do field name completion. */ -static int last_was_structop; - /* Read one token, getting characters through lexptr. */ static int @@ -1133,22 +1132,19 @@ yylex (void) int c; int namelen; unsigned int i; - char *tokstart; + const char *tokstart; char *uptokstart; - char *tokptr; + const char *tokptr; int explen, tempbufindex; static char *tempbuf; static int tempbufsize; - int saw_structop = last_was_structop; - last_was_structop = 0; retry: prev_lexptr = lexptr; + tokstart = lexptr; explen = strlen (lexptr); - tokstart = alloca (explen + 1); - memcpy (tokstart, lexptr, explen + 1); /* See if it is a special token of length 3. */ if (explen > 2) @@ -1179,7 +1175,7 @@ yylex (void) switch (c = *tokstart) { case 0: - if (saw_structop && search_field) + if (search_field && parse_completion) return COMPLETE; else return 0; @@ -1244,8 +1240,6 @@ yylex (void) /* Might be a floating point number. */ if (lexptr[1] < '0' || lexptr[1] > '9') { - if (parse_completion) - last_was_structop = 1; goto symbol; /* Nope, must be a symbol. */ } @@ -1264,7 +1258,7 @@ yylex (void) { /* It's a number. */ int got_dot = 0, got_e = 0, toktype; - char *p = tokstart; + const char *p = tokstart; int hex = input_radix > 10; if (c == '0' && (p[1] == 'x' || p[1] == 'X')) @@ -1369,11 +1363,8 @@ yylex (void) break; case '\\': { - const char *s, *o; - - o = s = ++tokptr; - c = parse_escape (parse_gdbarch, &s); - *tokptr += s - o; + ++tokptr; + c = parse_escape (parse_gdbarch, &tokptr); if (c == -1) { continue; @@ -1511,17 +1502,17 @@ yylex (void) if (*tokstart == '$') { - char c; + char *tmp; + /* $ is the normal prefix for pascal hexadecimal values but this conflicts with the GDB use for debugger variables so in expression to enter hexadecimal values we still need to use C syntax with 0xff */ write_dollar_variable (yylval.sval); - c = tokstart[namelen]; - tokstart[namelen] = 0; - intvar = lookup_only_internalvar (++tokstart); - --tokstart; - tokstart[namelen] = c; + tmp = alloca (namelen + 1); + memcpy (tmp, tokstart, namelen); + tmp[namelen] = '\0'; + intvar = lookup_only_internalvar (tmp + 1); free (uptokstart); return VARIABLE; } @@ -1541,7 +1532,7 @@ yylex (void) if (search_field && current_type) is_a_field = (lookup_struct_elt_type (current_type, tmp, 1) != NULL); - if (is_a_field || parse_completion) + if (is_a_field/* || parse_completion*/) sym = NULL; else sym = lookup_symbol (tmp, expression_context_block, @@ -1556,17 +1547,11 @@ yylex (void) } if (search_field && current_type) is_a_field = (lookup_struct_elt_type (current_type, tmp, 1) != NULL); - if (is_a_field || parse_completion) + if (is_a_field /*|| parse_completion*/) sym = NULL; else sym = lookup_symbol (tmp, expression_context_block, VAR_DOMAIN, &is_a_field_of_this); - if (sym || is_a_field_of_this.type != NULL || is_a_field) - for (i = 0; i <= namelen; i++) - { - if ((tokstart[i] >= 'a' && tokstart[i] <= 'z')) - tokstart[i] -= ('a'-'A'); - } } /* Third chance Capitalized (as GPC does). */ if (!sym && is_a_field_of_this.type == NULL && !is_a_field) @@ -1584,29 +1569,18 @@ yylex (void) } if (search_field && current_type) is_a_field = (lookup_struct_elt_type (current_type, tmp, 1) != NULL); - if (is_a_field || parse_completion) + if (is_a_field /*|| parse_completion*/) sym = NULL; else sym = lookup_symbol (tmp, expression_context_block, VAR_DOMAIN, &is_a_field_of_this); - if (sym || is_a_field_of_this.type != NULL || is_a_field) - for (i = 0; i <= namelen; i++) - { - if (i == 0) - { - if ((tokstart[i] >= 'a' && tokstart[i] <= 'z')) - tokstart[i] -= ('a'-'A'); - } - else - if ((tokstart[i] >= 'A' && tokstart[i] <= 'Z')) - tokstart[i] -= ('A'-'a'); - } } if (is_a_field) { tempbuf = (char *) realloc (tempbuf, namelen + 1); - strncpy (tempbuf, tokstart, namelen); tempbuf [namelen] = 0; + strncpy (tempbuf, tmp, namelen); + tempbuf [namelen] = 0; yylval.sval.ptr = tempbuf; yylval.sval.length = namelen; free (uptokstart); ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: Regression for gdb.pascal/* [Re: [RFA 4/4] Constify parse_linesepc] 2013-10-20 13:17 ` Pierre Muller @ 2013-10-20 13:27 ` Jan Kratochvil 0 siblings, 0 replies; 17+ messages in thread From: Jan Kratochvil @ 2013-10-20 13:27 UTC (permalink / raw) To: Pierre Muller Cc: 'Keith Seitz', gdb-patches, 'Sergio Durigan Junior' On Sun, 20 Oct 2013 15:17:04 +0200, Pierre Muller wrote: > The completion doesn't seem to work at all anymore for pascal language, Current FSF GDB HEAD is broken, it should be somehow fixed by Keith's fix up: https://sourceware.org/ml/gdb-patches/2013-10/msg00530.html From your patch it seems to me you were trying to fix FSF GDB HEAD reimplementing the patch above. Just to make clear we talk about the same GDB codebase. Thanks, Jan ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: Regression for gdb.pascal/* [Re: [RFA 4/4] Constify parse_linesepc] 2013-10-18 19:34 ` Regression for gdb.pascal/* [Re: [RFA 4/4] Constify parse_linesepc] Jan Kratochvil 2013-10-20 13:17 ` Pierre Muller @ 2013-10-29 16:39 ` Tom Tromey 2013-10-31 16:14 ` Pierre Muller 1 sibling, 1 reply; 17+ messages in thread From: Tom Tromey @ 2013-10-29 16:39 UTC (permalink / raw) To: Jan Kratochvil Cc: Pierre Muller, Keith Seitz, gdb-patches@sourceware.org ml, Sergio Durigan Junior >>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes: Jan> Maybe we could juse use Jan> [pascal patch] Use case_sensitive_off [Re: Regression for Jan> gdb.pascal/* [Re: [RFA 4/4] Constify parse_linesepc]] Jan> https://sourceware.org/ml/gdb-patches/2013-10/msg00581.html Jan> and forget about all the case changes then. I think this would be the best approach. However, I think it would be good for Pierre to weigh in. Pierre, you replied on this thread but I don't think you addressed Jan's patch, which seems like the best way forward. If Pierre can't respond soon-ish, I think we should go ahead in the interests of forward motion and regression-fixing; and deal with whatever fallout arises when he returns. Tom ^ permalink raw reply [flat|nested] 17+ messages in thread
* RE: Regression for gdb.pascal/* [Re: [RFA 4/4] Constify parse_linesepc] 2013-10-29 16:39 ` Tom Tromey @ 2013-10-31 16:14 ` Pierre Muller 2013-11-13 20:43 ` Keith Seitz 0 siblings, 1 reply; 17+ messages in thread From: Pierre Muller @ 2013-10-31 16:14 UTC (permalink / raw) To: 'Tom Tromey', 'Jan Kratochvil' Cc: 'Pierre Muller', 'Keith Seitz', gdb-patches, 'Sergio Durigan Junior' > -----Message d'origine----- > De : gdb-patches-owner@sourceware.org [mailto:gdb-patches- > owner@sourceware.org] De la part de Tom Tromey > Envoyé : mardi 29 octobre 2013 17:39 > À : Jan Kratochvil > Cc : Pierre Muller; Keith Seitz; gdb-patches@sourceware.org ml; Sergio > Durigan Junior > Objet : Re: Regression for gdb.pascal/* [Re: [RFA 4/4] Constify > parse_linesepc] > > >>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes: > > Jan> Maybe we could juse use > Jan> [pascal patch] Use case_sensitive_off [Re: Regression for > Jan> gdb.pascal/* [Re: [RFA 4/4] Constify parse_linesepc]] > Jan> https://sourceware.org/ml/gdb-patches/2013-10/msg00581.html > Jan> and forget about all the case changes then. > > I think this would be the best approach. > > However, I think it would be good for Pierre to weigh in. > Pierre, you replied on this thread but I don't think you addressed > Jan's > patch, which seems like the best way forward. I am sorry, but I have not enough time lately to look more deeply into this problem... One thing I do remember was about the fact that some of the internal procedures/functions used by Free Pascal compiler were written with lowercased assembly labels to avoid conflicts with usual procedures or functions Is Keith patch already in latest git? OK, it seems Keith last patch is not in, while it does indeed fix otherwise illegitimate stack access (alloca memory read after exiting function using it...) So Keith patch should go in. Concerning Jan's patch, I am more hesitant because the fact to change the default behavior of pascal parser to case-insensitive does have noticeable behavior changes for end-users... I did not see any difference in the testsuite between Keith and Jan's patch, as Keith patch alone already get's us back to only one failure in the test "p 0x1.1" I must admit that Free pascal never accepted to parse hexadecimal reals, is this accepted in any other pascal compiler ? According to 'git blame' this test was added by Jan on 2010-09-06... Second related question is: there is an xfail in the test, but I stil get FAIL here... Shouldn't I get XFAIL? > If Pierre can't respond soon-ish, I think we should go ahead in the > interests of forward motion and regression-fixing; and deal with > whatever fallout arises when he returns. As I said, let's commit Keith part and wait a little bit on the case insensitive change... I would also like to fix the completion bug in between to check if Jan's patch changes anything in that part.. Pierre as pascal language maintainer ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: Regression for gdb.pascal/* [Re: [RFA 4/4] Constify parse_linesepc] 2013-10-31 16:14 ` Pierre Muller @ 2013-11-13 20:43 ` Keith Seitz 0 siblings, 0 replies; 17+ messages in thread From: Keith Seitz @ 2013-11-13 20:43 UTC (permalink / raw) To: Pierre Muller, 'Tom Tromey', 'Jan Kratochvil' Cc: 'Pierre Muller', gdb-patches, 'Sergio Durigan Junior' On 10/31/2013 09:14 AM, Pierre Muller wrote: > OK, it seems Keith last patch is not in, > while it does indeed fix otherwise illegitimate stack access > (alloca memory read after exiting function using it...) > So Keith patch should go in. I have (finally) committed this. Thank you (all) again for reviewing it. Keith ^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2013-11-13 19:49 UTC | newest] Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2013-09-30 18:57 [RFA 4/4] Constify parse_linesepc Keith Seitz 2013-10-01 4:15 ` Sergio Durigan Junior 2013-10-01 20:16 ` Tom Tromey 2013-10-02 4:38 ` Keith Seitz 2013-10-16 9:57 ` Regression for gdb.pascal/* [Re: [RFA 4/4] Constify parse_linesepc] Jan Kratochvil 2013-10-16 22:07 ` Sergio Durigan Junior 2013-10-16 23:40 ` Sergio Durigan Junior 2013-10-17 18:18 ` Keith Seitz 2013-10-17 20:52 ` Tom Tromey 2013-10-18 17:20 ` Jan Kratochvil 2013-10-18 19:09 ` [pascal patch] Use case_sensitive_off [Re: Regression for gdb.pascal/* [Re: [RFA 4/4] Constify parse_linesepc]] Jan Kratochvil 2013-10-18 19:34 ` Regression for gdb.pascal/* [Re: [RFA 4/4] Constify parse_linesepc] Jan Kratochvil 2013-10-20 13:17 ` Pierre Muller 2013-10-20 13:27 ` Jan Kratochvil 2013-10-29 16:39 ` Tom Tromey 2013-10-31 16:14 ` Pierre Muller 2013-11-13 20:43 ` Keith Seitz
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).