From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mout.gmx.net (mout.gmx.net [212.227.15.15]) by sourceware.org (Postfix) with ESMTPS id 9CDBE3858427; Tue, 30 Apr 2024 19:34:36 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 9CDBE3858427 Authentication-Results: sourceware.org; dmarc=pass (p=quarantine dis=none) header.from=gmx.de Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmx.de ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 9CDBE3858427 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=212.227.15.15 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1714505679; cv=none; b=efKTicw9/9U9O+E1c39Svr+q1OeE6hv9eNp+ZlbupcqIfrg6AXnXkA1Sf0TODOHw15tEujFqxIaiBptqbESKCdFzxqWO+HIpHRQJNMeqWAc5JeDQ+nmFODVLMIb4fp5CZx0fGech6QxEnq0o0y6UgOuZEGYefmhjgTxs7oV2Sfk= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1714505679; c=relaxed/simple; bh=nzG2ZZBdiHfMa8b0Rgx6NK39xsLHbvHJsubZ+fHnWlk=; h=DKIM-Signature:Message-ID:Date:MIME-Version:Subject:To:From; b=vBc8sLLDy8TGeFyqj+OZKnwdLgH1xdoVMuZsVMvlbgDQkjWy6D10k127BOmVmuV/zOalJIl+PH9LdaEiBWYcBvmlq7MwfiM7fdSe+rCBd2sIXQw2vGx89naRPrhfbvpzB9Z4TtQY9hIawR6iYJ/+hqCn8blx/H2f4TfpThaOy50= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmx.de; s=s31663417; t=1714505674; x=1715110474; i=anlauf@gmx.de; bh=ubLJaIlptXN2Wu0sBo5V4X4PeI0efQrU+nqq3bde9GE=; h=X-UI-Sender-Class:Message-ID:Date:MIME-Version:Subject:To:Cc: References:From:In-Reply-To:Content-Type: Content-Transfer-Encoding:cc:content-transfer-encoding: content-type:date:from:message-id:mime-version:reply-to:subject: to; b=SdND0/ef97zzZ+VckNf6FASwpWE6LpWn1/Jw7GcE0CA3SlU/suGevG0uwyk7dk1U jCMTA1vxibdIgfXyabJmu21ZJqC4LZ2w0Z1zi0H8OAiBR44TOLBTBMr1Uw+kIQ5Go tCzEYSajZ1Z+O93Bhpg5oWHyNc0NNYfylA4cNyWJaKqjbpElg7FfzuLiPDX5onu9k p6EU4aNdXNuMP9lCTsEBF6HX7SwuM7GPSFdpz/GI9PW3g1JULzvhYTNFE9zTRZESp 1KUgYsHmzhAh7ZDm0McGwJAoJuG010pD1HAf5H+JqmpO/wkyO/j4zJHnt4Jj9yjRE K8IBxlf3HOhvmWJnMg== X-UI-Sender-Class: 724b4f7f-cbec-4199-ad4e-598c01a50d3a Received: from [192.168.178.29] ([79.232.144.193]) by mail.gmx.net (mrgmx005 [212.227.17.190]) with ESMTPSA (Nemesis) id 1MVeMA-1sBkbO1BDQ-00Rd3k; Tue, 30 Apr 2024 21:34:34 +0200 Message-ID: <4aac43ab-21f2-4bab-a564-dc470b6c2102@gmx.de> Date: Tue, 30 Apr 2024 21:34:32 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH] Fortran: fix issues with class(*) assignment [PR114827] To: Paul Richard Thomas Cc: fortran , gcc-patches Newsgroups: gmane.comp.gcc.patches,gmane.comp.gcc.fortran References: Content-Language: en-US From: Harald Anlauf In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: quoted-printable X-Provags-ID: V03:K1:N3qpzRBIdII7DDT0Pn8Ma86eiu9d7//SbMJRthCzvQFLGuHoexL VkXoxq2FAR8i1SAaTs/nmpZXZQ+WtH4ZW0Y7tjwlmwT5laW+opdtq0/u9S5PJVmtrruj86R kcNFbsk9q1CbexEPKRWnDo1EwK6/RzibR1cC0fdYQRSx+3MpOHWo80KN3fsN4vCjHOy6K35 WZ2bZ0G6IeE4C90nPs7Pg== UI-OutboundReport: notjunk:1;M01:P0:Pn9k+CQYkMs=;EVE4LddMmf/ZH2KhdD8ah6bQNyY Cu0MxMF5PPIyHIfojhP/qCRdKv2dzeHl+diKefMlE/CgaExKOO4NbM7K4JCO3EPjmtxmQ+pZ6 s2eJOLB8WsF7w8kBZplt74ghYG/VVIOR8Le1djzk8CacWMrNQIF7QttipRuVLRQHLJGCEJ9eY HMy0uBfpTkY30C+3A0BV/30yTBcvUCeItp/OP8H6KxZUFcSkVEYjydWDT1aTIL6NNGwuwQr3H 3FqGilPs6ej+dQc94hN9h2U/XGUAvof15etXT21LRZE79mwmSk9ntb8enrJyjbYkhIigjV91n K+zlT9NXj8x/PxVb7XsNoIgMuWMYD8JSZ77EzqRulVFV1LuNH15PF7PGlMVroGwpbD8FBKtEx Jayt1IFpzKMw8jgB6x64NJ5P6z4rHQipu/BODcrlUkeRhQlS2FOENlRBjA1n81dbELFiUavbg ZRbSvwP7c5HB0Hkro/YlKOLzgQC3klKQa5X1iKHJtlmqdQl03+A/yLQHXCMKHcsSppZv2jwl0 b8ByggTbTVYGgHTC12O548vOPGDAbZRVA6DGOUnxzgKIyiqot1oQTeBek+aIftUBQj7GKCzQO AlDkpthR1l0F9JXGFOdrlc5zGuxrGnYbwNk+/vRTbuiLaiMYOZE/5/sauuiXcVh4jFqpaV3a0 pZKrc0XNz/4oTb5mUmd5DZnqQ1ZzLJZ4SDnoNyPk9oLVm8U0hoGQixFGE0PwJdSIiqxRoLD0Q 5zNFhBUhV5LU8ULqFw/GoqBlhmugyJvLa/n1jz07H74Vq7qW6gaSblC9BaXzWwsEEYRwhPe31 Y6gyW3+6QIkypusEdhZA4THVToWJ1A2y4UCiXk4MUHQAg= X-Spam-Status: No, score=-12.3 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,GIT_PATCH_0,KAM_SHORT,RCVD_IN_DNSWL_LOW,RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_PASS,TXREP autolearn=ham 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 Paul, On 4/30/24 07:50, Paul Richard Thomas wrote: > Hi Harald, > > This patch is verging on 'obvious', ..... once one sees it :-) > > Yes, it's good for mainline and all active branches, when available. thanks for your quick review. I haven't committed it yet, because I forgot to check what happens with a class(*) allocatable function result on the r.h.s. of the assignment. One now gets an ICE with the testcase in your submission https://gcc.gnu.org/pipermail/fortran/2024-April/060426.html on the simple scalar assignment y =3D bar () instead of wrong code. Not very helpful. I tried the following change on top of the submitted patch: diff --git a/gcc/fortran/trans-expr.cc b/gcc/fortran/trans-expr.cc index 4ba40bfdbd3..cacf3c0dda1 100644 =2D-- a/gcc/fortran/trans-expr.cc +++ b/gcc/fortran/trans-expr.cc @@ -11995,7 +11996,11 @@ trans_class_assignment (stmtblock_t *block, gfc_expr *lhs, gfc_expr *rhs, /* Take into account _len of unlimited polymorphic entities. */ if (UNLIMITED_POLY (rhs)) { - tree len =3D trans_get_upoly_len (block, rhs); + tree len; + if (rhs->expr_type =3D=3D EXPR_VARIABLE) + len =3D trans_get_upoly_len (block, rhs); + else + len =3D gfc_class_len_get (gfc_get_class_from_expr (tmp)); len =3D fold_build2_loc (input_location, MAX_EXPR, size_type_node, fold_convert (size_type_node, len), size_one_node); This avoids the ICE, but depending on details of bar() this leads to different wrong code from before, and function bar() result(res) class(*), allocatable :: res res =3D sca end function bar behaves differently from function bar() class(*), allocatable :: bar bar =3D sca end function bar The minimal and sort of "safe" fix to avoid a new ICE while keeping the fix for simple assignments is to replace in the above snippet if (UNLIMITED_POLY (rhs)) by if (UNLIMITED_POLY (rhs) && rhs->expr_type =3D=3D EXPR_VARIABLE) omit the other changes above, and defer a fix for assignment of function results, as looking at the dump-tree suggests that this will be a bigger piece of work. (The .span looks suspicious all over the place...) The good thing is: a simple test with array-valued function results did not immediately break the submitted patch... ;-) What do you think? Thanks, Harald > Thanks > > Paul > > PS The fall-out pr114874 is so peculiar that I am dropping everything to > find the source. > > > On Mon, 29 Apr 2024 at 19:39, Harald Anlauf wrote: > >> Dear all, >> >> the attached patch fixes issues with assignments of unlimited polymorph= ic >> entities that were found with the help of valgrind or asan, see PR. >> Looking >> further into it, it turns out that allocation sizes as well as array sp= ans >> could be set incorrectly, leading to wrong results or heap corruption. >> >> The fix is rather straightforward: take into the _len of unlimited >> polymorphic entities when it is non-zero to get the correct allocation >> sizes and array spans. >> >> The patch has been tested by the reporter, see PR. >> >> Regtested on x86_64-pc-linux-gnu. OK for 15-mainline? >> >> I would like to backport this to active branches where appropriate, >> starting with 14 after it reopens after release. Is this OK? >> >> Thanks, >> Harald >> >> > From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from ciao.gmane.io (ciao.gmane.io [116.202.254.214]) by sourceware.org (Postfix) with ESMTPS id 151CE385843B for ; Tue, 30 Apr 2024 19:34:42 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 151CE385843B Authentication-Results: sourceware.org; dmarc=fail (p=quarantine dis=none) header.from=gmx.de Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=m.gmane-mx.org ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 151CE385843B Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=116.202.254.214 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1714505685; cv=none; b=DYL36cotSz3mjnEgtumV4XWBArfHyIamGDkRScn7eT0AoduYAs56oTSnVcufNC3uRV4A17aIbIdm+R2HcRXuN12t8t1+dTZLrh3qChxv5kgwqtxsn3mZ4wx+cKP8CkI7lsX4xDQIer4GHtVuoMn8yBiHx3ZhVzpGfX9hZRyDjuM= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1714505685; c=relaxed/simple; bh=0zRcgB8fkM/HcpLt4sRoJAFhoweQ/bNkdFdRqqCI1Js=; h=To:From:Subject:Date:Message-ID:Mime-Version; b=N+mlvTluks+TI8Pdm7DzCz+SFY0Ij/Ik5zIandblRvq7CxdOK/xXO+st8RtzvAOwM7P/CDCx0DIR20obmvv8tO7HEI4Dm2UR1p+ciM1i1A3qaqN83ksMXmUAF/SIbECjivA/o5OUIDtNjuHrdTERWKfGbh8w5pr4/Z0PYE0OR38= ARC-Authentication-Results: i=1; server2.sourceware.org Received: from list by ciao.gmane.io with local (Exim 4.92) (envelope-from ) id 1s1tFP-0004Pa-SZ for gcc-patches@gcc.gnu.org; Tue, 30 Apr 2024 21:34:39 +0200 X-Injected-Via-Gmane: http://gmane.org/ To: gcc-patches@gcc.gnu.org From: Harald Anlauf Subject: Re: [PATCH] Fortran: fix issues with class(*) assignment [PR114827] Date: Tue, 30 Apr 2024 21:34:32 +0200 Message-ID: <4aac43ab-21f2-4bab-a564-dc470b6c2102@gmx.de> References: Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit User-Agent: Mozilla Thunderbird Content-Language: en-US In-Reply-To: Cc: fortran@gcc.gnu.org X-Spam-Status: No, score=-9.2 required=5.0 tests=BAYES_00,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM,GIT_PATCH_0,HEADER_FROM_DIFFERENT_DOMAINS,KAM_DMARC_STATUS,KAM_SHORT,SPF_HELO_NONE,SPF_PASS,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: Message-ID: <20240430193432.ZM-ixwYH6vjQFv4bg54zyh2KeUaJVeBTO2KFecrMls0@z> Hi Paul, On 4/30/24 07:50, Paul Richard Thomas wrote: > Hi Harald, > > This patch is verging on 'obvious', ..... once one sees it :-) > > Yes, it's good for mainline and all active branches, when available. thanks for your quick review. I haven't committed it yet, because I forgot to check what happens with a class(*) allocatable function result on the r.h.s. of the assignment. One now gets an ICE with the testcase in your submission https://gcc.gnu.org/pipermail/fortran/2024-April/060426.html on the simple scalar assignment y = bar () instead of wrong code. Not very helpful. I tried the following change on top of the submitted patch: diff --git a/gcc/fortran/trans-expr.cc b/gcc/fortran/trans-expr.cc index 4ba40bfdbd3..cacf3c0dda1 100644 --- a/gcc/fortran/trans-expr.cc +++ b/gcc/fortran/trans-expr.cc @@ -11995,7 +11996,11 @@ trans_class_assignment (stmtblock_t *block, gfc_expr *lhs, gfc_expr *rhs, /* Take into account _len of unlimited polymorphic entities. */ if (UNLIMITED_POLY (rhs)) { - tree len = trans_get_upoly_len (block, rhs); + tree len; + if (rhs->expr_type == EXPR_VARIABLE) + len = trans_get_upoly_len (block, rhs); + else + len = gfc_class_len_get (gfc_get_class_from_expr (tmp)); len = fold_build2_loc (input_location, MAX_EXPR, size_type_node, fold_convert (size_type_node, len), size_one_node); This avoids the ICE, but depending on details of bar() this leads to different wrong code from before, and function bar() result(res) class(*), allocatable :: res res = sca end function bar behaves differently from function bar() class(*), allocatable :: bar bar = sca end function bar The minimal and sort of "safe" fix to avoid a new ICE while keeping the fix for simple assignments is to replace in the above snippet if (UNLIMITED_POLY (rhs)) by if (UNLIMITED_POLY (rhs) && rhs->expr_type == EXPR_VARIABLE) omit the other changes above, and defer a fix for assignment of function results, as looking at the dump-tree suggests that this will be a bigger piece of work. (The .span looks suspicious all over the place...) The good thing is: a simple test with array-valued function results did not immediately break the submitted patch... ;-) What do you think? Thanks, Harald > Thanks > > Paul > > PS The fall-out pr114874 is so peculiar that I am dropping everything to > find the source. > > > On Mon, 29 Apr 2024 at 19:39, Harald Anlauf wrote: > >> Dear all, >> >> the attached patch fixes issues with assignments of unlimited polymorphic >> entities that were found with the help of valgrind or asan, see PR. >> Looking >> further into it, it turns out that allocation sizes as well as array spans >> could be set incorrectly, leading to wrong results or heap corruption. >> >> The fix is rather straightforward: take into the _len of unlimited >> polymorphic entities when it is non-zero to get the correct allocation >> sizes and array spans. >> >> The patch has been tested by the reporter, see PR. >> >> Regtested on x86_64-pc-linux-gnu. OK for 15-mainline? >> >> I would like to backport this to active branches where appropriate, >> starting with 14 after it reopens after release. Is this OK? >> >> Thanks, >> Harald >> >> >