[PATCH] temporary fix for bogus language check

[PATCH] temporary fix for bogus language check

[Fred Fish]

In lookup_symbol(), gdb checks the current language and if it is C++,
trys to demangle the symbol name before looking it up.  This works
fine if we are both looking up a C++ symbol and the current language
is C++.  However it breaks down if we are trying to lookup a C++
symbol and the current language is not C++, such as when stopped
inside a C function (perhaps a C library call for example).

Consider decode_line_1 in linespec.c, which is called when you do
something like "br PCReader::work".  This function calls
find_methods(), which later calls lookup_symbol() with the mangled
name "work__8PCReader".  If the current language is C++,
lookup_symbol() demangles it to "PCReader::work(void)" and calls
lookup_symbol_aux with the demangled name, which succeeds.  If however
you try to set the same breakpoint when stopped at a C function
setting the breakpoint fails.

The following patch works around the problem by always attempting to
demangle the symbol to be looked up.  We could fix this problem by
eliminating the test of the current language, but then every gdb
session would incur the additional overhead of attempting to demangle
every symbol regardless of whether or not any C++ symbols were
present.  Another option is to set some global flag whenever symbols
for a C++ function are read in, and then do the current language test
unconditionally once we know that there might be C++ symbols
somewhere.  Yet another option is to add a parameter to lookup_symbol
that says whether to consider the possibility that the symbol to be
looked up is a C++ symbol, set that appropriately when calling
lookup_symbol, and use that value to decide whether or not to try
demangling the symbol.

Suggestions?

-Fred

