diff mbox

Fix c++/78621 generic lambda mangling

Message ID bb0243cb-938a-5fd5-e641-64dc4430e5a7@acm.org
State New
Headers show

Commit Message

Nathan Sidwell Dec. 7, 2016, 6:30 p.m. UTC
This patch fixes the generic lambda mangling bug that caused the 
demangler to infinitely recurse (78252).  The generic's auto parms fail 
the is_auto check because that's testing for the 'auto' identifier, 
whereas the parms are distinct template type parms named 'auto:$N'. 
Amending is_auto to do a textual compare of the identifier breaks 
things, and is insufficient anyway.  The mangler's squangling must (a) 
squangle subsequent auto uses as expected and (b) NOT squangle 
subsequent template type parm references to refer the the lambda's autos.

This patch achieves that by tsubsting the generic fn's parameters using 
a consed up array of regular auto types.  The make_auto_1 function is 
further split, so we can control the template LEVEL and ORIG_LEVEL 
values.  When mangling a lambda, we check if it's a generic lambda and 
then (after checking the ABI flag) perform this substitution.  We also 
set the needs_warning flag, if we detect the user's asked about ABI 
compatibility crossing version 11.

This use of tsubst can meet {TYPE,EXPR}_PARAMETER_PACKs, so I had to 
extend tsubst to process those objects.

lambda-mangle-1 is an augmented version of the testcase I included with 
78252.  It checks we retain the old mangling for ABI 10 and below.
lambda-mangle-2 is essentially the same testcase, but checks we use the 
new mangling for ABI=0
lambda-mangle-3 checks we issue a warning for -Wabi=10

I also included a demangler test for the new mangling.  It does demangle 
as expected.

Documentation about -fabi-version=11 also updated.

ok?

nathan
-- 
Nathan Sidwell

Comments

Jason Merrill Dec. 8, 2016, 7:08 p.m. UTC | #1
On 12/07/2016 01:30 PM, Nathan Sidwell wrote:
> This patch fixes the generic lambda mangling bug that caused the

> demangler to infinitely recurse (78252).  The generic's auto parms fail

> the is_auto check because that's testing for the 'auto' identifier,

> whereas the parms are distinct template type parms named 'auto:$N'.


Right.  Which is because they act as normal template parameters rather 
than like the auto type-specifier does in, say, a variable declaration. 
But indeed this produces bad mangling, and using Da in the mangling of a 
generic lambda seems reasonable.  You've probably seen my mail to the 
ABI list about it.  Are you still on that list?

> Amending is_auto to do a textual compare of the identifier breaks

> things, and is insufficient anyway.  The mangler's squangling must (a)

> squangle subsequent auto uses as expected and (b) NOT squangle

> subsequent template type parm references to refer the the lambda's autos.


Wow, I haven't seen the term "squangling" in more than a decade.  Did we 
ever use that in reference to the new ABI?

> This use of tsubst can meet {TYPE,EXPR}_PARAMETER_PACKs, so I had to

> extend tsubst to process those objects.


Hmm, we have deliberately avoided this in the past, since pack 
expansions mean different things in different contexts.  Can you use 
tsubst_arg_types instead?

Also, the name "make_auto_for_mangle" doesn't really indicate that it's 
doing a substitution of the function parameter types.

Jason
Nathan Sidwell Dec. 8, 2016, 7:23 p.m. UTC | #2
On 12/08/2016 02:08 PM, Jason Merrill wrote:

> Right.  Which is because they act as normal template parameters rather

> than like the auto type-specifier does in, say, a variable declaration.

> But indeed this produces bad mangling, and using Da in the mangling of a

> generic lambda seems reasonable.  You've probably seen my mail to the

> ABI list about it.  Are you still on that list?


Yes, thanks.

>> This use of tsubst can meet {TYPE,EXPR}_PARAMETER_PACKs, so I had to

>> extend tsubst to process those objects.

>

> Hmm, we have deliberately avoided this in the past, since pack

> expansions mean different things in different contexts.  Can you use

> tsubst_arg_types instead?


... will investigate.

> Also, the name "make_auto_for_mangle" doesn't really indicate that it's

> doing a substitution of the function parameter types.


yeah, probably not the best name anymore. (it started out as just 
creating the single auto type, but then I moved all the other stuff into 
it.)

nathan
-- 
Nathan Sidwell
diff mbox

Patch

