Bug 46332 - __cxa_demangle yields excess parentheses for function types
Summary: __cxa_demangle yields excess parentheses for function types
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: other (show other bugs)
Version: unknown
: P3 normal
Target Milestone: ---
Assignee: Ian Lance Taylor
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-11-06 15:39 UTC by Eelis
Modified: 2010-11-16 14:51 UTC (History)
5 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2010-11-10 05:17:05


Attachments
Possible patch (537 bytes, patch)
2010-11-09 19:55 UTC, Ian Lance Taylor
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Eelis 2010-11-06 15:39:20 UTC
Consider:

  #include <iostream>
  #include <typeinfo>
  #include <cxxabi.h>

  int main()
  {
    std::cout <<
     abi::__cxa_demangle(typeid( void() ).name(), 0, 0, 0) << '\n';
  }

The output is "void ()()", but it should really just be "void ()". If "void ()()" had any meaning at all, it would denote the type of a function returning a function, but of course that's not valid C++.

The same problem can also be seen by trying to compile and link the following program:

  template <typename> void f();
  int main() { f<void()>(); }

This produces:

  undefined reference to `void f<void ()()>()'

Here, too, "void ()()" is wrong and should just be "void ()".
Comment 1 Paolo Carlini 2010-11-07 11:08:40 UTC
Ian, can you have a look? Thanks in advance.
Comment 2 Paolo Carlini 2010-11-09 17:02:21 UTC
Actually, if this happens also in error messages cannot be a demangler issue only. I'm pretty sure to have seen this behavior mentioned already... Ian, any hint?
Comment 3 Eelis 2010-11-09 17:26:40 UTC
(In reply to comment #2)
> I'm pretty sure to have seen this behavior mentioned already...

You may be thinking of bug #36002. That one was about errors emitted during compilation, and has been fixed. This time it's about errors emitted during linking (and demangling explicitly). :)
Comment 4 Paolo Carlini 2010-11-09 18:07:08 UTC
Ah, ok, now I notice the linker is involved, thanks for emphasizing that and the fixed PR. Well, basing on that PR, this one should be fixable pretty easily!
Comment 5 Paolo Carlini 2010-11-09 18:09:54 UTC
HJ, can you help us figuring out which specific patch fixed 36002? Then I can work on applying the same tweak to the demangler...
Comment 6 Ian Lance Taylor 2010-11-09 19:53:18 UTC
I have a patch for this, but it breaks the libstdc++ test testsuite/abi/demangle/abi_examples/14.cc .  That test expects _Z3fooIiFvdEiEvv to demangle to "void foo<int, void ()(double), int>()".  With my patch it demangles to "void foo<int, void (double), int>()".  That is, the inner "()" goes away.  Now, clearly the inner "()" makes little sense, but I wonder whether it should be "(*)" instead.

Ben, you added the test on 2003-02-27.  The test says it comes from the ABI doc, but I can't find anything in the ABI doc which looks quite like this case.  Do you recall anything about this test?  Do you think simply omitting the inner "()" would be an appropriate demangling?
Comment 7 Ian Lance Taylor 2010-11-09 19:55:05 UTC
Created attachment 22352 [details]
Possible patch

This is the patch I tested which drops the inner "()".
Comment 8 Paolo Carlini 2010-11-09 19:55:49 UTC
Thanks.
Comment 9 H.J. Lu 2010-11-09 19:57:47 UTC
(In reply to comment #5)
> HJ, can you help us figuring out which specific patch fixed 36002? Then I can
> work on applying the same tweak to the demangler...


It was fixed by revision 145365:

http://gcc.gnu.org/ml/gcc-cvs/2009-03/msg00872.html
Comment 10 Benjamin Kosnik 2010-11-09 23:00:30 UTC
Hey Ian. I believe this comes from:

http://www.codesourcery.com/public/cxx-abi/abi-examples.html#mangling

But this looks to be slightly different from what is on the table now, ie:

_Z3fooIiPFidEiEvv 	void foo<int,int(*)(double),int>();
(Note: return type encoded for template function.)

From that, I see the (*) is the approved demangling. That does seem to be better to me than just removing the ().
Comment 11 Ian Lance Taylor 2010-11-10 05:17:05 UTC
I agree that for _Z3fooIiPFidEiEvv we should use (*), and in fact we do use (*), with or without the patch attached to this PR.  This PR is about a different sort of symbol, _Z3fooIiFvdEiEvv, or, in the current 14.cc, _Z3fooIiFvdEiEvv.  The key difference here is that these symbols use F rather than PF, which means that refer to a function type rather than a pointer to a function type.  I'm not sure exactly what it means to instantiate a template with a function type, but g++ does generate these sort of symbols.  I could change the () to (*), but that would imply a pointer to a function type which isn't what we have.

So my question for Ben is: should we change the expected demangling in 14.cc to be void foo<int, void (double), int>() or should we do something else.
Comment 12 Paolo Carlini 2010-11-10 08:24:59 UTC
IMHO, void (double) is fine for a function type: Ian's point about function type vs pointer to function type fully clarifies the issue for me. If currently the C++ front-end also uses 'void (double)' in error messages, consistency would be an additional reason.
Comment 13 Benjamin Kosnik 2010-11-10 15:24:28 UTC
> Should we change the expected demangling in 14.cc to
> be void foo<int, void (double), int>() or should we do something else.

I think 14.cc should be foo<int, void (double), int>(). Your change is ok with me.

FWIW, this is also consistent to C++ hackers using C++0x's std::function notation. So, at least we are consistent with that (as well as g++ error messages as pointed out by Paolo.)
Comment 14 Jonathan Wakely 2010-11-10 16:46:17 UTC
Yep, looks right to me
void (double) is a function type
void (*)(double) is a pointer to such a function type
void (&)(double) is a reference to such a type
Comment 15 ian@gcc.gnu.org 2010-11-13 01:21:17 UTC
Author: ian
Date: Sat Nov 13 01:21:12 2010
New Revision: 166695

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=166695
Log:
libiberty/:
	PR other/46332
	* cp-demangle.c (d_print_function_type): Don't print parentheses
	if there are no modifiers to print.
	* testsuite/demangle-expected: Tweak one test case, add another.
libstdc++/:
	* testsuite/abi/demangle/abi_examples/14.cc (main): Change
	expected demangling.

Modified:
    trunk/libiberty/ChangeLog
    trunk/libiberty/cp-demangle.c
    trunk/libiberty/testsuite/demangle-expected
    trunk/libstdc++-v3/ChangeLog
    trunk/libstdc++-v3/testsuite/abi/demangle/abi_examples/14.cc
Comment 16 Ian Lance Taylor 2010-11-13 01:24:20 UTC
Fixed in mainline.  Thanks for reporting the problem.

In practice the real fix is going to be in the GNU binutils, since that is the demangler code seen by most people.  I will get this patch into the next GNU binutils release.
Comment 17 Ian Lance Taylor 2010-11-16 14:51:44 UTC
I have added this patch to the binutils 2.21 release branch, so it will be in the GNU binutils 2.21 release.