Index: symtab.c
===================================================================
RCS file: /cvs/src/src/gdb/symtab.c,v
retrieving revision 1.48
diff -u -r1.48 symtab.c
--- symtab.c	2001/11/13 16:42:50	1.48
+++ symtab.c	2001/11/25 18:49:46
@@ -528,8 +528,11 @@
       modified_name = (char *) name;
 
   /* 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)
+     we can always binary search.
+     NOTE: We need to always try to demangle since the current_language might
+     be something other than C++ at the point when we are trying to set a
+     breakpoint in C++ code.  -fnf */
+  if (1 || (current_language->la_language == language_cplus))
     {
       modified_name2 = cplus_demangle (modified_name, DMGL_ANSI | DMGL_PARAMS);
       if (modified_name2)

Re: [PATCH] temporary fix for bogus language check

[Daniel Berlin]

On Sun, 25 Nov 2001, Fred Fish wrote:

> In lookup_symbol(), gdb checks the current language and if it is C++,
> trys to demangle the symbol name before looking it up.  This works
> fine if we are both looking up a C++ symbol and the current language
> is C++.  However it breaks down if we are trying to lookup a C++
> symbol and the current language is not C++, such as when stopped
> inside a C function (perhaps a C library call for example).
>
> Consider decode_line_1 in linespec.c, which is called when you do
> something like "br PCReader::work".  This function calls
> find_methods(), which later calls lookup_symbol() with the mangled
> name "work__8PCReader".  If the current language is C++,
> lookup_symbol() demangles it to "PCReader::work(void)" and calls
> lookup_symbol_aux with the demangled name, which succeeds.  If however
> you try to set the same breakpoint when stopped at a C function
> setting the breakpoint fails.

Of course, this raises the question, what exactly is the current language
supposed to mean? The language of the frame, or the language of the
expressions you are trying to enter.
If it's the first, this behavior is incorrect.
If it's the second, this behavior is correct.
The help says "Current source language", which doesn't help resolve the
problem, since we've never cared what the source language of the file is,
only what it's supposed to tell us about either the frame language, or the
expression language.
 >
> The following patch works around the problem by always attempting to
> demangle the symbol to be looked up

I remember doing this originally, and somebody objected that we
should only do it if the current language is C++, to avoid the cost of
always trying to demangle.  I didn't have the energy to argue.


>.  We could fix this problem by
> eliminating the test of the current language, but then every gdb
> session would incur the additional overhead of attempting to demangle
> every symbol regardless of whether or not any C++ symbols were
> present.  Another option is to set some global flag whenever symbols
> for a C++ function are read in, and then do the current language test
> unconditionally once we know that there might be C++ symbols
> somewhere.  Yet another option is to add a parameter to lookup_symbol
> that says whether to consider the possibility that the symbol to be
> looked up is a C++ symbol, set that appropriately when calling
> lookup_symbol, and use that value to decide whether or not to try
> demangling the symbol.

These shouldn't just be C++ specific.
IE modify the suggestions so that lookup_symbol should take a parameter
that tells you what languages the symbol might be for (maybe including an order so that the current frame
language/expression language is preferred over other languages, or
something), and we only do the demangling if they want to look it up in
C++.
Or keep a list of languages we've seen, rather than just whether we've
seen C++ or not.

Another option is to detect whether the name even could be mangled without
demangling it.
At least for C++, we know the ABI's can't be mixed, and thus, you have
only one C++ ABI.
So, you could add a function to the cp_abi_ops structure called
"could_be_mangled_name", and fill it in for the various C++ ABI's.

For gnu-v2, it's tricky to determine whether a name is mangled without
trying to demangle it (thus, you could just always return 1 in
could_be_mangled_name), but for gnu-v3, it's trivial.


> > Suggestions?
>
> -Fred
>
> Index: symtab.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/symtab.c,v
> retrieving revision 1.48
> diff -u -r1.48 symtab.c
> --- symtab.c	2001/11/13 16:42:50	1.48
> +++ symtab.c	2001/11/25 18:49:46
> @@ -528,8 +528,11 @@
>        modified_name = (char *) name;
>
>    /* 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)
> +     we can always binary search.
> +     NOTE: We need to always try to demangle since the current_language might
> +     be something other than C++ at the point when we are trying to set a
> +     breakpoint in C++ code.  -fnf */
> +  if (1 || (current_language->la_language == language_cplus))
>      {
>        modified_name2 = cplus_demangle (modified_name, DMGL_ANSI | DMGL_PARAMS);
>        if (modified_name2)
>

Re: [PATCH] temporary fix for bogus language check

[Eli Zaretskii]

On Sun, 25 Nov 2001, Fred Fish wrote:


> We could fix this problem by
> eliminating the test of the current language, but then every gdb
> session would incur the additional overhead of attempting to demangle
> every symbol regardless of whether or not any C++ symbols were
> present.  Another option is to set some global flag whenever symbols
> for a C++ function are read in, and then do the current language test
> unconditionally once we know that there might be C++ symbols
> somewhere.  Yet another option is to add a parameter to lookup_symbol
> that says whether to consider the possibility that the symbol to be
> looked up is a C++ symbol, set that appropriately when calling
> lookup_symbol, and use that value to decide whether or not to try
> demangling the symbol.

I think we should consider all the languages used in the program being
debugged.  How to find out what those languages are and whether to
cache them in some list is an implementation detail.

Re: [PATCH] temporary fix for bogus language check

[Elena Zannoni]

Fred Fish writes:
 > In lookup_symbol(), gdb checks the current language and if it is C++,
 > trys to demangle the symbol name before looking it up.  This works
 > fine if we are both looking up a C++ symbol and the current language
 > is C++.  However it breaks down if we are trying to lookup a C++
 > symbol and the current language is not C++, such as when stopped
 > inside a C function (perhaps a C library call for example).
 > 
 > Consider decode_line_1 in linespec.c, which is called when you do
 > something like "br PCReader::work".  This function calls
 > find_methods(), which later calls lookup_symbol() with the mangled
 > name "work__8PCReader".  If the current language is C++,
 > lookup_symbol() demangles it to "PCReader::work(void)" and calls
 > lookup_symbol_aux with the demangled name, which succeeds.  If however
 > you try to set the same breakpoint when stopped at a C function
 > setting the breakpoint fails.
 > 
 > The following patch works around the problem by always attempting to
 > demangle the symbol to be looked up.  We could fix this problem by
 > eliminating the test of the current language, but then every gdb
 > session would incur the additional overhead of attempting to demangle
 > every symbol regardless of whether or not any C++ symbols were
 > present.  Another option is to set some global flag whenever symbols
 > for a C++ function are read in, and then do the current language test
 > unconditionally once we know that there might be C++ symbols
 > somewhere.  Yet another option is to add a parameter to lookup_symbol
 > that says whether to consider the possibility that the symbol to be
 > looked up is a C++ symbol, set that appropriately when calling
 > lookup_symbol, and use that value to decide whether or not to try
 > demangling the symbol.


Hmm, I can see the problem. We have the language information stored in
the symtabs, but we don't know where the symbol is yet, so that's
useless.  I am not sure that the idea of saving somewhere the language
information as you read symbols in would work here. It may be that you
are setting your very first breakpoint in a c++ function, and you
haven't read the symbols in yet. Is psymtab_to_symtab called before
lookup_symbol or not?

Actually I start to like the idea that the caller of lookup_symbol
should know whether or not the name should be demangled. After all
decode_line_1 has already figured out it is dealing with a c++ name,
why throw that info away? Or (thinking out loud) would it make sense
to never pass a mangled symbol to lookup_symbol (can the caller take
care of demangling it?).

Elena


 > 
 > Suggestions?
 > 
 > -Fred
 > 
 > Index: symtab.c
 > ===================================================================
 > RCS file: /cvs/src/src/gdb/symtab.c,v
 > retrieving revision 1.48
 > diff -u -r1.48 symtab.c
 > --- symtab.c	2001/11/13 16:42:50	1.48
 > +++ symtab.c	2001/11/25 18:49:46
 > @@ -528,8 +528,11 @@
 >        modified_name = (char *) name;
 >  
 >    /* 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)
 > +     we can always binary search.
 > +     NOTE: We need to always try to demangle since the current_language might
 > +     be something other than C++ at the point when we are trying to set a
 > +     breakpoint in C++ code.  -fnf */
 > +  if (1 || (current_language->la_language == language_cplus))
 >      {
 >        modified_name2 = cplus_demangle (modified_name, DMGL_ANSI | DMGL_PARAMS);
 >        if (modified_name2)

[PATCH] temporary fix for bogus language check

[Fred Fish]

In lookup_symbol(), gdb checks the current language and if it is C++,
trys to demangle the symbol name before looking it up.  This works
fine if we are both looking up a C++ symbol and the current language
is C++.  However it breaks down if we are trying to lookup a C++
symbol and the current language is not C++, such as when stopped
inside a C function (perhaps a C library call for example).

Consider decode_line_1 in linespec.c, which is called when you do
something like "br PCReader::work".  This function calls
find_methods(), which later calls lookup_symbol() with the mangled
name "work__8PCReader".  If the current language is C++,
lookup_symbol() demangles it to "PCReader::work(void)" and calls
lookup_symbol_aux with the demangled name, which succeeds.  If however
you try to set the same breakpoint when stopped at a C function
setting the breakpoint fails.

The following patch works around the problem by always attempting to
demangle the symbol to be looked up.  We could fix this problem by
eliminating the test of the current language, but then every gdb
session would incur the additional overhead of attempting to demangle
every symbol regardless of whether or not any C++ symbols were
present.  Another option is to set some global flag whenever symbols
for a C++ function are read in, and then do the current language test
unconditionally once we know that there might be C++ symbols
somewhere.  Yet another option is to add a parameter to lookup_symbol
that says whether to consider the possibility that the symbol to be
looked up is a C++ symbol, set that appropriately when calling
lookup_symbol, and use that value to decide whether or not to try
demangling the symbol.

Suggestions?

-Fred

Index: symtab.c
===================================================================
RCS file: /cvs/src/src/gdb/symtab.c,v
retrieving revision 1.48
diff -u -r1.48 symtab.c
--- symtab.c	2001/11/13 16:42:50	1.48
+++ symtab.c	2001/11/25 18:49:46
@@ -528,8 +528,11 @@
       modified_name = (char *) name;
 
   /* 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)
+     we can always binary search.
+     NOTE: We need to always try to demangle since the current_language might
+     be something other than C++ at the point when we are trying to set a
+     breakpoint in C++ code.  -fnf */
+  if (1 || (current_language->la_language == language_cplus))
     {
       modified_name2 = cplus_demangle (modified_name, DMGL_ANSI | DMGL_PARAMS);
       if (modified_name2)

Re: [PATCH] temporary fix for bogus language check

[Daniel Berlin]

On Sun, 25 Nov 2001, Fred Fish wrote:

> In lookup_symbol(), gdb checks the current language and if it is C++,
> trys to demangle the symbol name before looking it up.  This works
> fine if we are both looking up a C++ symbol and the current language
> is C++.  However it breaks down if we are trying to lookup a C++
> symbol and the current language is not C++, such as when stopped
> inside a C function (perhaps a C library call for example).
>
> Consider decode_line_1 in linespec.c, which is called when you do
> something like "br PCReader::work".  This function calls
> find_methods(), which later calls lookup_symbol() with the mangled
> name "work__8PCReader".  If the current language is C++,
> lookup_symbol() demangles it to "PCReader::work(void)" and calls
> lookup_symbol_aux with the demangled name, which succeeds.  If however
> you try to set the same breakpoint when stopped at a C function
> setting the breakpoint fails.

Of course, this raises the question, what exactly is the current language
supposed to mean? The language of the frame, or the language of the
expressions you are trying to enter.
If it's the first, this behavior is incorrect.
If it's the second, this behavior is correct.
The help says "Current source language", which doesn't help resolve the
problem, since we've never cared what the source language of the file is,
only what it's supposed to tell us about either the frame language, or the
expression language.
 >
> The following patch works around the problem by always attempting to
> demangle the symbol to be looked up

I remember doing this originally, and somebody objected that we
should only do it if the current language is C++, to avoid the cost of
always trying to demangle.  I didn't have the energy to argue.


>.  We could fix this problem by
> eliminating the test of the current language, but then every gdb
> session would incur the additional overhead of attempting to demangle
> every symbol regardless of whether or not any C++ symbols were
> present.  Another option is to set some global flag whenever symbols
> for a C++ function are read in, and then do the current language test
> unconditionally once we know that there might be C++ symbols
> somewhere.  Yet another option is to add a parameter to lookup_symbol
> that says whether to consider the possibility that the symbol to be
> looked up is a C++ symbol, set that appropriately when calling
> lookup_symbol, and use that value to decide whether or not to try
> demangling the symbol.

These shouldn't just be C++ specific.
IE modify the suggestions so that lookup_symbol should take a parameter
that tells you what languages the symbol might be for (maybe including an order so that the current frame
language/expression language is preferred over other languages, or
something), and we only do the demangling if they want to look it up in
C++.
Or keep a list of languages we've seen, rather than just whether we've
seen C++ or not.

Another option is to detect whether the name even could be mangled without
demangling it.
At least for C++, we know the ABI's can't be mixed, and thus, you have
only one C++ ABI.
So, you could add a function to the cp_abi_ops structure called
"could_be_mangled_name", and fill it in for the various C++ ABI's.

For gnu-v2, it's tricky to determine whether a name is mangled without
trying to demangle it (thus, you could just always return 1 in
could_be_mangled_name), but for gnu-v3, it's trivial.


> > Suggestions?
>
> -Fred
>
> Index: symtab.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/symtab.c,v
> retrieving revision 1.48
> diff -u -r1.48 symtab.c
> --- symtab.c	2001/11/13 16:42:50	1.48
> +++ symtab.c	2001/11/25 18:49:46
> @@ -528,8 +528,11 @@
>        modified_name = (char *) name;
>
>    /* 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)
> +     we can always binary search.
> +     NOTE: We need to always try to demangle since the current_language might
> +     be something other than C++ at the point when we are trying to set a
> +     breakpoint in C++ code.  -fnf */
> +  if (1 || (current_language->la_language == language_cplus))
>      {
>        modified_name2 = cplus_demangle (modified_name, DMGL_ANSI | DMGL_PARAMS);
>        if (modified_name2)
>

Re: [PATCH] temporary fix for bogus language check

[Eli Zaretskii]

On Sun, 25 Nov 2001, Fred Fish wrote:


> We could fix this problem by
> eliminating the test of the current language, but then every gdb
> session would incur the additional overhead of attempting to demangle
> every symbol regardless of whether or not any C++ symbols were
> present.  Another option is to set some global flag whenever symbols
> for a C++ function are read in, and then do the current language test
> unconditionally once we know that there might be C++ symbols
> somewhere.  Yet another option is to add a parameter to lookup_symbol
> that says whether to consider the possibility that the symbol to be
> looked up is a C++ symbol, set that appropriately when calling
> lookup_symbol, and use that value to decide whether or not to try
> demangling the symbol.

I think we should consider all the languages used in the program being
debugged.  How to find out what those languages are and whether to
cache them in some list is an implementation detail.

Re: [PATCH] temporary fix for bogus language check

[Elena Zannoni]

Fred Fish writes:
 > In lookup_symbol(), gdb checks the current language and if it is C++,
 > trys to demangle the symbol name before looking it up.  This works
 > fine if we are both looking up a C++ symbol and the current language
 > is C++.  However it breaks down if we are trying to lookup a C++
 > symbol and the current language is not C++, such as when stopped
 > inside a C function (perhaps a C library call for example).
 > 
 > Consider decode_line_1 in linespec.c, which is called when you do
 > something like "br PCReader::work".  This function calls
 > find_methods(), which later calls lookup_symbol() with the mangled
 > name "work__8PCReader".  If the current language is C++,
 > lookup_symbol() demangles it to "PCReader::work(void)" and calls
 > lookup_symbol_aux with the demangled name, which succeeds.  If however
 > you try to set the same breakpoint when stopped at a C function
 > setting the breakpoint fails.
 > 
 > The following patch works around the problem by always attempting to
 > demangle the symbol to be looked up.  We could fix this problem by
 > eliminating the test of the current language, but then every gdb
 > session would incur the additional overhead of attempting to demangle
 > every symbol regardless of whether or not any C++ symbols were
 > present.  Another option is to set some global flag whenever symbols
 > for a C++ function are read in, and then do the current language test
 > unconditionally once we know that there might be C++ symbols
 > somewhere.  Yet another option is to add a parameter to lookup_symbol
 > that says whether to consider the possibility that the symbol to be
 > looked up is a C++ symbol, set that appropriately when calling
 > lookup_symbol, and use that value to decide whether or not to try
 > demangling the symbol.


Hmm, I can see the problem. We have the language information stored in
the symtabs, but we don't know where the symbol is yet, so that's
useless.  I am not sure that the idea of saving somewhere the language
information as you read symbols in would work here. It may be that you
are setting your very first breakpoint in a c++ function, and you
haven't read the symbols in yet. Is psymtab_to_symtab called before
lookup_symbol or not?

Actually I start to like the idea that the caller of lookup_symbol
should know whether or not the name should be demangled. After all
decode_line_1 has already figured out it is dealing with a c++ name,
why throw that info away? Or (thinking out loud) would it make sense
to never pass a mangled symbol to lookup_symbol (can the caller take
care of demangling it?).

Elena


 > 
 > Suggestions?
 > 
 > -Fred
 > 
 > Index: symtab.c
 > ===================================================================
 > RCS file: /cvs/src/src/gdb/symtab.c,v
 > retrieving revision 1.48
 > diff -u -r1.48 symtab.c
 > --- symtab.c	2001/11/13 16:42:50	1.48
 > +++ symtab.c	2001/11/25 18:49:46
 > @@ -528,8 +528,11 @@
 >        modified_name = (char *) name;
 >  
 >    /* 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)
 > +     we can always binary search.
 > +     NOTE: We need to always try to demangle since the current_language might
 > +     be something other than C++ at the point when we are trying to set a
 > +     breakpoint in C++ code.  -fnf */
 > +  if (1 || (current_language->la_language == language_cplus))
 >      {
 >        modified_name2 = cplus_demangle (modified_name, DMGL_ANSI | DMGL_PARAMS);
 >        if (modified_name2)