[RFC] regresssion(internal-error) printing subprogram argument

  > Indeed, looks like I don't have that.  I have "interfacesS".
> I've attached the whole file.

That explains it :).

> So it may be that we still need to add another special case
> for Ada somewhere.  Would old GDB from before the C++
> wildmatching pass this test for you?

I went binary searching for the source of the regression and
when I found that it was "caused" by the change requiring variables
without debugging information to be cast before they can be printed,
I gently head-slapped myself, adjusted the testcase to use something
other than an integer variable, and voila - even GDB 7.10 suffers
from the problem.  We just didn't see it for integer variables simply
because we were lucky!

I ran out of time again today, but at least the WIP patch got
augmented with a testcase that currently fails before the patch
is applied.

I think the patch itself is probably correct, although I'd like
to do some archeology to understand the comment attached to
that location. I'm pretty confused by it, when we could simply
say that symbols from languages that do not follow the C++ mangling
should not be demangled by gdb_demangle -- at least for as long
as gdb_demangle is equivalent to cplusplus_demangle in practice...

And just as a reminder for myself, I said in my other email
last week that I wanted also to review all the calls to gdb_demangle.
  

On 01/29/2018 10:38 AM, Joel Brobecker wrote:
>> Indeed, looks like I don't have that.  I have "interfacesS".
>> I've attached the whole file.
> 
> That explains it :).
> 
>> So it may be that we still need to add another special case
>> for Ada somewhere.  Would old GDB from before the C++
>> wildmatching pass this test for you?
> 
> I went binary searching for the source of the regression and
> when I found that it was "caused" by the change requiring variables
> without debugging information to be cast before they can be printed,
> I gently head-slapped myself, adjusted the testcase to use something
> other than an integer variable, and voila - even GDB 7.10 suffers
> from the problem.  We just didn't see it for integer variables simply
> because we were lucky!
> 
> I ran out of time again today, but at least the WIP patch got
> augmented with a testcase that currently fails before the patch
> is applied.
> 
> I think the patch itself is probably correct, although I'd like
> to do some archeology to understand the comment attached to
> that location. I'm pretty confused by it, when we could simply
> say that symbols from languages that do not follow the C++ mangling
> should not be demangled by gdb_demangle -- at least for as long
> as gdb_demangle is equivalent to cplusplus_demangle in practice...

Yeah, the commentary around that code isn't exactly clear.
Why doesn't that code use language->la_demangle instead,
for example.

The other day I noticed that gdb_demangle -> bfd_demangle ->
cplus_demangle does have support for demangling other languages.
For Ada, see the GNAT_DEMANGLING handling in
libiberty.c:cplus-dem.c:cplus_demangle, which takes you to:

 /* Demangle ada names.  The encoding is documented in gcc/ada/exp_dbug.ads.  */

 char *
 ada_demangle (const char *mangled, int option ATTRIBUTE_UNUSED)
 {


When I saw that, I wondered why is it that GDB has a separate
implementation of Ada decoding in gdb/ada-lang.c.  The only plausible
explanation I came up with is that gdb's version decodes into a
buffer that is shared between invocations while libiberty's version
always heap-allocates the result.  Maybe it was an efficiency thing?
Do you know?

I ran into that around this discussion
<https://sourceware.org/ml/gdb/2017-11/msg00029.html> where I
was wondering whether we could speed up demangling by letting
bfd_demangle / cplus_demangle try all languages and return back
which one worked:

 ~~~
 An idea I had would be to try to combine most the language_sniff_from_mangled_name
 implementations in one call, by deferring to cplus_demangle which seems
 like could take care of gnu v3, rust, java, gnat, dlang and old C++ mangling
 schemes all at once.  This would avoid the overhead of gdb_demangle and
 bfd_demangle for each language-specific demangling call, at least.  Not sure.
 ~~~

Because ada_decode comes up high in profiles today...

> 
> And just as a reminder for myself, I said in my other email
> last week that I wanted also to review all the calls to gdb_demangle.

Thanks,
Pedro Alves
  

> The other day I noticed that gdb_demangle -> bfd_demangle ->
> cplus_demangle does have support for demangling other languages.
> For Ada, see the GNAT_DEMANGLING handling in
> libiberty.c:cplus-dem.c:cplus_demangle, which takes you to:
> 
>  /* Demangle ada names.  The encoding is documented in gcc/ada/exp_dbug.ads.  */
> 
>  char *
>  ada_demangle (const char *mangled, int option ATTRIBUTE_UNUSED)
>  {

Ha! Thanks for digging; who would have thought...

> When I saw that, I wondered why is it that GDB has a separate
> implementation of Ada decoding in gdb/ada-lang.c.  The only plausible
> explanation I came up with is that gdb's version decodes into a
> buffer that is shared between invocations while libiberty's version
> always heap-allocates the result.  Maybe it was an efficiency thing?
> Do you know?

I don't unfortunately. I'm pretty sure GDB's ada_decode was already
present before I joined AdaCore...

It is true that start up performance is critical, particularly in Ada,
because one of the strengths of Ada is to help design very large apps.
I think that, eventually, we're going to have to accept that computing
power is gained by adding parallelism, and try to take advantage of that
to speed things up (with the dangers in terms of stability that it
entails), so that aspect might become less relevant once we make that
transition.

> I ran into that around this discussion
> <https://sourceware.org/ml/gdb/2017-11/msg00029.html> where I
> was wondering whether we could speed up demangling by letting
> bfd_demangle / cplus_demangle try all languages and return back
> which one worked:
> 
>  ~~~
>  An idea I had would be to try to combine most the language_sniff_from_mangled_name
>  implementations in one call, by deferring to cplus_demangle which seems
>  like could take care of gnu v3, rust, java, gnat, dlang and old C++ mangling
>  schemes all at once.  This would avoid the overhead of gdb_demangle and
>  bfd_demangle for each language-specific demangling call, at least.  Not sure.
>  ~~~
> 
> Because ada_decode comes up high in profiles today...

Can we avoid the calls to most demanglers entirely by passing
the language to symbol_set_names? I didn't do an extensive search
of all the callers. We could have a fallback where language_unknown
means try all the demanglers, but for the cases we care about,
which is mostly DWARF debugging info, that could save a lot of
unnecessary attempts, like you say.

My feeling on the global situation with those demanglers is that
we take a risk each time we use the demanglers themselves to
try to determine which language the symbol belongs to. Both
the C++ and the Ada mangling/encoding algorithms are not exclusive
enough, and we're seeing in this thread how easy it is to get
it wrong. For minimal symbols, we don't have the information,
so that's one valid situation where we have no choice but to guess.
But when reading DWARF debugging information, for instance, we do
have the language information, so we should take advantage of it.

Thinking out loud, this leads me to wonder whether it's a good idea
to store the demangled name instead of the linkage name. You'd do
the search on the mangled name rather than on the demangled one,
so you would not have to guess when loading the symbols. Maybe it's
a good idea if it helps performance, and perhaps we can plug that
weakness differently...