* Re: [PATCH] printcmds.exp: escape curly braces in array print pattern match
[not found] <39D45529.7E67D55E@axis.com>
@ 2000-09-29 11:17 ` Michael Snyder
[not found] ` <39D4E6A9.B8402198@cygnus.com>
0 siblings, 1 reply; 2+ messages in thread
From: Michael Snyder @ 2000-09-29 11:17 UTC (permalink / raw)
To: Orjan Friberg; +Cc: gdb-patches
Orjan Friberg wrote:
>
> The following patch fixes a problem with matching output from printing
> arrays in printcmds.exp. A left curly brace '{' followed by a number
> must be escaped, to avoid it being interpreted as the beginning of a
> bound. I escape it with \\. A right curly brace '}' does not seem to
> have any special meaning unless preceded by '{', but for consistency I
> escaped those as well (to indicate that it's not part of a bound).
> However, it does clutter the exp-file, so it might not be the best way
> to do it. I'm awaiting comments before submitting the corresponding
> patch for setvar.exp.
Verified that it does not cause any additional fails on Solaris.
I would recommend accepting the patch (though I don't know if
there is a preferable way of doing it).
Stan? Fernando?
> On a native gdb, i686-pc-linux-gnu, before the patch:
>
> # of expected passes 658
> # of expected failures 1
> # of unresolved testcases 11
>
> After the patch:
>
> # of expected passes 669
> # of expected failures 1
>
> 2000-09-29 Orjan Friberg <orjanf@axis.com>
>
> * gdb.base/printcmds.exp: Escaped curly braces in array print pattern
> match.
>
> -- ***
> /home/orjanf/gdb-5.0-latest/src/gdb/testsuite/gdb.base/printcmds.exp
> Tue Mar 14 18:01:05 2000
> --- printcmds.exp Fri Sep 29 10:19:19 2000
> *************** proc test_print_int_arrays {} {
> *** 543,555 ****
> gdb_test "set print elements 24" ""
>
> gdb_test "p int1dim" \
> ! " = {0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11}"
> gdb_test "p int2dim" \
> ! " = {{0, 1, 2, 3}, {4, 5, 6, 7}, {8, 9, 10, 11}}"
> gdb_test "p int3dim" \
> ! " = {{{0, 1}, {2, 3}, {4, 5}}, {{6, 7}, {8, 9}, {10, 11}}}"
> gdb_test "p int4dim" \
> ! " = {{{{0, 1}, {2, 3}, {4, 5}}, {{6, 7}, {8, 9}, {10, 11}}}}"
> }
>
> proc test_print_typedef_arrays {} {
> --- 543,555 ----
> gdb_test "set print elements 24" ""
>
> gdb_test "p int1dim" \
> ! " = \\{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11\\}"
> gdb_test "p int2dim" \
> ! " = \\{\\{0, 1, 2, 3\\}, \\{4, 5, 6, 7\\}, \\{8, 9, 10,
> 11\\}\\}"
> gdb_test "p int3dim" \
> ! " = \\{\\{\\{0, 1\\}, \\{2, 3\\}, \\{4, 5\\}\\}, \\{\\{6, 7\\},
> \\{8, 9\
> \}, \\{10, 11\\}\\}\\}"
> gdb_test "p int4dim" \
> ! " = \\{\\{\\{\\{0, 1\\}, \\{2, 3\\}, \\{4, 5\\}\\}, \\{\\{6,
> 7\\}, \\{8,
> 9\\}, \\{10, 11\\}\\}\\}\\}"
> }
>
> proc test_print_typedef_arrays {} {
> *************** proc test_print_typedef_arrays {} {
> *** 558,564 ****
> gdb_test "set print elements 24" ""
>
> gdb_test "p a1" \
> ! " = {2, 4, 6, 8, 10, 12, 14, 16, 18, 20}"
> gdb_test "p a1\[0\]" " = 2"
> gdb_test "p a1\[9\]" " = 20"
>
> --- 558,564 ----
> gdb_test "set print elements 24" ""
>
> gdb_test "p a1" \
> ! " = \\{2, 4, 6, 8, 10, 12, 14, 16, 18, 20\\}"
> gdb_test "p a1\[0\]" " = 2"
> gdb_test "p a1\[9\]" " = 20"
>
> *************** proc test_print_typedef_arrays {} {
> *** 570,581 ****
>
> proc test_artificial_arrays {} {
> # Send \026@ instead of just @ in case the kill character is @.
> ! gdb_test "p int1dim\[0\]\026@2" " = {0, 1}" {p int1dim[0]@2}
> gdb_test "p int1dim\[0\]\026@2\026@3" \
> ! "({{0, 1}, {2, 3}, {4, 5}}|\[Cc\]annot.*)" \
> {p int1dim[0]@2@3}
> gdb_test {p/x (short [])0x12345678} \
> ! " = ({0x1234, 0x5678}|{0x5678, 0x1234})"
> }
>
> proc test_print_char_arrays {} {
> --- 570,581 ----
>
> proc test_artificial_arrays {} {
> # Send \026@ instead of just @ in case the kill character is @.
> ! gdb_test "p int1dim\[0\]\026@2" " = \\{0, 1\\}" {p int1dim[0]@2}
> gdb_test "p int1dim\[0\]\026@2\026@3" \
> ! "(\\{\\{0, 1\\}, \\{2, 3\\}, \\{4, 5\\}\\}|\[Cc\]annot.*)" \
> {p int1dim[0]@2@3}
> gdb_test {p/x (short [])0x12345678} \
> ! " = \\{0x1234, 0x5678\\}|\\{0x5678, 0x1234\\}"
> }
>
> proc test_print_char_arrays {} {
> *************** proc test_print_array_constants {} {
> *** 655,665 ****
> setup_xfail "a29k-*-udi" 2416
> gdb_test "print {'a','b','c'}" " = \"abc\""
> setup_xfail "a29k-*-udi" 2416
> ! gdb_test "print {0,1,2}" " = {0, 1, 2}"
> setup_xfail "a29k-*-udi" 2416
> ! gdb_test "print {(long)0,(long)1,(long)2}" " = {0, 1, 2}"
> setup_xfail "a29k-*-udi" 2416
> ! gdb_test "print {{0,1,2},{3,4,5}}" " = {{0, 1, 2}, {3, 4, 5}}"
> setup_xfail "a29k-*-udi" 2416
> gdb_test "print {4,5,6}\[2\]" " = 6"
> setup_xfail "a29k-*-udi" 2416
> --- 655,665 ----
> setup_xfail "a29k-*-udi" 2416
> gdb_test "print {'a','b','c'}" " = \"abc\""
> setup_xfail "a29k-*-udi" 2416
> ! gdb_test "print {0,1,2}" " = \\{0, 1, 2\\}"
> setup_xfail "a29k-*-udi" 2416
> ! gdb_test "print {(long)0,(long)1,(long)2}" " = \\{0, 1,
> 2\\}"
> setup_xfail "a29k-*-udi" 2416
> ! gdb_test "print {{0,1,2},{3,4,5}}" " = \\{\\{0, 1, 2\\}, \\{3, 4,
> 5\\}\\}
> "
> setup_xfail "a29k-*-udi" 2416
> gdb_test "print {4,5,6}\[2\]" " = 6"
> setup_xfail "a29k-*-udi" 2416
>
> --
> Orjan Friberg E-mail: orjan.friberg@axis.com
> Axis Communications AB Phone: +46 46 272 17 68
From msnyder@redhat.com Fri Sep 29 11:20:00 2000
From: Michael Snyder <msnyder@redhat.com>
To: DJBARROW@de.ibm.com
Cc: gdb-patches@sourceware.cygnus.com, schwidefsky@de.ibm.com, BOAS@de.ibm.com
Subject: Re: New s390 gdb patch
Date: Fri, 29 Sep 2000 11:20:00 -0000
Message-id: <39D4DD55.1F3E@redhat.com>
References: <C1256969.0039B4EB.00@d12mta09.de.ibm.com>
X-SW-Source: 2000-09/msg00244.html
Content-length: 850
DJBARROW@de.ibm.com wrote:
>
> Thanks for your response,
>
> I'll check out again in the next few days once I've other work out of the
> way.
>
> Martin already has submitted patches against bfd so I'll put the binutils
> into a separate patch as gdb binutils
> is not the same as the latest binutils.
>
> The check for link_map_32 was completely wrong for starters there is no
> file called sys/link.h on aix or linux
> & the test didn't even compile thus gcc returned a non zero error code not
> the actual code snippet which was supposed to be
> run therfore HAVE_LINK_MAP_32 was defined in config.h
That (link32) was probably due to a configure bug that has been fixed.
If you just take out any changes of yours and leave it the way
it is in sourceware, hopefully it should work for you now.
Sorry for the trouble (it was my bug).
Michael
From msnyder@redhat.com Fri Sep 29 11:29:00 2000
From: Michael Snyder <msnyder@redhat.com>
To: Syd Polk <spolk@redhat.com>
Cc: Orjan Friberg <orjan.friberg@axis.com>, gdb-patches@sourceware.cygnus.com
Subject: Re: Expect pattern match in printcmds.exp and setvar.exp
Date: Fri, 29 Sep 2000 11:29:00 -0000
Message-id: <39D4DF65.2090@redhat.com>
References: <39D378F8.66B542BF@axis.com> <4.2.0.58.20000929111700.00c16ef0@pop.cygnus.com>
X-SW-Source: 2000-09/msg00245.html
Content-length: 2001
Syd Polk wrote:
>
> At 09:09 PM 9/28/00 +0200, Orjan Friberg wrote:
> >Orjan Friberg wrote:
> > >
> > > Obviously I haven't understood expect's pattern matching, so a URL would
> > > be greatly appreciated. I've been digging through the expect info pages
> > > without finding a list of the meaning of special characters. I have
> > > patches for both test cases ready, but I would like to understand why it
> > > doesn't work in the first place before submitting them.
> > >
> >
> >Just to save other people from the trouble; I educated myself with
> >dev.scriptics.com:
> >
> >{
> > when followed by a character other than a digit, matches the
> >left-brace character `{';
> > when followed by a digit, it is the beginning of a bound (see
> >above)
> >
> >The cases where I had to put \\ the { was followed by a digit, which
> >sort of explains it. Still, I'm a bit surprised that these testcases
> >should fail for native gdb without anybody else noticing, so I will
> >defer from posting the patches until someone else confirms that they
> >actually do.
> >
> >Built from the very latest cvs sources, for a native i686-pc-linux-gnu I
> >get:
> >
> >printcmds.exp:
> >
> ># of expected passes 658
> ># of expected failures 1
> ># of unresolved testcases 11
> >
> >setvar.exp:
> >
> ># of expected passes 98
> ># of unexpected failures 20
> ># of unresolved testcases 22
> >
> >All of the unexpected failures and unresolved testcases are due to this
> >pattern matching problem. runtest --version on my system says:
> >
> >Expect version is 5.31.2
> >Tcl version is 8.2
> >Framework version is 1.3.1
>
> We do not support Tcl 8.2 when running our tests; as you have seen the
> regexp symantics changed. You must run the test suite with the version of
> dejagnu, expect and tcl that get checked out when you check out gdb.
>
> If your change still runs with Tcl 8.0 as the base, then it will be ok.
Seemingly it does.
From fnasser@cygnus.com Fri Sep 29 12:00:00 2000
From: Fernando Nasser <fnasser@cygnus.com>
To: Michael Snyder <msnyder@redhat.com>
Cc: Orjan Friberg <orjan.friberg@axis.com>, gdb-patches@sourceware.cygnus.com
Subject: Re: [PATCH] printcmds.exp: escape curly braces in array print pattern match
Date: Fri, 29 Sep 2000 12:00:00 -0000
Message-id: <39D4E6A9.B8402198@cygnus.com>
References: <39D45529.7E67D55E@axis.com> <39D4DCBE.18DC@redhat.com>
X-SW-Source: 2000-09/msg00246.html
Content-length: 6157
Michael Snyder wrote:
>
> Orjan Friberg wrote:
> >
> > The following patch fixes a problem with matching output from printing
> > arrays in printcmds.exp. A left curly brace '{' followed by a number
> > must be escaped, to avoid it being interpreted as the beginning of a
> > bound. I escape it with \\. A right curly brace '}' does not seem to
> > have any special meaning unless preceded by '{', but for consistency I
> > escaped those as well (to indicate that it's not part of a bound).
> > However, it does clutter the exp-file, so it might not be the best way
> > to do it. I'm awaiting comments before submitting the corresponding
> > patch for setvar.exp.
>
> Verified that it does not cause any additional fails on Solaris.
> I would recommend accepting the patch (though I don't know if
> there is a preferable way of doing it).
>
> Stan? Fernando?
>
Writing a little wrapper proc will do the trick, i.e. let the test pattern clear
and yet fixing the matching problem.
proc gdb_test_escape_braces {pattern args} {
<escape the braces in pattern yelding esc_pattern>
<call gdb_test with the esc_pattern created above>
}
> > On a native gdb, i686-pc-linux-gnu, before the patch:
> >
> > # of expected passes 658
> > # of expected failures 1
> > # of unresolved testcases 11
> >
> > After the patch:
> >
> > # of expected passes 669
> > # of expected failures 1
> >
> > 2000-09-29 Orjan Friberg <orjanf@axis.com>
> >
> > * gdb.base/printcmds.exp: Escaped curly braces in array print pattern
> > match.
> >
> > -- ***
> > /home/orjanf/gdb-5.0-latest/src/gdb/testsuite/gdb.base/printcmds.exp
> > Tue Mar 14 18:01:05 2000
> > --- printcmds.exp Fri Sep 29 10:19:19 2000
> > *************** proc test_print_int_arrays {} {
> > *** 543,555 ****
> > gdb_test "set print elements 24" ""
> >
> > gdb_test "p int1dim" \
> > ! " = {0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11}"
> > gdb_test "p int2dim" \
> > ! " = {{0, 1, 2, 3}, {4, 5, 6, 7}, {8, 9, 10, 11}}"
> > gdb_test "p int3dim" \
> > ! " = {{{0, 1}, {2, 3}, {4, 5}}, {{6, 7}, {8, 9}, {10, 11}}}"
> > gdb_test "p int4dim" \
> > ! " = {{{{0, 1}, {2, 3}, {4, 5}}, {{6, 7}, {8, 9}, {10, 11}}}}"
> > }
> >
> > proc test_print_typedef_arrays {} {
> > --- 543,555 ----
> > gdb_test "set print elements 24" ""
> >
> > gdb_test "p int1dim" \
> > ! " = \\{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11\\}"
> > gdb_test "p int2dim" \
> > ! " = \\{\\{0, 1, 2, 3\\}, \\{4, 5, 6, 7\\}, \\{8, 9, 10,
> > 11\\}\\}"
> > gdb_test "p int3dim" \
> > ! " = \\{\\{\\{0, 1\\}, \\{2, 3\\}, \\{4, 5\\}\\}, \\{\\{6, 7\\},
> > \\{8, 9\
> > \}, \\{10, 11\\}\\}\\}"
> > gdb_test "p int4dim" \
> > ! " = \\{\\{\\{\\{0, 1\\}, \\{2, 3\\}, \\{4, 5\\}\\}, \\{\\{6,
> > 7\\}, \\{8,
> > 9\\}, \\{10, 11\\}\\}\\}\\}"
> > }
> >
> > proc test_print_typedef_arrays {} {
> > *************** proc test_print_typedef_arrays {} {
> > *** 558,564 ****
> > gdb_test "set print elements 24" ""
> >
> > gdb_test "p a1" \
> > ! " = {2, 4, 6, 8, 10, 12, 14, 16, 18, 20}"
> > gdb_test "p a1\[0\]" " = 2"
> > gdb_test "p a1\[9\]" " = 20"
> >
> > --- 558,564 ----
> > gdb_test "set print elements 24" ""
> >
> > gdb_test "p a1" \
> > ! " = \\{2, 4, 6, 8, 10, 12, 14, 16, 18, 20\\}"
> > gdb_test "p a1\[0\]" " = 2"
> > gdb_test "p a1\[9\]" " = 20"
> >
> > *************** proc test_print_typedef_arrays {} {
> > *** 570,581 ****
> >
> > proc test_artificial_arrays {} {
> > # Send \026@ instead of just @ in case the kill character is @.
> > ! gdb_test "p int1dim\[0\]\026@2" " = {0, 1}" {p int1dim[0]@2}
> > gdb_test "p int1dim\[0\]\026@2\026@3" \
> > ! "({{0, 1}, {2, 3}, {4, 5}}|\[Cc\]annot.*)" \
> > {p int1dim[0]@2@3}
> > gdb_test {p/x (short [])0x12345678} \
> > ! " = ({0x1234, 0x5678}|{0x5678, 0x1234})"
> > }
> >
> > proc test_print_char_arrays {} {
> > --- 570,581 ----
> >
> > proc test_artificial_arrays {} {
> > # Send \026@ instead of just @ in case the kill character is @.
> > ! gdb_test "p int1dim\[0\]\026@2" " = \\{0, 1\\}" {p int1dim[0]@2}
> > gdb_test "p int1dim\[0\]\026@2\026@3" \
> > ! "(\\{\\{0, 1\\}, \\{2, 3\\}, \\{4, 5\\}\\}|\[Cc\]annot.*)" \
> > {p int1dim[0]@2@3}
> > gdb_test {p/x (short [])0x12345678} \
> > ! " = \\{0x1234, 0x5678\\}|\\{0x5678, 0x1234\\}"
> > }
> >
> > proc test_print_char_arrays {} {
> > *************** proc test_print_array_constants {} {
> > *** 655,665 ****
> > setup_xfail "a29k-*-udi" 2416
> > gdb_test "print {'a','b','c'}" " = \"abc\""
> > setup_xfail "a29k-*-udi" 2416
> > ! gdb_test "print {0,1,2}" " = {0, 1, 2}"
> > setup_xfail "a29k-*-udi" 2416
> > ! gdb_test "print {(long)0,(long)1,(long)2}" " = {0, 1, 2}"
> > setup_xfail "a29k-*-udi" 2416
> > ! gdb_test "print {{0,1,2},{3,4,5}}" " = {{0, 1, 2}, {3, 4, 5}}"
> > setup_xfail "a29k-*-udi" 2416
> > gdb_test "print {4,5,6}\[2\]" " = 6"
> > setup_xfail "a29k-*-udi" 2416
> > --- 655,665 ----
> > setup_xfail "a29k-*-udi" 2416
> > gdb_test "print {'a','b','c'}" " = \"abc\""
> > setup_xfail "a29k-*-udi" 2416
> > ! gdb_test "print {0,1,2}" " = \\{0, 1, 2\\}"
> > setup_xfail "a29k-*-udi" 2416
> > ! gdb_test "print {(long)0,(long)1,(long)2}" " = \\{0, 1,
> > 2\\}"
> > setup_xfail "a29k-*-udi" 2416
> > ! gdb_test "print {{0,1,2},{3,4,5}}" " = \\{\\{0, 1, 2\\}, \\{3, 4,
> > 5\\}\\}
> > "
> > setup_xfail "a29k-*-udi" 2416
> > gdb_test "print {4,5,6}\[2\]" " = 6"
> > setup_xfail "a29k-*-udi" 2416
> >
> > --
> > Orjan Friberg E-mail: orjan.friberg@axis.com
> > Axis Communications AB Phone: +46 46 272 17 68
--
Fernando Nasser
Red Hat - Toronto E-Mail: fnasser@cygnus.com
2323 Yonge Street, Suite #300 Tel: 416-482-2661 ext. 311
Toronto, Ontario M4P 2C9 Fax: 416-482-6299
From dberlin@cygnus.com Fri Sep 29 12:09:00 2000
From: Daniel Berlin <dberlin@cygnus.com>
To: Elena Zannoni <ezannoni@cygnus.com>
Cc: Elena Zannoni <ezannoni@cygnus.com>, gdb-patches@sources.redhat.com
Subject: Re: [RFA]: Revised C++ patch
Date: Fri, 29 Sep 2000 12:09:00 -0000
Message-id: <Pine.SOL.3.91.1000929114926.23249C-100000@cse.cygnus.com>
References: <14804.54758.687966.413023@kwikemart.cygnus.com>
X-SW-Source: 2000-09/msg00247.html
Content-length: 11257
> > > I would prefer if you could split this up into 4 different patches that
> > > could be checked in separately.
> > Sure, if you like.
>
> Yes please. That would be great.
>
> > > [More comments below]
> > >
> > >
> > > I am extremely confused by these changelog entries. Which ones are the
> > > real ones? You have listed changes for valops.c that are not included
> > > in the diffs.
> > Both are.
> > The valops.c changes are there because they were already approved and
> > put in. I didn't expect to have to wait so long for approval. It was all
> > originally part of the same patch, way back when.
>
> Ok, so, you agree with me, these ChangeLogs don't apply anymore to the
> current patch and need to be changed.
Yes, but that's a minor issue, IMHO.
I need to check in that part of the changelog regardless, because I
wasn't expecting approval to take this long.
In other words, the valops.c entry isn't anywhere *else* right now.
>
> > > Also, why is this split over 2 different ChangeLogs entries? >
> > Because it ook over a month to get approval, and i made revisions.
> > > Some more comments: >
> > > The ChangeLogs should not be submitted as diffs.
> > >
> > Originally, they weren't.
> > I got tired of splitting them out multiple times.
>
> Sorry, I know it's a pain, but there are procedures that have to be
> followed, and I encourage you to do that.
Fine.
>
> > > You changed the macro OPNAME_PREFIX_P, but you didn't update its
> > > comment. Actually, I think this macro is used only in 2 spots,
> > > and it has been simplified, so, could it be eliminated?
> > >
> > The comment still holds, and needed no update.
>
> I am referring to this bit:
>
> Currently 'o' 'p' CPLUS_MARKER is used for both the symbol in the
> symbol-file and the names in gdb's symbol table.
>
> The macro is now checking for 'operator', not for 'o', 'p', and cplus_marker.
Okay, i'll delete the three lines, since they no longer apply.
>
> > No, it can't be eliminated, because it might not be g++ specific in the
> > future.
> > In fact, i can almost guarantee it will stay. When i change everything to
> > stop having different variables for differnet runtime models, i'll update
> > the comment.
> > > Did you need to introduce the goto in decode_line_1()? I would prefer
> > > if there was an alternative approach.
> > >
> > Yes, I did.
> > There is no alternative approach anyone could think of. It would require
> > splitting up decode_line_1, and i jut don't have time to do this and
> > debug it. decode_line_1 is too hairy and too important to break in random
> > ways by splitting it up.
>
> Yes, it is hairy, that's why I thought that a goto would make it even
> more so.
Not really, it specifically only affects template parameters.
What happens is that it strips too much because it wasn't quoted, so we
need to tell it to pretend it was quoted, so it doesn't strip as much.
However, you only can do this in cases where it *does* strip too much, if
you do it all the time, you break it because you'll try to look up a
symbol that doesn't exist. Hence the goto.
> > > We need to retry what we just did with one slightly different
> > setting. If someone can point out how to do this without the goto,
> > and without splitting the function, i'm happy to listen.
>
> I'll take a look.
Please do. I tried for hours, then just gave up.
>
> > > You are eliminating support for GNU mangled names in gdb_mangle_name(),
> > > can you explain why? (I am probably out of the loop on this one).
> > >
> > No i'm not, what gave you that idea. I'm fixing a problem decoding operators.
> > We just don't need to decode them anymore.
> >
>
> OK, I just saw that the specific code was gone. Based on the comment:
> /* Only needed for GNU-mangled names. ANSI-mangled names
> work with the normal mechanisms. */
>
> Can you explain why we don't need to decode them anymore? (sorry, I am
> little thick sometimes)
It's actually that we don't need to *reencode* them anymore.
gdb_mangle_name is trying to generate a mangled name, because in
stabs, the mangled names that are in methods of classes, aren't full
names. so to get the full mangled name, we need to do a little magic.
Since about egcs 1.1.2, operator names, and only operator names, are
fully mangled names, and thus, gdb_mangle_name doesn't need to do
magic to get the mangled name for operators.
It just has to return what we have for the mangled name already, which is
what it now does in he fixed version.
Of course, this breaks C++ support for really old gcc, but C++ suport in
really old GCC was already very broken on it's own, so i'm not really
hurting it.
I plan on checking this part in today, as it's a C++ change, rather than
a symtab change. I have no clue who put the thing in symtab.c, but it's
clearly not symbol related.
> > > What does the setting of the language to language_auto as first thing in
> > > SYMBOL_INIT_DEMANGLED_NAME fix?
> > The fact that languages don't always get set properly before it inits the
> > demangled name, cuasing massive demangling failures.
> > Before, it would go to deamngle it, see the language of the symbol as
> > unknown, and never try to demangle it again, ever.
>
> Ah, Ok, got it. Thanks.
>
> > This is wrong, and a bad optimization.
> > Whether demangling works or not should not be dependent on the order in
> > which you initialize the symbol structures.
> >
> > >
> > > Bottom line, can you show examples of gdb behavior that was broken
> > > before and fixed after these changes?
> > Of course.
> > Java demangling failures, C++ demangling problems in stack backtraces, etc.
> > Things weren't getting demangled when they should.
> > >
> > > Also, I still see some formatting and indentation problems.
> > I went back and reindented it properly later, I just never resubmitted a
> > revised patch.
>
> Ok, make sure you include these fixes when you split the patches into
> smaller ones.
>
> > >
> > > This code:
> > > > + /* If we are using C++ language, demangle the name before doing a lookup, so
> > > > + we can always binary search. */
> > > > + if (current_language->la_language == language_cplus)
> > > > + {
> > > > + modified_name2 = cplus_demangle (modified_name, DMGL_ANSI |
> > > > + DMGL_PARAMS);
> > > > + }
> > > > + else
> > > > + {
> > > > + needtofreename=0;
> > > > + }
> > > > +
> > > > + if (!modified_name2)
> > > > + {
> > > > + modified_name2 = modified_name;
> > > > + needtofreename=0;
> > > > + }
> > > > + returnval = lookup_symbol_aux (modified_name2, block, namespace,
> > > > + is_a_field_of_this, symtab);
> > > > + if (needtofreename)
> > > > + free(modified_name2);
> > > > +
> > > > + return returnval;
> > > > + }
> > >
> > > Could be simplified if needtofreename was initialized to 0 instead of 1.
> >
> > Are you positive? I tried this once, and I ended up with a memory leak,
> > according to purify.
> >
>
> I thought you could get rid of the 2 "needtofreeframe = 0;" bits and
> have just one "needtofreeframe = 1;" statement.
>
> Something like this:
>
>
> if (current_language->la_language == language_cplus)
> {
> modified_name2 = cplus_demangle (modified_name, DMGL_ANSI |
> DMGL_PARAMS);
> if (modified_name2)
> {
> modified_name = modified_name2;
> needtofreename = 1;
> }
> }
>
> returnval = lookup_symbol_aux (modified_name, block, namespace,
> is_a_field_of_this, symtab);
> if (needtofreename)
> free (modified_name2);
>
> return returnval;
>
> Would this work?
I think so, i'll purify it to make sure.
>
> > >
> > > I don't understand the change to use SYMBOL_MATCHES_NAME in
> > > lookup_block_symbol. It doesn't check for
> > > SYMBOL_NAMESPACE (sym) == namespace
> > > anymore.
> > >
> > It doesn't need to in that case, the comment is right. It's impossible
> > for that to occur.
> > So using SYMBOL_MATCHES_NAME alone will take care of it.
> > I didn't remove any of the other namespace checks.
> >
>
> Ok, right. Then you need to change the comment as well because it
> doesn't apply anymore.
Okey dokey.
>
> > >
> > > OK, now that I have stared at your patches for some time, the main idea
> > > is to demangle the c++ name before you do the lookup, and then do a
> > > binary search, instead of a linear search. The lookup functions have
> > > been modified to make sure that we compare on the demangled name if we
> > > have it (by using SYMBOL_SOURCE_NAME instead of SYMBOL_NAME). Yes?
> > >
> > Right.
> >
> > And i made the block lookup changes while i was debugging to make sure it
> > all worked, and noticed we lookup in the same block 100 times for no
> > apparent reason.
>
> Yes, that's the other enhancement.
>
> >
> > > I just tried you patch on solaris 2.5.1 and I got one extra testsuite
> > > failure: FAIL: gdb.c++/namespace.exp: info func xyzq
> > >
> > Yup.
> > This is a problem in the test, actually.
> > If you look at the log, it clearly found the function, the regexp just
> > isn't matching the output anymore for some reason.
> >
>
> I think the order has changed? It could be worthwhile to investigate
> this a little further and update the test if necessary.
Yes, the order has changed.
I'll probably just break it up to try to match each function name seperately.
>
> > > So, OK, I think I need to see these changes split into 4 different
> > > patches. And for the bug fixes I would like examples, ideally
> > > testsuite additions.
> > >
> > I'd rather split it into 3.
> > The symbol changes, the mangle changes, and the name completion changes.
> > Is that okay?
> > I can commit number 2 without approval, actually.
>
> 3 is fine, but I would prefer to see all of them resubmitted if you
> don't mind. I promise to be faster this time around.
Well, I don't need approval for number 2, as I said, because they are C++
fixs, rather than symbol fixes, but i'll resubmit it because that's the
procedure.
> > >
> > It's very simple to reproduce the bugs it's fixing, if you really want
> > to see them.
> > For the operator names, please try creating a map or vector, and calling
> > operator [] by doing
> > print yourmap[5]
> >
> > It won't work, because it'll never find the operator, because it gets
> > mangled wrong.
> >
> > I have quite a few bug reports about this in my inbox somewhere if you
> > really want me to be more specific.
> >
>
> Just one example would be enough.
>
i'll put in a testcase.
> > For the demangling problem, it's a general bug that shows up in various
> > places (like things not being demangled right when you do info
> > functions, etc)
>
> Just one simple testcase would be good.
I'll put it in a testcase.
>
> > Do all of our C++ systems supoprt the STL yet?
> > If so, i'll happily add a bunch of more tets i've been working on, which
> > use the STL.
> >
>
> Don't know.
>
> Elena
>
From kevinb@cygnus.com Fri Sep 29 19:54:00 2000
From: Kevin Buettner <kevinb@cygnus.com>
To: gdb-patches@sourceware.cygnus.com
Subject: Re: [PATCH RFC] Protoize procfs.c, ptx4-nat.c
Date: Fri, 29 Sep 2000 19:54:00 -0000
Message-id: <1000930025404.ZM30053@ocotillo.lan>
References: <1000927181249.ZM19432@ocotillo.lan> <kevinb@cygnus.com>
X-SW-Source: 2000-09/msg00248.html
Content-length: 215
On Sep 27, 11:12am, Kevin Buettner wrote:
> * procfs.c (proc_iterate_over_mappings, proc_iterate_over_threads,
> procfs_xfer_memory): Protoize.
> * ptx4-nat.c (proc_iterate_over_mappings): Protoize.
Committed.
From kevinb@cygnus.com Fri Sep 29 20:05:00 2000
From: Kevin Buettner <kevinb@cygnus.com>
To: gdb-patches@sourceware.cygnus.com
Subject: [PATCH RFC] Protoize remote-adapt.c, remote-array.c
Date: Fri, 29 Sep 2000 20:05:00 -0000
Message-id: <1000930030529.ZM30081@ocotillo.lan>
X-SW-Source: 2000-09/msg00249.html
Content-length: 3084
More protoization...
I did end up removing a FIXME comment. I believe it was admonishing
the maintainer to merge array_write_inferior_memory() and
array_read_inferior_memory() into a single function. I see no reason
to do this; both of these functions are of reasonable size and are
complex enough that it makes sense to leave them separate.
* remote-adapt.c (adapt_insert_breakpoint, adapt_remove_breakpoint):
Protoize.
* remote-array.c (write_monitor, array_xfer_memory): Protoize.
Index: remote-adapt.c
===================================================================
RCS file: /cvs/src/src/gdb/remote-adapt.c,v
retrieving revision 1.4
diff -u -r1.4 remote-adapt.c
--- remote-adapt.c 2000/09/01 00:12:10 1.4
+++ remote-adapt.c 2000/09/30 02:51:43
@@ -1293,10 +1293,14 @@
#define MAX_BREAKS 8
static int num_brkpts = 0;
+
+/* Insert a breakpoint at ADDR. SAVE is normally the address of the
+ pattern buffer where the instruction that the breakpoint overwrites
+ is saved. It is unused here since the Adapt Monitor is responsible
+ for saving/restoring the original instruction. */
+
static int
-adapt_insert_breakpoint (addr, save)
- CORE_ADDR addr;
- char *save; /* Throw away, let adapt save instructions */
+adapt_insert_breakpoint (CORE_ADDR addr, char *save)
{
if (num_brkpts < MAX_BREAKS)
{
@@ -1314,10 +1318,13 @@
}
}
+
+/* Remove a breakpoint at ADDR. SAVE is normally the previously
+ saved pattern, but is unused here as the Adapt Monitor is
+ responsible for saving/restoring instructions. */
+
static int
-adapt_remove_breakpoint (addr, save)
- CORE_ADDR addr;
- char *save; /* Throw away, let adapt save instructions */
+adapt_remove_breakpoint (CORE_ADDR addr, char *save)
{
if (num_brkpts > 0)
{
Index: remote-array.c
===================================================================
RCS file: /cvs/src/src/gdb/remote-array.c,v
retrieving revision 1.6
diff -u -r1.6 remote-array.c
--- remote-array.c 2000/09/01 00:12:10 1.6
+++ remote-array.c 2000/09/30 02:51:44
@@ -216,9 +216,7 @@
* write_monitor -- send raw data to monitor.
*/
static void
-write_monitor (data, len)
- char data[];
- int len;
+write_monitor (char data[], int len)
{
if (SERIAL_WRITE (array_desc, data, len))
fprintf (stderr, "SERIAL_WRITE failed: %s\n", safe_strerror (errno));
@@ -1020,14 +1018,15 @@
return (count);
}
-/* FIXME-someday! merge these two. */
+/* Transfer LEN bytes between GDB address MYADDR and target address
+ MEMADDR. If WRITE is non-zero, transfer them to the target,
+ otherwise transfer them from the target. TARGET is unused.
+
+ Returns the number of bytes transferred. */
+
static int
-array_xfer_memory (memaddr, myaddr, len, write, target)
- CORE_ADDR memaddr;
- char *myaddr;
- int len;
- int write;
- struct target_ops *target; /* ignored */
+array_xfer_memory (CORE_ADDR memaddr, char *myaddr, int len, int write,
+ struct target_ops *target)
{
if (write)
return array_write_inferior_memory (memaddr, myaddr, len);
^ permalink raw reply [flat|nested] 2+ messages in thread