From patchwork Mon Apr 27 00:58:13 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Iain Buclaw X-Patchwork-Id: 1277273 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=gcc.gnu.org (client-ip=8.43.85.97; helo=sourceware.org; envelope-from=gcc-patches-bounces@gcc.gnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=gcc.gnu.org Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.a=rsa-sha256 header.s=default header.b=ObThUELg; dkim-atps=neutral Received: from sourceware.org (server2.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 499RGp11Xrz9sRf for ; Mon, 27 Apr 2020 10:58:26 +1000 (AEST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 08543383F850; Mon, 27 Apr 2020 00:58:24 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 08543383F850 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1587949104; bh=V3JQmjVWavqTcszuSNI6cFL5NB+vPnZOUhniYSa9+gs=; h=To:Subject:Date:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:From; b=ObThUELgVQQn/+JV+Vc6xhImd+tSj0Jdvai0i89M2jKYT6LkjbARHMrxF51+0LyY8 QmMtaEcEmxGZsuTQrMbGmGtfmRrE82alO5bBKsgIAS4CjjxC73Vc51RcWsNST5nwuE MpL8aVRLOtWbDqpfolrU1Wj1zjQk68TbbUwdjKNE= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from mout-p-202.mailbox.org (mout-p-202.mailbox.org [80.241.56.172]) by sourceware.org (Postfix) with ESMTPS id 42550385DC1B for ; Mon, 27 Apr 2020 00:58:20 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 42550385DC1B Received: from smtp1.mailbox.org (smtp1.mailbox.org [IPv6:2001:67c:2050:105:465:1:1:0]) (using TLSv1.2 with cipher ECDHE-RSA-CHACHA20-POLY1305 (256/256 bits)) (No client certificate requested) by mout-p-202.mailbox.org (Postfix) with ESMTPS id 499RGg1klKzQlKB; Mon, 27 Apr 2020 02:58:19 +0200 (CEST) X-Virus-Scanned: amavisd-new at heinlein-support.de Received: from smtp1.mailbox.org ([80.241.60.240]) by spamfilter06.heinlein-hosting.de (spamfilter06.heinlein-hosting.de [80.241.56.125]) (amavisd-new, port 10030) with ESMTP id Zt8yjvgn3lyI; Mon, 27 Apr 2020 02:58:15 +0200 (CEST) To: gcc-patches@gcc.gnu.org Subject: [committed] d: Fix ICE in assign_temp, at function.c:984 (PR94777) Date: Mon, 27 Apr 2020 02:58:13 +0200 Message-Id: <20200427005813.30508-1-ibuclaw@gdcproject.org> MIME-Version: 1.0 X-Rspamd-Queue-Id: BDDBE1776 X-Rspamd-Score: 0.29 / 15.00 / 15.00 X-Spam-Status: No, score=-27.0 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_SHORT, RCVD_IN_DNSWL_LOW, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Iain Buclaw via Gcc-patches From: Iain Buclaw Reply-To: Iain Buclaw Errors-To: gcc-patches-bounces@gcc.gnu.org Sender: "Gcc-patches" Hi, This patch fixes an ICE in the D front-end when passing non-trivially copyable types to variadic functions. 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. Bootstrapped and regression tested on x86_64-linux-gnu, with the --target_board configurations -m64, -m32, -mx32. Committed to mainline. Regards Iain --- 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/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/gdc.dg/pr94777a.d | 15 +++ gcc/testsuite/gdc.dg/pr94777b.d | 181 ++++++++++++++++++++++++++++++++ 7 files changed, 231 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/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/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); +}