[patch 0/2] physname reg.: C++ breakpoints / linespec fixes

[patch 0/2] physname reg.: C++ breakpoints / linespec fixes

[Jan Kratochvil]

Hi,

as discussed in the "The future of dwarf2_physname" thread GDB lost the
ability to fallback to minimal symbols (ELF .symtab & co.) from full symbols
(DWARF .debug_info etc.) in some cases, due to physname:
-         if (paren_pointer == NULL)
-           return decode_compound (argptr, funfirstline, canonical,
change into unconditional - approx.:
+           return decode_compound (argptr, funfirstline, canonical,

[patch 1/2]
One reason if there are some bugs and/or discrepancies between GCC, DWARF and
GDB regarding DWARF which is sometimes even missing DW_AT_linkage_name.
The minimal symbols are at least one way how to `break' at some function.

Another point is that sometimes the DWARF is missing, either for parts of the
app compiled without -g or for system libraries which do not have separate
debug info installed.

[patch 2/2]
Another problem is "Junk at end of arguments." for more complicated function
specifiers.  It is due to the physname patch part:
-  else if (paren_pointer != NULL)
-    {
-      p = paren_pointer + 1;
-    }
The idea here is instead of chasing which every string can still be a part of
the function specification (as being done by current physname code) one can
rather to stop on any string which looks as a linespec stopper.  It would be
a problem if it could be ambiguous but I haven't found any countercase, would
you?  That is:
 * it is worse to parse "operator()" and leave "(int)" there
   causing "Junk at end of arguments." 
instead of the failure case of the proposed code:
 * possibly consider whole "funcname threaf 1" (`threaf' is a `thread' typo)
   as a function name failing to look up symbol "funcname threaf 1".  It just
   leads to a different error type but it would end up by an error anyway.

I have not addressed specific PRs, I was addressing to have no regressions
against pre-physname GDB, that is to the FSF gdb-7.1 state, before:
	commit 42284fdf9d8cdb20c8e833bdbdb2b56977fea525
	Author: Keith Seitz <keiths@redhat.com>
	Date:   Tue Mar 9 18:09:04 2010 +0000
	    dwarf2_physname patchset:

as tested by my gdb-breakpoint*.pl scripts posted at:
	Re: [RFA] 12266 (typedef'd parameters) revisited again
	http://sourceware.org/ml/gdb-patches/2011-06/msg00067.html

that is to be able to at least:
	(gdb) break 'function name as seen in nm -C'

So far I have not tried to test regressions without quotes but (a) hopefully
it also works and (b) adding quotes should be users be used to as
a workaround, it is worse if one cannot figure out how to putsthe breakpoint.

I understand all the various bugs in decode_compound code (such as the one about `operator ()') and in physname
mangling etc. can be fixed but these two
patches try to address it in a generic way.  linespec is IMNSHO a hack anyway
and one day it should be merged with the general expression parser (used for
example for `print'); which should be further merged with GCC/G++ parser.
	Expression Parser Plug-In Available
	http://sourceware.org/ml/archer/2011-q1/msg00122.html

So the goal here is to get no regressions with gdb-breakpoint*.pl scripts,
I did not try to summarize all the regression types to create a GDB testcase
for each of them.  The new testcases are only illustrative for the specific
fixes FAIL->PASS, I was not trying to address each specific problems by this
patchset anyway.


Thanks,
Jan

Re: [patch 0/2] physname reg.: C++ breakpoints / linespec fixes

[Jan Kratochvil]

On Sun, 05 Jun 2011 22:26:15 +0200, Jan Kratochvil wrote:
> linespec is IMNSHO a hack anyway and one day it should be merged with the
> general expression parser (used for example for `print');

This part I tried in:
	http://sourceware.org/gdb/wiki/ArcherBranchManagement
	archer-jankratochvil-linespec

but it may rather just give a picture than to base anything on top of it in
the future.


Regards,
Jan

Re: [patch 0/2] physname reg.: C++ breakpoints / linespec fixes

[Tom Tromey]

>>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:

Jan> linespec is IMNSHO a hack anyway and one day it should be merged
Jan> with the general expression parser (used for example for `print');
Jan> which should be further merged with GCC/G++ parser.  Expression
Jan> Parser Plug-In Available
Jan> http://sourceware.org/ml/archer/2011-q1/msg00122.html

I am not sure this is the right direction.  I have a few issues with it.

First, it seems to me that we'll always have to keep some part of
linespecs around, because 'break file:line' is important.  So, we'll
always have to look at the argument in multiple ways and decide what to
do.

Second, I suspect this ties linespecs too closely to the current language.
It seems reasonable to me for 'break ns::func()' to work in a C frame.

Third, IIRC your branch is based on the idea of the parser constructing
an expression, which is then decoded (or evalled?) to find the correct
symbol.  I think this approach neglects the possibility that a linespec
could be ambiguous, another hot topic.

That said, I would welcome a detailed plan to redo linespec.  Maybe
those objections are not very strong.

Tom

Re: [patch 0/2] physname reg.: C++ breakpoints / linespec fixes

[Jan Kratochvil]

On Tue, 07 Jun 2011 22:23:53 +0200, Tom Tromey wrote:
> >>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:
> 
> Jan> linespec is IMNSHO a hack anyway and one day it should be merged
> Jan> with the general expression parser (used for example for `print');
> Jan> which should be further merged with GCC/G++ parser.  Expression
> Jan> Parser Plug-In Available
> Jan> http://sourceware.org/ml/archer/2011-q1/msg00122.html
> 
> I am not sure this is the right direction.  I have a few issues with it.
> 
> First, it seems to me that we'll always have to keep some part of
> linespecs around, because 'break file:line' is important.  So, we'll
> always have to look at the argument in multiple ways and decide what to
> do.

The `line' case is sure not useful for expressions.  But for example
`file:func' is currently implemented only in linespec and it should be
supported even by expressions - see the bottom example.

The `file:line' could be implemented as a special expression as otherwise all
the parsing and escaping of the possibly whitespaced `file' part would have to
be implemented twice and we would get to the same pain.


> Second, I suspect this ties linespecs too closely to the current language.
> It seems reasonable to me for 'break ns::func()' to work in a C frame.

C++ is generally backward compatible with C, I do not find some problems with
fallback to this or that language if the other language parsing gave an error.

For the special cases of some valid parsing in both languages but ambiguous
due to some overloaded C++ operators one can require proper `set language' to
be set.  One can face it only in some heavily overloaded projects (like
Mozilla IIRC) and the developers would be aware of it.


> Third, IIRC your branch is based on the idea of the parser constructing
> an expression, which is then decoded (or evalled?) to find the correct
> symbol.  I think this approach neglects the possibility that a linespec
> could be ambiguous, another hot topic.

Debugger could handle some way even possibly-ambigous expressions.
`print funcname' depends on context and the debugger user is IMO not so
strictly aware of the current context like the programmer is.


Thanks,
Jan


==> a/f.c <==
static void f (void) {}
void x (void) { f (); }

==> b/f.c <==
static void f (void) {}
void y (void) { f (); }

==> m.c <==
extern void x (void);
extern void y (void);
int
main (void)
{
  x ();
  y ();
  return 0;
}

gcc -o m a/f.c b/f.c m.c -Wall -g 
(gdb) b a/f.c:f
Breakpoint 1 at 0x400478: file a/f.c, line 1.
(gdb) b b/f.c:f
Breakpoint 2 at 0x40048c: file b/f.c, line 1.
(gdb) b f
Note: breakpoint 2 also set at pc 0x40048c.
Breakpoint 3 at 0x40048c: file b/f.c, line 1.
(gdb) p f
$1 = {void (void)} 0x400488 <f>
(gdb) p a/f.c:f
No symbol "a" in current context.
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ = it should work.
(gdb) p b/f.c:f
No symbol "b" in current context.
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ = it should work.

Re: [patch 0/2] physname reg.: C++ breakpoints / linespec fixes

[Jan Kratochvil]

On Wed, 08 Jun 2011 16:43:07 +0200, Jan Kratochvil wrote:
> But for example `file:func' is currently implemented only in linespec and it
> should be supported even by expressions - see the bottom example.
[...]
> ==> a/f.c <==
> static void f (void) {}
> void x (void) { f (); }
> 
> ==> b/f.c <==
> static void f (void) {}
> void y (void) { f (); }
> 
> ==> m.c <==
> extern void x (void);
> extern void y (void);
> int
> main (void)
> {
>   x ();
>   y ();
>   return 0;
> }
> 
> gcc -o m a/f.c b/f.c m.c -Wall -g 
> (gdb) b a/f.c:f
> Breakpoint 1 at 0x400478: file a/f.c, line 1.
> (gdb) b b/f.c:f
> Breakpoint 2 at 0x40048c: file b/f.c, line 1.
> (gdb) b f
> Note: breakpoint 2 also set at pc 0x40048c.
> Breakpoint 3 at 0x40048c: file b/f.c, line 1.
> (gdb) p f
> $1 = {void (void)} 0x400488 <f>
> (gdb) p a/f.c:f
> No symbol "a" in current context.
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ = it should work.
> (gdb) p b/f.c:f
> No symbol "b" in current context.
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ = it should work.

I forgot it is doable in expressions, just with a different syntax than in
linespec:

(gdb) p 'a/f.c'::f
$1 = {void (void)} 0x400474 <f>
(gdb) p 'b/f.c'::f
$2 = {void (void)} 0x400488 <f>



Regards,
Jan