From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from esa2.mentor.iphmx.com (esa2.mentor.iphmx.com [68.232.141.98]) by sourceware.org (Postfix) with ESMTPS id 6E8983857C48; Tue, 28 Nov 2023 14:36:43 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 6E8983857C48 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=codesourcery.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=mentor.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 6E8983857C48 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=68.232.141.98 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1701182205; cv=none; b=Kyw9fE4h7LijabHFgvn8A+wsuEMD5L6v8SNupzee+pL76vRS1lNFVFJCPavA3ZR53Esb7AInzIxWZI2A8vfosflnCBlFzj9cyllSMtdKt6Qy/MAu5WbF4AIByFYpipYOhj2/HDBwPqMrgA4qdymCdM/viRz+sR5a/6HWkosL0zQ= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1701182205; c=relaxed/simple; bh=H14/Ky8RIaMRcKDJgPw/28fih3caN6vC9j+fv7R0azk=; h=Message-ID:Date:MIME-Version:Subject:To:From; b=McbxrP4S8nn5ctsDeNQXaJX+9A6bhZBKO5bd2LR2W9dVVtU4zYairQfoEPUehfEO1mvJAvwgygLBDMF6zoQgstuWffgCoapTcmNaFJjC7MokgsUOa+vv6hWcCgi9gna58Zkgp7IEMv5rs3JJ8HNAxIiO8/CET5555lvFVqsRcuA= ARC-Authentication-Results: i=1; server2.sourceware.org X-CSE-ConnectionGUID: Resmn2HwQlibQp+QtSCQ6w== X-CSE-MsgGUID: Xdb5itnjQ1mzR4tH108Zjw== X-IronPort-AV: E=Sophos;i="6.04,234,1695715200"; d="scan'208";a="26767852" Received: from orw-gwy-01-in.mentorg.com ([192.94.38.165]) by esa2.mentor.iphmx.com with ESMTP; 28 Nov 2023 06:36:42 -0800 IronPort-SDR: Pky1WDeOtyvi78xkE2w7dtMzF/WERLNkE7gXGMFCFKWO+17fXGiqiP+QAW3RSAe4/kS4mH0kbm U32P+DeC+yO512snAWFeG2V8N8zCURGlq7AODFUiZLkBDSNg72PnamNN2g9YnVZfiPlPMhcgtM FrQ7rvEMIOq4kGmgGVN/znHxVhLiBZDkNce2GXRpt0LK6C8J5g/Aa51l7ie67ik7cVjaUgbzZg T2EWjaFd1e82bjx0NiTEjvtKRnqVF3UgcrSEGr2Bbou5AIy1dmi+hVv1XiBRp9oK0IR0VtAcc5 r5E= Message-ID: <8243a622-7afc-4bba-9d5c-8e39952366e2@codesourcery.com> Date: Tue, 28 Nov 2023 15:36:37 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v2] Fortran: fix reallocation on assignment of polymorphic variables [PR110415] Content-Language: en-US To: Andrew Jenner , gcc Patches , References: <4733a0ea-1a3e-4cf3-8b1e-3e1efac91dd0@codesourcery.com> From: Tobias Burnus In-Reply-To: <4733a0ea-1a3e-4cf3-8b1e-3e1efac91dd0@codesourcery.com> Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: quoted-printable X-Originating-IP: [137.202.0.90] X-ClientProxiedBy: svr-ies-mbx-12.mgc.mentorg.com (139.181.222.12) To svr-ies-mbx-12.mgc.mentorg.com (139.181.222.12) X-Spam-Status: No, score=-5.3 required=5.0 tests=BAYES_00,HEADER_FROM_DIFFERENT_DOMAINS,KAM_DMARC_STATUS,KAM_SHORT,SPF_HELO_PASS,SPF_PASS,TXREP,T_SCC_BODY_TEXT_LINE autolearn=no autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: Hi Andrew, On 27.11.23 18:35, Andrew Jenner wrote: > This is the second version of the patch - previous discussion at: > https://gcc.gnu.org/pipermail/gcc-patches/2023-November/636671.html > > This patch adds the testcase from PR110415 and fixes the bug. > > The problem is that in a couple of places in trans_class_assignment in > trans-expr.cc, we need to get the run-time size of the polymorphic > object from the vtbl, but we are currently getting that vtbl from the > lhs of the assignment rather than the rhs. This gives us the old value > of the size but we need to pass the new size to __builtin_malloc and > __builtin_realloc. > > I'm fixing this by adding a parameter to trans_class_vptr_len_assignment > to retrieve the tree corresponding the vptr from the object on the rhs > of the assignment, and then passing this where it is needed. In the case > where trans_class_vptr_len_assignment returns NULL_TREE for the rhs vptr > we use the lhs vptr as before. > > To get this to work I also needed to change the implementation of > trans_class_vptr_len_assignment to create a temporary for the assignment > in more circumstances. Currently, the "a =3D func()" assignment in MAIN__ > doesn't hit the "Create a temporary for complication expressions" case > on line 9951 because "DECL_P (rse->expr)" is true - the expression has > already been placed into a temporary. That means we don't hit the "if > (temp_rhs ..." case on line 10038 and go on to get the vptr_expr from > "gfc_lval_expr_from_sym (gfc_find_vtab (&re->ts))" on line 10057 which > is the vtbl of the static type rather than the dynamic one from the rhs. > So with this fix we create an extra temporary, but that should be > optimised away in the middle-end so there should be no run-time effect. > > I'm not sure if this is the best way to fix this (the Fortran front-end > is new territory for me) but I've verified that the testcase passes with > this change, fails without it, and that the change does not introduce > any FAILs when running the gfortran testcases on x86_64-pc-linux-gnu. > > After the previous submission, Tobias Burnus found a closely related > problem and contributed testcases and a fix for it, which I have > incorporated into this version of the patch. The problem in this case > is with the __builtin_realloc call that is executed if one polymorphic > variable is replaced by another. The return value of this call was > being ignored rather than used to replace the pointer being reallocated. > > Is this OK for mainline, GCC 13 and OG13? LGTM =E2=80=93 Thanks! OK for mainline and after some grace period for GCC = 13. OG13 does not require approval, hence, either cherry pick directly or wait until it is in GCC 13 and then "git merge" GCC 13 to OG13. Thanks, Tobias > gcc/fortran/ > PR fortran/110415 > * trans-expr.cc (trans_class_vptr_len_assignment): Add > from_vptrp parameter. Populate it. Don't check for DECL_P > when deciding whether to create temporary. > (trans_class_pointer_fcn, gfc_trans_pointer_assignment): Add > NULL argument to trans_class_vptr_len_assignment calls. > (trans_class_assignment): Get rhs_vptr from > trans_class_vptr_len_assignment and use it for determining size > for allocation/reallocation. Use return value from realloc. > > gcc/testsuite/ > PR fortran/110415 > * gfortran.dg/pr110415.f90: New test. > * gfortran.dg/asan/pr110415-2.f90: New test. > * gfortran.dg/asan/pr110415-3.f90: New test. ----------------- Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstra=C3=9Fe 201= , 80634 M=C3=BCnchen; Gesellschaft mit beschr=C3=A4nkter Haftung; Gesch=C3= =A4ftsf=C3=BChrer: Thomas Heurung, Frank Th=C3=BCrauf; Sitz der Gesellschaf= t: M=C3=BCnchen; Registergericht M=C3=BCnchen, HRB 106955