2016-12-07  Nathan Sidwell  <nathan@acm.org>

	gcc/
	PR c++/78621
	* doc/invoke.texi (-fabi-version): Document generic lambda
	mangling for version 11.

	gcc/cp/
	PR c++/78621
	* cp-tree.h (make_auto_for_mangle): Declare.
	* mangle.c (write_closure_type_name): Handle generic lambdas
	correctly.
	* pt.c (tsubst): Add {TYPE,EXPR}_PACK_EXPANSION handling.
	(make_auto_raw): Break out of ...
	(make_auto_1): ... here.  Call it.
	(make_auto_for_mangle): New.

	gcc/testsuite/
	PR c++/78621
	* g++.dg/cpp1y/lambda-mangle-1.C: New.
	* g++.dg/cpp1y/lambda-mangle-2.C: New.
	* g++.dg/cpp1y/lambda-mangle-3.C: New.

	libiberty/
	PR c++/78621
	* testsuite/demangle-expected: Add fixed-ABI generic lambda tests.

Index: gcc/cp/cp-tree.h
===================================================================
--- gcc/cp/cp-tree.h	(revision 243371)
+++ gcc/cp/cp-tree.h	(working copy)
@@ -6108,6 +6108,7 @@ 
 extern void check_template_variable		(tree);
 extern tree make_auto				(void);
 extern tree make_decltype_auto			(void);
+extern tree make_auto_for_mangle		(tree, tree);
 extern tree make_template_placeholder		(tree);
 extern tree do_auto_deduction                   (tree, tree, tree);
 extern tree do_auto_deduction                   (tree, tree, tree,
Index: gcc/cp/mangle.c
===================================================================
--- gcc/cp/mangle.c	(revision 243371)
+++ gcc/cp/mangle.c	(working copy)
@@ -1640,6 +1640,19 @@ 
   tree lambda = CLASSTYPE_LAMBDA_EXPR (type);
   tree parms = TYPE_ARG_TYPES (TREE_TYPE (fn));
 
+  if (generic_lambda_fn_p (fn))
+    {
+      /* A generic lambda's auto parms are represented as distinct
+         TEMPLATE_TYPE_PARMs, which are both distinct and not is_auto.
+         But we must mangle (& squangle) them as (the same) 'auto'.
+         So do some substitution to arrange that.  */
+      if (abi_warn_or_compat_version_crosses (11))
+	G.need_abi_warning = 1;
+
+      if (abi_version_at_least (11))
+	parms = make_auto_for_mangle (fn, parms);
+    }
+
   MANGLE_TRACE_TREE ("closure-type-name", type);
 
   write_string ("Ul");
Index: gcc/cp/pt.c
===================================================================
--- gcc/cp/pt.c	(revision 243371)
+++ gcc/cp/pt.c	(working copy)
@@ -13813,6 +13813,42 @@ 
 	return r;
       }
 
+    case TYPE_PACK_EXPANSION:
+    case EXPR_PACK_EXPANSION:
+      {
+	tree r;
+
+	if (code == EXPR_PACK_EXPANSION)
+	  {
+	    r = make_node (code);
+	    TREE_TYPE (r) = type;
+	    TREE_CONSTANT (r) = TREE_CONSTANT (t);
+	    PACK_EXPANSION_SIZEOF_P (r) = PACK_EXPANSION_SIZEOF_P (t);
+	  }
+	else
+	  {
+	    r = cxx_make_type (code);
+	    SET_TYPE_STRUCTURAL_EQUALITY (r);
+	  }
+
+	WILDCARD_PACK_P (r) = WILDCARD_PACK_P (t);
+	PACK_EXPANSION_LOCAL_P (r) = PACK_EXPANSION_LOCAL_P (t);
+
+	tree pattern = PACK_EXPANSION_PATTERN (t);
+	pattern = tsubst (pattern, args, complain, in_decl);
+	SET_PACK_EXPANSION_PATTERN (r, pattern);
+
+	tree packs = PACK_EXPANSION_PARAMETER_PACKS (t);
+	packs = tsubst (packs, args, complain, in_decl);
+	PACK_EXPANSION_PARAMETER_PACKS (r) = packs;
+
+	tree extra = PACK_EXPANSION_EXTRA_ARGS (t);
+	extra = tsubst (extra, args, complain, in_decl);
+	PACK_EXPANSION_EXTRA_ARGS (r) = extra;
+
+	return r;
+      }
+
     case VOID_CST:
     case INTEGER_CST:
     case REAL_CST:
