This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
RE: Regression for gdb.pascal/* [Re: [RFA 4/4] Constify parse_linesepc]
- From: "Pierre Muller" <pierre dot muller at ics-cnrs dot unistra dot fr>
- To: "'Jan Kratochvil'" <jan dot kratochvil at redhat dot com>, "'Keith Seitz'" <keiths at redhat dot com>
- Cc: <gdb-patches at sourceware dot org>, "'Sergio Durigan Junior'" <sergiodj at redhat dot com>
- Date: Sun, 20 Oct 2013 15:17:04 +0200
- Subject: RE: Regression for gdb.pascal/* [Re: [RFA 4/4] Constify parse_linesepc]
- Authentication-results: sourceware.org; auth=none
- References: <5249C987 dot 50809 at redhat dot com> <87d2no4uim dot fsf at fleche dot redhat dot com> <524BA344 dot 2070802 at redhat dot com> <20131016095743 dot GA17072 at host2 dot jankratochvil dot net> <m3zjq8hnub dot fsf at redhat dot com> <m3vc0wg4yg dot fsf at redhat dot com> <52602A08 dot 4020705 at redhat dot com> <20131018193445 dot GA12496 at host2 dot jankratochvil dot net>
> -----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);