[RFA 4/4] Constify parse_linesepc

[RFA 4/4] Constify parse_linesepc

[Keith Seitz]

[-- 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, &copy);
+  *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, &copy);
+  *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);

Re: [RFA 4/4] Constify parse_linesepc

[Sergio Durigan Junior]

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, &copy);
> +  *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, &copy);
> +  *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

Re: [RFA 4/4] Constify parse_linesepc

[Tom Tromey]

>>>>> "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

Re: [RFA 4/4] Constify parse_linesepc

[Keith Seitz]

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

Regression for gdb.pascal/* [Re: [RFA 4/4] Constify parse_linesepc]

[Jan Kratochvil]

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

Re: Regression for gdb.pascal/* [Re: [RFA 4/4] Constify parse_linesepc]

[Sergio Durigan Junior]

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;
       }
 

Re: Regression for gdb.pascal/* [Re: [RFA 4/4] Constify parse_linesepc]

[Sergio Durigan Junior]

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

Re: Regression for gdb.pascal/* [Re: [RFA 4/4] Constify parse_linesepc]

[Keith Seitz]

[-- 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);

Re: Regression for gdb.pascal/* [Re: [RFA 4/4] Constify parse_linesepc]

[Tom Tromey]

>>>>> "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

Re: Regression for gdb.pascal/* [Re: [RFA 4/4] Constify parse_linesepc]

[Jan Kratochvil]

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

[pascal patch] Use case_sensitive_off [Re: Regression for gdb.pascal/* [Re: [RFA 4/4] Constify parse_linesepc]]

[Jan Kratochvil]

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,

Re: Regression for gdb.pascal/* [Re: [RFA 4/4] Constify parse_linesepc]

[Jan Kratochvil]

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

RE: Regression for gdb.pascal/* [Re: [RFA 4/4] Constify parse_linesepc]

[Pierre Muller]

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

Re: Regression for gdb.pascal/* [Re: [RFA 4/4] Constify parse_linesepc]

[Jan Kratochvil]

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

Re: Regression for gdb.pascal/* [Re: [RFA 4/4] Constify parse_linesepc]

[Tom Tromey]

>>>>> "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

RE: Regression for gdb.pascal/* [Re: [RFA 4/4] Constify parse_linesepc]

[Pierre Muller]

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

Re: Regression for gdb.pascal/* [Re: [RFA 4/4] Constify parse_linesepc]

[Keith Seitz]

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