* [expr] [patch] Fix "break expr if (cond)" regression
@ 2010-01-21 16:41 Jan Kratochvil
2010-01-21 17:36 ` Keith Seitz
2010-01-21 21:05 ` Tom Tromey
0 siblings, 2 replies; 3+ messages in thread
From: Jan Kratochvil @ 2010-01-21 16:41 UTC (permalink / raw)
To: Keith Seitz; +Cc: archer
commit 21e418c04290aa5d2e75543d31fe3fe5d70d6d41
Checked-in on archer-jankratochvil-fedora13 but this regression is present on
just archer-keiths-expr-cumulative.
The regression has been introduced by:
commit 97a9a53e60460dd246de6a1d5ce50bf3497a1eb8
Author: keiths <keiths@redhat.com>
Date: Thu Jan 14 18:51:45 2010 -0800
* linespec.c (decode_line_1): Keep overload information for the
non-compound case.
(gdb) break marker1 if (1==1)
-Breakpoint 4 at 0x80485eb: file gdb/testsuite/gdb.base/break1.c, line 45.
-PASS: gdb.base/condbreak.exp: break marker1 if (1==1)
+Function "marker1 if (1==1)" not defined.
+Make breakpoint pending on future shared library load? (y or [n]) n
+FAIL: gdb.base/condbreak.exp: break marker1 if (1==1) (got interactive prompt)
I do not fully agree with my patch but I also do not fully agree with the
former patch, maybe bison should be used to parse the expression, but rather
someone (Tom? unaware) said that "break E [thread T] [if C]" should be rather
changed to the more parseable+standardized "break [-thread T] [-if C] E" so
that these expression parsing difficulties would be present only as a backward
compatibility and do not need to supersede the former functionality.
-FAIL: gdb.base/chng-syms.exp: setting conditional breakpoint on function (got interactive prompt)
-FAIL: gdb.base/chng-syms.exp: running to stop_here first time
-FAIL: gdb.base/chng-syms.exp: continue until exit at breakpoint first time through (the program is no longer running)
+PASS: gdb.base/chng-syms.exp: setting conditional breakpoint on function
+PASS: gdb.base/chng-syms.exp: continue until exit at breakpoint first time through
-FAIL: gdb.base/condbreak.exp: break marker1 if (1==1) (got interactive prompt)
+PASS: gdb.base/condbreak.exp: break marker1 if (1==1)
-FAIL: gdb.base/condbreak.exp: break marker2 if (a==43) (got interactive prompt)
-FAIL: gdb.base/condbreak.exp: break marker3 if (multi_line_if_conditional(1,1,1)==0) (got interactive prompt)
+PASS: gdb.base/condbreak.exp: break marker2 if (a==43)
+PASS: gdb.base/condbreak.exp: break marker3 if (multi_line_if_conditional(1,1,1)==0)
-FAIL: gdb.base/condbreak.exp: breakpoint info
+PASS: gdb.base/condbreak.exp: breakpoint info
-FAIL: gdb.base/condbreak.exp: run until breakpoint at marker1
-FAIL: gdb.base/condbreak.exp: run until breakpoint at marker2
-FAIL: gdb.base/condbreak.exp: break main if (1==1) thread 999 (got interactive prompt)
-FAIL: gdb.base/condbreak.exp: break main thread 999 if (1==1) (got interactive prompt)
+PASS: gdb.base/condbreak.exp: run until breakpoint at marker1
+PASS: gdb.base/condbreak.exp: run until breakpoint at marker2
+PASS: gdb.base/condbreak.exp: break main if (1==1) thread 999
+PASS: gdb.base/condbreak.exp: break main thread 999 if (1==1)
-FAIL: gdb.base/condbreak.exp: break *main if (1==1) task 999
-FAIL: gdb.base/condbreak.exp: break *main task 999 if (1==1)
+PASS: gdb.base/condbreak.exp: break *main if (1==1) task 999
+PASS: gdb.base/condbreak.exp: break *main task 999 if (1==1)
-FAIL: gdb.base/condbreak.exp: break *main if (1==1) ta 999
-FAIL: gdb.base/condbreak.exp: run until breakpoint at marker3 (the program is no longer running)
-FAIL: gdb.base/condbreak.exp: run until breakpoint at marker4 (the program is no longer running)
+PASS: gdb.base/condbreak.exp: break *main if (1==1) ta 999
+PASS: gdb.base/condbreak.exp: run until breakpoint at marker3
+PASS: gdb.base/condbreak.exp: run until breakpoint at marker4
* linespec.c: Include <ctype.h>.
(decode_line_1 <q != NULL>): Ignore '(' used after " if".
---
gdb/linespec.c | 9 ++++++++-
1 files changed, 8 insertions(+), 1 deletions(-)
diff --git a/gdb/linespec.c b/gdb/linespec.c
index a9b4f1e..4e54a3a 100644
--- a/gdb/linespec.c
+++ b/gdb/linespec.c
@@ -40,6 +40,7 @@
#include "interps.h"
#include "mi/mi-cmds.h"
#include "target.h"
+#include <ctype.h>
/* We share this one with symtab.c, but it is not exported widely. */
@@ -822,7 +823,13 @@ decode_line_1 (char **argptr, int funfirstline, struct symtab *default_symtab,
/* Keep method overload information. */
q = strchr (p, '(');
if (q != NULL)
- p = strrchr (q, ')') + 1;
+ {
+ /* Ignore '(' used after " if". */
+ while (q > p && isspace (q[-1]))
+ q--;
+ if (!(q >= p + 3 && strncmp (&q[-2], "if", 2) == 0 && isspace (q[-3])))
+ p = strrchr (q, ')') + 1;
+ }
/* Make sure we keep important kewords like "const" */
if (strncmp (p, " const", 6) == 0)
--
1.6.6
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [expr] [patch] Fix "break expr if (cond)" regression
2010-01-21 16:41 [expr] [patch] Fix "break expr if (cond)" regression Jan Kratochvil
@ 2010-01-21 17:36 ` Keith Seitz
2010-01-21 21:05 ` Tom Tromey
1 sibling, 0 replies; 3+ messages in thread
From: Keith Seitz @ 2010-01-21 17:36 UTC (permalink / raw)
To: Jan Kratochvil; +Cc: archer
On 01/21/2010 08:40 AM, Jan Kratochvil wrote:
> commit 97a9a53e60460dd246de6a1d5ce50bf3497a1eb8
> Author: keiths<keiths@redhat.com>
> Date: Thu Jan 14 18:51:45 2010 -0800
>
> * linespec.c (decode_line_1): Keep overload information for the
> non-compound case.
Grr... Linespecs again. Once you get into them, they never let you go.
> (gdb) break marker1 if (1==1)
> -Breakpoint 4 at 0x80485eb: file gdb/testsuite/gdb.base/break1.c, line 45.
> -PASS: gdb.base/condbreak.exp: break marker1 if (1==1)
> +Function "marker1 if (1==1)" not defined.
> +Make breakpoint pending on future shared library load? (y or [n]) n
> +FAIL: gdb.base/condbreak.exp: break marker1 if (1==1) (got interactive prompt)
Ugh! My bad. On my last commit, I checked in something dubious: the call
to strrchr -- I originally wrote something a little more elaborate to do
that job. Now I remember why I originally did that. I reverted that bit
of the change locally, and that fixes all the regressions.
Here's the patch that I pulled from the history:
diff --git a/gdb/linespec.c b/gdb/linespec.c
index d6f0ed8..fdd7b5a 100644
--- a/gdb/linespec.c
+++ b/gdb/linespec.c
@@ -820,9 +820,25 @@ decode_line_1 (char **argptr, int funfirstline,
struct symtab *default_symtab,
p = find_template_name_end (p);
/* Keep method overload information. */
- q = strchr (p, '(');
- if (q != NULL)
- p = strrchr (q, ')') + 1;
+ if (*p == '(')
+ {
+ int depth = 0;
+
+ while (*p)
+ {
+ if (*p == '(')
+ ++depth;
+ else if (*p == ')')
+ {
+ if (--depth == 0)
+ {
+ ++p;
+ break;
+ }
+ }
+ ++p;
+ }
+ }
/* Make sure we keep important kewords like "const" */
if (strncmp (p, " const", 6) == 0)
> I do not fully agree with my patch but I also do not fully agree with the
> former patch, maybe bison should be used to parse the expression, but rather
> someone (Tom? unaware) said that "break E [thread T] [if C]" should be rather
> changed to the more parseable+standardized "break [-thread T] [-if C] E" so
> that these expression parsing difficulties would be present only as a backward
> compatibility and do not need to supersede the former functionality.
Yeah, it would be wonderful, in general, to get rid of linespecs
entirely. They suck big time.
Sorry about that.
Keith
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [expr] [patch] Fix "break expr if (cond)" regression
2010-01-21 16:41 [expr] [patch] Fix "break expr if (cond)" regression Jan Kratochvil
2010-01-21 17:36 ` Keith Seitz
@ 2010-01-21 21:05 ` Tom Tromey
1 sibling, 0 replies; 3+ messages in thread
From: Tom Tromey @ 2010-01-21 21:05 UTC (permalink / raw)
To: Jan Kratochvil; +Cc: Keith Seitz, archer
>>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:
Jan> I do not fully agree with my patch but I also do not fully agree
Jan> with the former patch, maybe bison should be used to parse the
Jan> expression, but rather someone (Tom? unaware) said that "break E
Jan> [thread T] [if C]" should be rather changed to the more
Jan> parseable+standardized "break [-thread T] [-if C] E" so that these
Jan> expression parsing difficulties would be present only as a backward
Jan> compatibility and do not need to supersede the former
Jan> functionality.
In the end gdb went the other route, of making "thread" special in the
lexer -- but with a twist:
/* For the same reason (breakpoint conditions), "thread N"
terminates the expression. "thread" could be an identifier, but
an identifier is never followed by a number without intervening
punctuation. "task" is similar. Handle abbreviations of these,
similarly to breakpoint.c:find_condition_and_thread. */
[...]
if (*p >= '0' && *p <= '9')
return 0;
I think linespecs would be more maintainable if we came up with a
grammar for them and implemented them using bison (or any parser
generator), but I don't know if that is even possible while preserving
compatibility.
Tom
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2010-01-21 21:05 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-01-21 16:41 [expr] [patch] Fix "break expr if (cond)" regression Jan Kratochvil
2010-01-21 17:36 ` Keith Seitz
2010-01-21 21:05 ` Tom Tromey
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).