From 2370bdbb0b29b14401d8508d846c0e01c64d82fc Mon Sep 17 00:00:00 2001 From: Iain Buclaw Date: Sun, 26 Apr 2020 23:39:32 +0200 Subject: [PATCH] d: Fix ICE in assign_temp, at function.c:984 (PR94777) Named arguments were being passed around by invisible reference, just not variadic arguments. There is a need to de-duplicate the routines that handle declaration/parameter promotion and reference checking. However for now, the parameter helper functions have just been renamed to parameter_reference_p and parameter_type, to make it more clear that it is the Parameter equivalent to declaration_reference_p and declaration_type. On writing the tests, a forward-reference bug was discovered on x86_64 during va_list type semantic. This was due to fields not having their parent set-up correctly. gcc/d/ChangeLog: PR d/94777 * d-builtins.cc (build_frontend_type): Set parent for generated fields of built-in types. * d-codegen.cc (argument_reference_p): Rename to ... (parameter_reference_p): ... this. (type_passed_as): Rename to ... (parameter_type): ... this. Make TREE_ADDRESSABLE types restrict. (d_build_call): Move handling of non-POD types here from ... * d-convert.cc (convert_for_argument): ... here. * d-tree.h (argument_reference_p): Rename declaration to ... (parameter_reference_p): ... this. (type_passed_as): Rename declaration to ... (parameter_type): ... this. * types.cc (TypeVisitor::visit (TypeFunction *)): Update caller. gcc/testsuite/ChangeLog: PR d/94777 * gdc.dg/pr94777a.d: New test. * gdc.dg/pr94777b.d: New test. --- gcc/d/ChangeLog | 17 +++ gcc/d/d-builtins.cc | 1 + gcc/d/d-codegen.cc | 32 +++++- gcc/d/d-convert.cc | 19 +--- gcc/d/d-tree.h | 4 +- gcc/d/types.cc | 2 +- gcc/testsuite/ChangeLog | 6 ++ gcc/testsuite/gdc.dg/pr94777a.d | 15 +++ gcc/testsuite/gdc.dg/pr94777b.d | 181 ++++++++++++++++++++++++++++++++ 9 files changed, 254 insertions(+), 23 deletions(-) create mode 100644 gcc/testsuite/gdc.dg/pr94777a.d create mode 100644 gcc/testsuite/gdc.dg/pr94777b.d diff --git a/gcc/d/ChangeLog b/gcc/d/ChangeLog index ab3028d8d75..3b5fc12a8fd 100644 --- a/gcc/d/ChangeLog +++ b/gcc/d/ChangeLog @@ -1,3 +1,20 @@ +2020-04-27 Iain Buclaw + + PR d/94777 + * d-builtins.cc (build_frontend_type): Set parent for generated + fields of built-in types. + * d-codegen.cc (argument_reference_p): Rename to ... + (parameter_reference_p): ... this. + (type_passed_as): Rename to ... + (parameter_type): ... this. Make TREE_ADDRESSABLE types restrict. + (d_build_call): Move handling of non-POD types here from ... + * d-convert.cc (convert_for_argument): ... here. + * d-tree.h (argument_reference_p): Rename declaration to ... + (parameter_reference_p): ... this. + (type_passed_as): Rename declaration to ... + (parameter_type): ... this. + * types.cc (TypeVisitor::visit (TypeFunction *)): Update caller. + 2020-04-26 Iain Buclaw * decl.cc (get_symbol_decl): Set DECL_DECLARED_INLINE_P or diff --git a/gcc/d/d-builtins.cc b/gcc/d/d-builtins.cc index cb8f43a88e5..a5654a66bf5 100644 --- a/gcc/d/d-builtins.cc +++ b/gcc/d/d-builtins.cc @@ -253,6 +253,7 @@ build_frontend_type (tree type) = Identifier::idPool (IDENTIFIER_POINTER (DECL_NAME (field))); VarDeclaration *vd = VarDeclaration::create (Loc (), ftype, fident, NULL); + vd->parent = sdecl; vd->offset = tree_to_uhwi (DECL_FIELD_OFFSET (field)); vd->semanticRun = PASSsemanticdone; vd->csym = field; diff --git a/gcc/d/d-codegen.cc b/gcc/d/d-codegen.cc index 8dc1ab264f8..12c6f138362 100644 --- a/gcc/d/d-codegen.cc +++ b/gcc/d/d-codegen.cc @@ -172,7 +172,7 @@ declaration_type (Declaration *decl) Return TRUE if parameter ARG is a reference type. */ bool -argument_reference_p (Parameter *arg) +parameter_reference_p (Parameter *arg) { Type *tb = arg->type->toBasetype (); @@ -186,7 +186,7 @@ argument_reference_p (Parameter *arg) /* Returns the real type for parameter ARG. */ tree -type_passed_as (Parameter *arg) +parameter_type (Parameter *arg) { /* Lazy parameters are converted to delegates. */ if (arg->storageClass & STClazy) @@ -207,9 +207,18 @@ type_passed_as (Parameter *arg) tree type = build_ctype (arg->type); /* Parameter is passed by reference. */ - if (TREE_ADDRESSABLE (type) || argument_reference_p (arg)) + if (parameter_reference_p (arg)) return build_reference_type (type); + /* Pass non-POD structs by invisible reference. */ + if (TREE_ADDRESSABLE (type)) + { + type = build_reference_type (type); + /* There are no other pointer to this temporary. */ + type = build_qualified_type (type, TYPE_QUAL_RESTRICT); + } + + /* Front-end has already taken care of type promotions. */ return type; } @@ -1954,6 +1963,23 @@ d_build_call (TypeFunction *tf, tree callable, tree object, targ = build2 (COMPOUND_EXPR, TREE_TYPE (t), targ, t); } + /* Parameter is a struct passed by invisible reference. */ + if (TREE_ADDRESSABLE (TREE_TYPE (targ))) + { + Type *t = arg->type->toBasetype (); + gcc_assert (t->ty == Tstruct); + StructDeclaration *sd = ((TypeStruct *) t)->sym; + + /* Nested structs also have ADDRESSABLE set, but if the type has + neither a copy constructor nor a destructor available, then we + need to take care of copying its value before passing it. */ + if (arg->op == TOKstructliteral || (!sd->postblit && !sd->dtor)) + targ = force_target_expr (targ); + + targ = convert (build_reference_type (TREE_TYPE (targ)), + build_address (targ)); + } + vec_safe_push (args, targ); } } diff --git a/gcc/d/d-convert.cc b/gcc/d/d-convert.cc index 9ee149b8386..e2921ec33f0 100644 --- a/gcc/d/d-convert.cc +++ b/gcc/d/d-convert.cc @@ -672,25 +672,10 @@ convert_for_argument (tree expr, Parameter *arg) if (!POINTER_TYPE_P (TREE_TYPE (expr))) return build_address (expr); } - else if (argument_reference_p (arg)) + else if (parameter_reference_p (arg)) { /* Front-end shouldn't automatically take the address. */ - return convert (type_passed_as (arg), build_address (expr)); - } - else if (TREE_ADDRESSABLE (TREE_TYPE (expr))) - { - /* Type is a struct passed by invisible reference. */ - Type *t = arg->type->toBasetype (); - gcc_assert (t->ty == Tstruct); - StructDeclaration *sd = ((TypeStruct *) t)->sym; - - /* Nested structs also have ADDRESSABLE set, but if the type has - neither a copy constructor nor a destructor available, then we - need to take care of copying its value before passing it. */ - if (!sd->postblit && !sd->dtor) - expr = force_target_expr (expr); - - return convert (type_passed_as (arg), build_address (expr)); + return convert (parameter_type (arg), build_address (expr)); } return expr; diff --git a/gcc/d/d-tree.h b/gcc/d/d-tree.h index 89feb9e7010..48587d96e38 100644 --- a/gcc/d/d-tree.h +++ b/gcc/d/d-tree.h @@ -504,8 +504,8 @@ extern tree d_decl_context (Dsymbol *); extern tree copy_aggregate_type (tree); extern bool declaration_reference_p (Declaration *); extern tree declaration_type (Declaration *); -extern bool argument_reference_p (Parameter *); -extern tree type_passed_as (Parameter *); +extern bool parameter_reference_p (Parameter *); +extern tree parameter_type (Parameter *); extern tree build_integer_cst (dinteger_t, tree = d_int_type); extern tree build_float_cst (const real_t &, Type *); extern tree d_array_length (tree); diff --git a/gcc/d/types.cc b/gcc/d/types.cc index f6ae5740f01..59a90b49243 100644 --- a/gcc/d/types.cc +++ b/gcc/d/types.cc @@ -720,7 +720,7 @@ public: for (size_t i = 0; i < n_args; i++) { - tree type = type_passed_as (Parameter::getNth (t->parameters, i)); + tree type = parameter_type (Parameter::getNth (t->parameters, i)); fnparams = chainon (fnparams, build_tree_list (0, type)); } } diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog index 07fe8a68598..1f412a0ef1d 100644 --- a/gcc/testsuite/ChangeLog +++ b/gcc/testsuite/ChangeLog @@ -1,3 +1,9 @@ +2020-04-27 Iain Buclaw + + PR d/94777 + * gdc.dg/pr94777a.d: New test. + * gdc.dg/pr94777b.d: New test. + 2020-04-26 Iain Sandoe PR c++/94752 diff --git a/gcc/testsuite/gdc.dg/pr94777a.d b/gcc/testsuite/gdc.dg/pr94777a.d new file mode 100644 index 00000000000..a58fa557e35 --- /dev/null +++ b/gcc/testsuite/gdc.dg/pr94777a.d @@ -0,0 +1,15 @@ +// https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94777 +// { dg-do compile } + +extern void variadic(...); + +void f94777() +{ + struct S94777 + { + int fld; + this(this) { } + } + auto var = S94777(0); + variadic(var, S94777(1)); +} diff --git a/gcc/testsuite/gdc.dg/pr94777b.d b/gcc/testsuite/gdc.dg/pr94777b.d new file mode 100644 index 00000000000..a230adb0cd9 --- /dev/null +++ b/gcc/testsuite/gdc.dg/pr94777b.d @@ -0,0 +1,181 @@ +// https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94777 +// { dg-additional-options "-fmain -funittest" } +// { dg-do run { target hw } } +// { dg-skip-if "needs gcc/config.d" { ! d_runtime } } + +void testVariadic(T)(int nargs, ...) +{ + import core.stdc.stdarg; + foreach(i; 0 .. nargs) + { + auto arg = va_arg!T(_argptr); + static if (__traits(compiles, arg.value)) + { + assert(arg.value == i); + } + else static if (__traits(compiles, arg[0])) + { + foreach (value; arg) + assert(value == i); + } + else + { + assert(arg == T.init); + } + } +} + +/******************************************/ + +struct Constructor +{ + static int count; + int value; + this(int v) { count++; this.value = v; } +} + +unittest +{ + auto a0 = Constructor(0); + auto a1 = Constructor(1); + auto a2 = Constructor(2); + testVariadic!Constructor(3, a0, a1, a2); + assert(Constructor.count == 3); +} + +/******************************************/ + +struct Postblit +{ + static int count = 0; + int value; + this(this) { count++; } +} + +unittest +{ + auto a0 = Postblit(0); + auto a1 = Postblit(1); + auto a2 = Postblit(2); + testVariadic!Postblit(3, a0, a1, a2); + assert(Postblit.count == 3); +} + +/******************************************/ + +struct Destructor +{ + static int count = 0; + int value; + ~this() { count++; } +} + +unittest +{ + { + auto a0 = Destructor(0); + auto a1 = Destructor(1); + auto a2 = Destructor(2); + static assert(!__traits(compiles, testVariadic!Destructor(3, a0, a1, a2))); + } + assert(Destructor.count == 3); +} + +/******************************************/ + +struct CopyConstructor +{ + static int count = 0; + int value; + this(int v) { this.value = v; } + this(ref typeof(this) other) { count++; this.value = other.value; } +} + +unittest +{ + auto a0 = CopyConstructor(0); + auto a1 = CopyConstructor(1); + auto a2 = CopyConstructor(2); + testVariadic!CopyConstructor(3, a0, a1, a2); + // NOTE: Cpctors are not implemented yet. + assert(CopyConstructor.count == 0 || CopyConstructor.count == 3); +} + +/******************************************/ + +unittest +{ + struct Nested + { + int value; + } + + auto a0 = Nested(0); + auto a1 = Nested(1); + auto a2 = Nested(2); + testVariadic!Nested(3, a0, a1, a2); +} + +/******************************************/ + +unittest +{ + struct Nested2 + { + int value; + } + + void testVariadic2(int nargs, ...) + { + import core.stdc.stdarg; + foreach(i; 0 .. nargs) + { + auto arg = va_arg!Nested2(_argptr); + assert(arg.value == i); + } + } + + auto a0 = Nested2(0); + auto a1 = Nested2(1); + auto a2 = Nested2(2); + testVariadic2(3, a0, a1, a2); +} + +/******************************************/ + +struct EmptyStruct +{ +} + +unittest +{ + auto a0 = EmptyStruct(); + auto a1 = EmptyStruct(); + auto a2 = EmptyStruct(); + testVariadic!EmptyStruct(3, a0, a1, a2); +} + +/******************************************/ + +alias StaticArray = int[4]; + +unittest +{ + StaticArray a0 = 0; + StaticArray a1 = 1; + StaticArray a2 = 2; + // XBUG: Front-end rewrites static arrays as dynamic arrays. + //testVariadic!StaticArray(3, a0, a1, a2); +} + +/******************************************/ + +alias EmptyArray = void[0]; + +unittest +{ + auto a0 = EmptyArray.init; + auto a1 = EmptyArray.init; + auto a2 = EmptyArray.init; + testVariadic!EmptyArray(3, a0, a1, a2); +}