Patchwork RFC [ABI]: C++ PATCH for c++/44540 - avoid mangling attribute noreturn

login
register
mail settings
Submitter Jason Merrill
Date July 6, 2010, 9:49 p.m.
Message ID <4C33A4D2.8020105@redhat.com>
Download mbox | patch
Permalink /patch/58065/
State New
Headers show

Comments

Jason Merrill - July 6, 2010, 9:49 p.m.
The ICE in PR 44540 called attention to a long-standing mangling bug: 
attributes noreturn and const are represented internally as 
TYPE_VOLATILE and TYPE_CONST on FUNCTION_TYPE, and we were emitting them 
as cv-quals on the mangled form of the function type in a parameter of 
pointer-to-function type.  Clearly this is wrong, as they aren't part of 
the type system, and a declaration which didn't use the attributes would 
mangle differently.

The testcase was derived from the systemtap sources, where the relevant 
declaration has C linkage, so it would not be affected by this change. 
But it is an ABI change.

Thoughts?

Jason
Mark Mitchell - July 6, 2010, 10:23 p.m.
Jason Merrill wrote:

> The ICE in PR 44540 called attention to a long-standing mangling bug:
> attributes noreturn and const are represented internally as
> TYPE_VOLATILE and TYPE_CONST on FUNCTION_TYPE, and we were emitting them
> as cv-quals on the mangled form of the function type in a parameter of
> pointer-to-function type.

Oh, ugh.  I've always thought that using TYPE_{VOLATILE,CONST} for these
attributes was an ugly representation hack, but I didn't think about the
impact on mangling.

> The testcase was derived from the systemtap sources, where the relevant
> declaration has C linkage, so it would not be affected by this change.
> But it is an ABI change.

I guess I think this ought to require an -fabi-version=N option.  Your
change is clearly correct, but to the extent that this matters to
anyone, it is going to break existing working code.
Richard Guenther - July 7, 2010, 9:30 a.m.
On Wed, Jul 7, 2010 at 12:23 AM, Mark Mitchell <mark@codesourcery.com> wrote:
> Jason Merrill wrote:
>
>> The ICE in PR 44540 called attention to a long-standing mangling bug:
>> attributes noreturn and const are represented internally as
>> TYPE_VOLATILE and TYPE_CONST on FUNCTION_TYPE, and we were emitting them
>> as cv-quals on the mangled form of the function type in a parameter of
>> pointer-to-function type.
>
> Oh, ugh.  I've always thought that using TYPE_{VOLATILE,CONST} for these
> attributes was an ugly representation hack, but I didn't think about the
> impact on mangling.
>
>> The testcase was derived from the systemtap sources, where the relevant
>> declaration has C linkage, so it would not be affected by this change.
>> But it is an ABI change.
>
> I guess I think this ought to require an -fabi-version=N option.  Your
> change is clearly correct, but to the extent that this matters to
> anyone, it is going to break existing working code.

Wouldn't it be better to steal some other bits for this purpose and
"fix" it on the middle-end side?

Richard.

> --
> Mark Mitchell
> CodeSourcery
> mark@codesourcery.com
> (650) 331-3385 x713
>
Mark Mitchell - July 7, 2010, 3:14 p.m.
Richard Guenther wrote:

>> I guess I think this ought to require an -fabi-version=N option.  Your
>> change is clearly correct, but to the extent that this matters to
>> anyone, it is going to break existing working code.
> 
> Wouldn't it be better to steal some other bits for this purpose and
> "fix" it on the middle-end side?

I think that at some point the mangling has to change.  It's valid to
declare these functions with and without an attribute.  So, we have to
do something.

I agree that an orthogonal improvement would be to stop abusing
TYPE_CONST and TYPE_VOLATILE for these attributes and record them on
TYPE_ATTRIBUTES.  But, that would be much bigger than Jason's patch.
Joseph S. Myers - July 26, 2010, 2:17 p.m.
On Wed, 7 Jul 2010, Mark Mitchell wrote:

> I agree that an orthogonal improvement would be to stop abusing
> TYPE_CONST and TYPE_VOLATILE for these attributes and record them on
> TYPE_ATTRIBUTES.  But, that would be much bigger than Jason's patch.

And, to stop sometimes putting them on a DECL but not on the type of that 
DECL; if you stop using the qualifiers as the internal representation, 
moving to recording them internally only on types should be easier.

Patch

commit c22c049821b367a80c65b3fce9a1e196ef63f956
Author: Jason Merrill <jason@redhat.com>
Date:   Thu Jul 1 14:32:58 2010 -0400

    	PR c++/44540
    	* mangle.c (write_CV_qualifiers_for_type): Ignore TYPE_QUALS on
    	FUNCTION_TYPE and METHOD_TYPE.

diff --git a/gcc/cp/mangle.c b/gcc/cp/mangle.c
index e825952..40b21b9 100644
--- a/gcc/cp/mangle.c
+++ b/gcc/cp/mangle.c
@@ -1978,6 +1978,11 @@  write_CV_qualifiers_for_type (const tree type)
      array.  */
   cp_cv_quals quals = TYPE_QUALS (type);
 
+  /* Attribute const/noreturn are not reflected in mangling.  */
+  if (TREE_CODE (type) == FUNCTION_TYPE
+      || TREE_CODE (type) == METHOD_TYPE)
+    return 0;
+
   if (quals & TYPE_QUAL_RESTRICT)
     {
       write_char ('r');
diff --git a/gcc/testsuite/g++.dg/ext/noreturn1.C b/gcc/testsuite/g++.dg/ext/noreturn1.C
new file mode 100644
index 0000000..11e0aed
--- /dev/null
+++ b/gcc/testsuite/g++.dg/ext/noreturn1.C
@@ -0,0 +1,13 @@ 
+// Test that attribute noreturn is not part of the mangled name.
+
+void baz (const char *fmt, ...);
+
+// { dg-final { scan-assembler "_Z3barPFvPKczE" } }
+void bar (void (*baz) (const char *fmt, ...)
+			   __attribute__ ((noreturn, format (printf, 1, 2))));
+
+void
+foo ()
+{
+  bar (&baz);
+}