From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from esa3.mentor.iphmx.com (esa3.mentor.iphmx.com [68.232.137.180]) by sourceware.org (Postfix) with ESMTPS id 73CD3384F031; Wed, 21 Jul 2021 17:26:16 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 73CD3384F031 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=codesourcery.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=mentor.com IronPort-SDR: xqJcDvKhMs7JeV2iiW6QE8JP2u+BrIibJNQrngCETyv9g+a19DqUHjxD11JSbD+o8kmeKwQIa/ sq01TPbWGJteVQmKev+iPdVKz00P4cBbRwnTuUicxuWrQWP3CSQIbuDjKt0IFePtiloWRwreRp oRuzaV1ZjiTCGtU+DYNFMdwgz0UoKp5zpbEmeV04u4CIFkXe5CpQcvTYk4BejDIvWs+X+Nic+w JQoEbjL2a4rEFfLn8+vdXFvdt1Uch5R4xLbNWBg0WilbzFFUNEnmoG2VpxPRuPq0eaypGIbWTZ Pn9W+zdlworJzGSKd1tjy+mn X-IronPort-AV: E=Sophos;i="5.84,258,1620720000"; d="scan'208";a="63779661" Received: from orw-gwy-01-in.mentorg.com ([192.94.38.165]) by esa3.mentor.iphmx.com with ESMTP; 21 Jul 2021 09:26:15 -0800 IronPort-SDR: Qbvez95Sh5/L3WHQOvAs82+Ok6AKlpeVN1HkHm7K5wgsnAYfT4L9oUNnrsbBnTBGI/HByvF0YH ovmQVo8lzFUoH4QlT3Jfd1sSAAMA0Q8DG6GGLmMTFlhVWWrHqGjAIQlaNC71l0EV7YuxDgGd88 HlHuBWnXGcuPcjd50c0TZNXvCyHGXE7U7E02WSBRWjoNyGiBAFo6sgcFUAfmYZrvu4SES8StZN v4mMOWcnIIAFwhmpC0yVGocUiPpmi6N7idj2EVwqd+kx5nVN7j7LbcjcN/UmlHs7soq/Nwz+zT mRU= Subject: Re: [PATCH, Fortran] [PR libfortran/101317] Bind(c): Improve error checking in CFI_* functions To: Sandra Loosemore , "fortran@gcc.gnu.org" , "gcc-patches@gcc.gnu.org" References: <8fc3b97c-b122-9c2c-4657-4f98cf82973e@codesourcery.com> From: Tobias Burnus Message-ID: Date: Wed, 21 Jul 2021 19:26:09 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.12.0 MIME-Version: 1.0 In-Reply-To: <8fc3b97c-b122-9c2c-4657-4f98cf82973e@codesourcery.com> Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: quoted-printable Content-Language: en-US X-Originating-IP: [137.202.0.90] X-ClientProxiedBy: SVR-IES-MBX-07.mgc.mentorg.com (139.181.222.7) To svr-ies-mbx-01.mgc.mentorg.com (139.181.222.1) X-Spam-Status: No, score=-5.8 required=5.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS, KAM_DMARC_STATUS, NICE_REPLY_A, SPF_HELO_PASS, SPF_PASS, TXREP autolearn=no autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) 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-List-Received-Date: Wed, 21 Jul 2021 17:26:18 -0000 On 17.07.21 02:49, Sandra Loosemore wrote: > This patch is for PR101317, one of the bugs uncovered by the TS29113 > testsuite. Here I'd observed that CFI_establish, etc was not > diagnosing some invalid-argument situations documented in the > standard, although it was properly catching others. After fixing > those I discovered a couple small mistakes in the test cases and fixed > those too. Some first comments =E2=80=93 I think I have to read though the file ISO_Fortran_binding.c itself and not only your patch. > --- a/libgfortran/runtime/ISO_Fortran_binding.c > +++ b/libgfortran/runtime/ISO_Fortran_binding.c > @@ -232,7 +232,16 @@ CFI_allocate (CFI_cdesc_t *dv, const CFI_index_t low= er_bounds[], > /* If the type is a Fortran character type, the descriptor's element > length is replaced by the elem_len argument. */ > if (dv->type =3D=3D CFI_type_char || dv->type =3D=3D CFI_type_ucs4_ch= ar) > - dv->elem_len =3D elem_len; > + { > + if (unlikely (compile_options.bounds_check) && elem_len =3D=3D 0) > + { > + fprintf ("CFI_allocate: The supplied elem_len must be " > + "greater than zero (elem_len =3D %d).\n", > + (int) elem_len); I think there is no need to use '(elem_len =3D %d)' given that it is always= zero as stated in the error message itself. (Appears twice) However, the check itself is also wrong =E2=80=93 cf. below. * * * Talking about CFI_allocatable, there is also another bug in that function, untouched by your patch: /* If the type is a character, the descriptor's element length is replace= d by the elem_len argument. */ if (dv->type =3D=3D CFI_type_char || dv->type =3D=3D CFI_type_ucs4_char = || dv->type =3D=3D CFI_type_signed_char) dv->elem_len =3D elem_len; The bug is that CFI_type_signed_char is not a character type. > + else if (unlikely (compile_options.bounds_check) > + && type < 0) Pointless line break. > + fprintf (stderr, "CFI_establish: Extents must be nonnegative = " > + "(extents[%d] =3D %d).\n", i, (int)extents[i]); > + return CFI_INVALID_EXTENT; > + } How about PRIiPTR + ptrdiff_t instead of %d + (int) cast? At least as positive value, extent may exceed INT_MAX. (Twice) > if (result->type =3D=3D CFI_type_char || result->type =3D=3D CFI_type= _ucs4_char) > - result->elem_len =3D elem_len; > + { > + if (unlikely (compile_options.bounds_check) && elem_len =3D=3D 0) > + { > + fprintf ("CFI_select_part: The supplied elem_len must be " > + "greater than zero (elem_len =3D %d).\n", > + (int) elem_len); What's wrong with ["", ""]? Or with: character(len=3D:), allocatable :: str2(:) str2 =3D [str1(5:4)] both are len(...) =3D=3D 0 arrays with 1 or 2 elements. > + if (source->attribute =3D=3D CFI_attribute_other > + && source->rank > 0 > + && source->dim[source->rank - 1].extent =3D=3D -1) > + { > + fprintf (stderr, "CFI_setpointer: The source is a " > + "nonallocatable nonpointer object that is an " > + "assumed-size array.\n"); I think you could just check for assumed rank =E2=80=93 without CFI_attribute_other in the 'if' and 'nonallocatable nonpointer' in the error message. Only nonallocatable nonpointer variables can be of assumed size (in Fortran); I think that makes the message simpler (focusing on the issue) and if the C user passes an allocatable/pointer, which is assumed rank, it is also a bug. Tobias ----------------- 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