This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH v3 2/9] Explicit locations: introduce new struct event_location-based API
- From: Keith Seitz <keiths at redhat dot com>
- To: Doug Evans <xdje42 at gmail dot com>
- Cc: gdb-patches at sourceware dot org
- Date: Thu, 07 May 2015 09:59:26 -0700
- Subject: Re: [PATCH v3 2/9] Explicit locations: introduce new struct event_location-based API
- Authentication-results: sourceware.org; auth=none
- References: <20150217220619 dot 1312 dot 39861 dot stgit at valrhona dot uglyboxes dot com> <20150217220641 dot 1312 dot 59694 dot stgit at valrhona dot uglyboxes dot com> <m3bnkclblf dot fsf at sspiff dot org>
On 03/01/2015 12:14 PM, Doug Evans wrote:
> Keith Seitz <keiths@redhat.com> writes:
>>
>> +/* See description in linespec.h. */
>> +
>> +void
>> +linespec_lex_to_end (char **stringp)
>> +{
>> + linespec_parser parser;
>> + struct cleanup *cleanup;
>> + linespec_token token;
>> + volatile struct gdb_exception e;
>> + const char *p, *orig;
>> +
>> + if (stringp == NULL || *stringp == NULL)
>> + return;
>> +
>> + linespec_parser_new (&parser, 0, current_language, NULL, 0, NULL);
>> + cleanup = make_cleanup (linespec_parser_delete, &parser);
>> + parser.lexer.saved_arg = *stringp;
>> + parser.keyword_ok = 1;
>
> ====
> parser.keyword_ok = 0; ?
Nope. Not in this case. We need to stop lexing when we find a keyword.
Otherwise, default locations will break. From signest.exp:
(gdb) break if 0
Function "if 0" not defined.
Make breakpoint pending on future shared library load? (y or [n])
>> + orig = p = *stringp;
>> + parser.lexer.stream = &p;
>> +
>> + TRY_CATCH (e, RETURN_MASK_ERROR)
>> + {
>> + do
>> + {
>> + /* Stop before any comma tokens; we need it to keep it
>> + as the next token in the string. */
>> + token = linespec_lexer_peek_token (&parser);
>> + if (token.type == LSTOKEN_COMMA)
>> + break;
>> +
>> + /* For addresses advance the parser stream past
>> + any parsed input and stop lexing. */
>> + if (token.type == LSTOKEN_STRING
>> + && *LS_TOKEN_STOKEN (token).ptr == '*')
>
> ====
> Will this mis-parse an address appearing after the first token?
> [IOW, *address must be the first token, right?]
That's right. I have made no attempt to change the current behavior. But
it doesn't really matter in this context. The chunk dealing with address
locations is removed from this function in the
explicit-address-locations patch. [It causes no regressions in the test
suite, so buildbot should not report any, either.]
>> + {
>> + const char *arg, *orig;
>> +
>> + orig = arg = *stringp;
>> + (void) linespec_expression_to_pc (&arg);
>> + PARSER_STREAM (&parser) += arg - orig;
>
> ====
> How about: PARSER_STREAM (&parser) = arg; ?
Sure. [NOTE: This disappears in subsequent patch.]
>> + break;
>> + }
>> +
>> + token = linespec_lexer_consume_token (&parser);
>> +
>> + /* Keywords are okay after the first token. */
>> + parser.keyword_ok = 1;
>> + }
>> + while (token.type != LSTOKEN_EOI && token.type != LSTOKEN_KEYWORD);
>> + }
>> +
>> + *stringp += PARSER_STREAM (&parser) - orig;
>
> ====
> Similary, how about: *stringp = PARSER_STREAM (&parser); ?
>
Unfortunately, we cannot do this. PARSER_STREAM is const, stringp is
not. Well, we probably could do this, but it would require
constification of a lot of other functions/files. I'd be happy to
investigate that as a follow-up patch.
I'll have a new series posted here in a bit.
Keith