@@ -24278,20 +24314,19 @@ 
     }
 }
 
-/* Returns a type which represents 'auto' or 'decltype(auto)'.  We use a
-   TEMPLATE_TYPE_PARM with a level one deeper than the actual template
-   parms.  If set_canonical is true, we set TYPE_CANONICAL on it.  */
+/* Returns a template type parm to represent 'auto' or
+   'decltype(auto)'.  If SET_CANONICAL is true, we set TYPE_CANONICAL
+   on it.  set the parm's LEVEL and ORIG_LEVEL as specified.  */
 
 static tree
-make_auto_1 (tree name, bool set_canonical)
+make_auto_raw (tree name, bool set_canonical, int level, int orig_level)
 {
   tree au = cxx_make_type (TEMPLATE_TYPE_PARM);
   TYPE_NAME (au) = build_decl (input_location,
 			       TYPE_DECL, name, au);
   TYPE_STUB_DECL (au) = TYPE_NAME (au);
   TEMPLATE_TYPE_PARM_INDEX (au) = build_template_parm_index
-    (0, processing_template_decl + 1, processing_template_decl + 1,
-     TYPE_NAME (au), NULL_TREE);
+    (0, level, orig_level, TYPE_NAME (au), NULL_TREE);
   if (set_canonical)
     TYPE_CANONICAL (au) = canonical_type_parameter (au);
   DECL_ARTIFICIAL (TYPE_NAME (au)) = 1;
@@ -24300,6 +24335,18 @@ 
   return au;
 }
 
+/* Returns a type to represent 'auto' or 'decltype(auto)'.  We use a
+   TEMPLATE_TYPE_PARM with a level one deeper than the actual template
+   parms.  If set_canonical is true, we set TYPE_CANONICAL on it.  */
+
+static tree
+make_auto_1 (tree name, bool set_canonical)
+{
+  return make_auto_raw (name, set_canonical,
+			processing_template_decl + 1,
+			processing_template_decl + 1);
+}
+
 tree
 make_decltype_auto (void)
 {
@@ -24352,6 +24399,25 @@ 
   return decl;
 }
 
+/* FN a generic lambda function and PARMS is its parameters.  Perform
+   appropriate substitution turning the distinct template type parms
+   into a regular auto template type parm suitable for mangler
+   use.  */
+
+tree
+make_auto_for_mangle (tree fn, tree parms)
+{
+  tree ti_args = TI_ARGS (DECL_TEMPLATE_INFO (fn));
+  unsigned ix = NUM_TMPL_ARGS (INNERMOST_TEMPLATE_ARGS (ti_args));
+  tree args = make_tree_vec (ix);
+  tree auto_parm = make_auto_raw (auto_identifier, true,
+				  1, TMPL_ARGS_DEPTH (ti_args));
+  while (ix--)
+    TREE_VEC_ELT (args, ix) = auto_parm;
+
+  return tsubst (parms, args, tf_none, NULL_TREE);
+}
+
 /* Given type ARG, return std::initializer_list<ARG>.  */
 
 static tree
Index: gcc/doc/invoke.texi
===================================================================
--- gcc/doc/invoke.texi	(revision 243371)
+++ gcc/doc/invoke.texi	(working copy)
@@ -2250,7 +2250,7 @@ 
 attributes (e.g. @samp{stdcall}).
 
 Version 11, which first appeared in G++ 7, corrects the mangling of
-sizeof... expressions.  It also implies
+sizeof... expressions and generic lambdas.  It also implies
 @option{-fnew-inheriting-ctors}.
 
 See also @option{-Wabi}.
Index: gcc/testsuite/g++.dg/cpp1y/lambda-mangle-1.C
===================================================================
--- gcc/testsuite/g++.dg/cpp1y/lambda-mangle-1.C	(revision 0)
+++ gcc/testsuite/g++.dg/cpp1y/lambda-mangle-1.C	(working copy)
@@ -0,0 +1,81 @@ 
+// { dg-do compile { target c++14 } }
+// { dg-additional-options "-fabi-version=10" }
+
+// PRs 78252, 78621
+
+// We erroneously mangle lambda auto parms as-if template parameters (T<n>_),
+// rather than auto (Da).  While that's unfortunate, it'd be best if
+// we didn't accidentally change that.
+
+template<typename T> class X;
+
+template<typename T>
+T &&forward (T &v)
+{
+  return static_cast<T &&> (v);
+}
+
+template<typename T>
+void eat (T &v)
+{
+}
+
+template<typename S, typename T>
+  void eat (S &, T &v)
+{
+}
+
+void Foo ()
+{
+  auto lam = [](auto &) { };
+  auto lam_1 = [](int &, auto &) { };
+  auto lam_2 = [](auto &, X<int> &) { };
+  auto lam_3 = [](auto (*)[5]) { };
+
+  forward (lam);
+  forward (lam_1);
+  forward (lam_2);
+  forward (lam_3);
+
+  eat (lam);
+  eat (lam_1);
+  eat (lam_2);
+  eat (lam_3);
+
+  auto lambda_1 = [](float *, float *) { };
+  auto lambda_2 = [](auto *, auto *) { };
+
+  int *i;
+  
+  eat (i, lambda_1);
+  eat (i, lambda_2);
+}
+
+template<typename X> void Bar ()
+{
+  auto lambda_1 = [](X *, float *, float *) { };
+  auto lambda_2 = [](X *, auto *, auto *) { };
+
+  int *i;
+  
+  eat (i, lambda_1);
+  eat (i, lambda_2);
+}
+
+void Baz ()
+{
+  Bar<short> ();
+}
+
+// { dg-final { scan-assembler "_Z7forwardIZ3FoovEUlRT_E_EOS0_S1_:" } }
+// { dg-final { scan-assembler "_Z7forwardIZ3FoovEUlRiRT_E0_EOS1_S2_:" } }
+// { dg-final { scan-assembler "_Z7forwardIZ3FoovEUlRT_R1XIiEE1_EOS0_S1_:" } }
+// { dg-final { scan-assembler "_Z7forwardIZ3FoovEUlPA5_T_E2_EOS0_RS0_:" } }
+// { dg-final { scan-assembler "_Z3eatIZ3FoovEUlRT_E_EvS1_:" } }
+// { dg-final { scan-assembler "_Z3eatIZ3FoovEUlRiRT_E0_EvS2_:" } }
+// { dg-final { scan-assembler "_Z3eatIZ3FoovEUlRT_R1XIiEE1_EvS1_:" } }
+// { dg-final { scan-assembler "_Z3eatIZ3FoovEUlPA5_T_E2_EvRS0_:" } }
+// { dg-final { scan-assembler "_Z3eatIPiZ3FoovEUlPfS1_E3_EvRT_RT0_:" } }
+// { dg-final { scan-assembler "_Z3eatIPiZ3FoovEUlPT_PT0_E4_EvRS1_RS3_:" } }
+// { dg-final { scan-assembler "_Z3eatIPiZ3BarIsEvvEUlPsPfS3_E_EvRT_RT0_:" } }
+// { dg-final { scan-assembler "_Z3eatIPiZ3BarIsEvvEUlPsPT_PT0_E0_EvRS3_RS5_:" } }
Index: gcc/testsuite/g++.dg/cpp1y/lambda-mangle-2.C
===================================================================
--- gcc/testsuite/g++.dg/cpp1y/lambda-mangle-2.C	(revision 0)
+++ gcc/testsuite/g++.dg/cpp1y/lambda-mangle-2.C	(working copy)
@@ -0,0 +1,88 @@ 
+// { dg-do compile { target c++14 } }
+
+// PRs 78621
+
+// We erroneously mangled lambda auto parms as-if template parameters (T<n>_),
+// rather than auto (Da). Fixed in abi version 11
+
+template<typename T> class X;
+
+template<typename T>
+T &&forward (T &v)
+{
+  return static_cast<T &&> (v);
+}
+
+template<typename T>
+void eat (T &v)
+{
+}
+
+template<typename S, typename T>
+  void eat (S &, T &v)
+{
+}
+
+void Foo ()
+{
+  auto lam = [](auto &) { };
+  auto lam_1 = [](int &, auto &) { };
+  auto lam_2 = [](auto &, X<int> &) { };
+  auto lam_3 = [](auto (*)[5]) { };
+
+  forward (lam);
+  forward (lam_1);
+  forward (lam_2);
+  forward (lam_3);
+
+  eat (lam);
+  eat (lam_1);
+  eat (lam_2);
+  eat (lam_3);
+
+  // The auto lambda should mangle similarly to the non-auto one
+  auto lambda_1 = [](float *, float *) { };
+  auto lambda_2 = [](auto *, auto *) { };
+  auto lambda_3 = [](auto *, auto *) { };
+
+  int *i;
+  
+  eat (i, lambda_1);
+  eat (i, lambda_2);
+
+  // The autos should squangle to the first one.
+  eat (lambda_2, lambda_3);
+}
+
+template<typename X> void Bar ()
+{
+  auto lambda_1 = [](X *, float *, float *) { };
+  auto lambda_2 = [](X *, auto *, auto *) { };
+  auto lambda_3 = [](X *, auto *...) {};
+  
+  int *i;
+  
+  eat (i, lambda_1);
+  eat (i, lambda_2);
+  eat (i, lambda_3);
+}
+
+void Baz ()
+{
+  Bar<short> ();
+}
+
+// { dg-final { scan-assembler "_Z7forwardIZ3FoovEUlRDaE_EOT_RS2_:" } }
+// { dg-final { scan-assembler "_Z7forwardIZ3FoovEUlRiRDaE0_EOT_RS3_:" } }
+// { dg-final { scan-assembler "_Z7forwardIZ3FoovEUlRDaR1XIiEE1_EOT_RS5_:" } }
+// { dg-final { scan-assembler "_Z7forwardIZ3FoovEUlPA5_DaE2_EOT_RS3_:" } }
+// { dg-final { scan-assembler "_Z3eatIZ3FoovEUlRDaE_EvRT_:" } }
+// { dg-final { scan-assembler "_Z3eatIZ3FoovEUlRiRDaE0_EvRT_:" } }
+// { dg-final { scan-assembler "_Z3eatIZ3FoovEUlRDaR1XIiEE1_EvRT_:" } }
+// { dg-final { scan-assembler "_Z3eatIZ3FoovEUlPA5_DaE2_EvRT_:" } }
+// { dg-final { scan-assembler "_Z3eatIPiZ3FoovEUlPfS1_E3_EvRT_RT0_:" } }
+// { dg-final { scan-assembler "_Z3eatIPiZ3FoovEUlPDaS1_E4_EvRT_RT0_:" } }
+// { dg-final { scan-assembler "_Z3eatIZ3FoovEUlPDaS0_E4_Z3FoovEUlS0_S0_E5_EvRT_RT0_:" } }
+// { dg-final { scan-assembler "_Z3eatIPiZ3BarIsEvvEUlPsPfS3_E_EvRT_RT0_:" } }
+// { dg-final { scan-assembler "_Z3eatIPiZ3BarIsEvvEUlPsPDaS3_E0_EvRT_RT0_:" } }
+// { dg-final { scan-assembler "_Z3eatIPiZ3BarIsEvvEUlPsPDazE1_EvRT_RT0_:" } }
Index: gcc/testsuite/g++.dg/cpp1y/lambda-mangle-3.C
===================================================================
--- gcc/testsuite/g++.dg/cpp1y/lambda-mangle-3.C	(revision 0)
+++ gcc/testsuite/g++.dg/cpp1y/lambda-mangle-3.C	(working copy)
@@ -0,0 +1,18 @@ 
+// { dg-do compile { target c++14 } }
+// { dg-additional-options "-Wabi=10" }
+
+// PRs 78621
+
+// check we warn about changed mangling of generic lambdas.
+
+template<typename T>
+void eat (T &v) // { dg-warning "mangled name" }
+{
+}
+
+void Foo ()
+{
+  auto lam = [](auto &) { };
+
+  eat (lam);
+}
Index: libiberty/testsuite/demangle-expected
===================================================================
--- libiberty/testsuite/demangle-expected	(revision 243371)
+++ libiberty/testsuite/demangle-expected	(working copy)
@@ -4634,3 +4634,11 @@ 
 # ?: expression with missing third component could crash.
 AquT_quT_4mxautouT_4mxxx
 AquT_quT_4mxautouT_4mxxx
+
+# pr78621 We were incorrectly mangling generic lambdas.  Check the fix
+# for that demangles as expected
+_Z3eatIPiZ3FoovEUlPDaS1_E4_EvRT_RT0_
+void eat<int*, Foo()::{lambda(auto*, auto*)#6}>(int*&, Foo()::{lambda(auto*, auto*)#6}&)
+
+_Z3eatIPiZ3BarIsEvvEUlPsPDaS3_E0_EvRT_RT0_
+void eat<int*, void Bar<short>()::{lambda(short*, auto*, auto*)#2}>(int*&, void Bar<short>()::{lambda(short*, auto*, auto*)#2}&)