From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr1-x42e.google.com (mail-wr1-x42e.google.com [IPv6:2a00:1450:4864:20::42e]) by sourceware.org (Postfix) with ESMTPS id 9A9063858D28; Sat, 27 Jul 2024 17:23:27 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 9A9063858D28 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 9A9063858D28 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2a00:1450:4864:20::42e ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1722101014; cv=none; b=LpURNqBEHP/F1XRCoPyuk7Ib2R1c7l+Sw4/ngPEvHGS1hJ+lPJa2nGzebHoWkKgjuhArKOFniKqiINw1r/b2b583+cl4RGiF6+F884n50Q5MZZbnjF9WIzAYhbzGVGWCKscD8d3XH5gZbdydNFOuAoSS6FZyGFL4OjTqqtFcycU= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1722101014; c=relaxed/simple; bh=IzinSCZJCROtP/NmWf5qM1Vnx7FFGtz0kKF9sj66Wk4=; h=DKIM-Signature:Date:From:To:Subject:Message-ID:MIME-Version; b=crtxKbrZaA7+9fdFXIgXgGNykUr6jRuhPfBjiYbseKI03qY4CSUadTjPFL2c8fj6KHQuFrGxE/U8SXje2dID05+wkdy57tjS1YWvjjQObXg7YWI/SRAZ6hcjGsGYw7+hVXG1YMG6QZ1BvBoNPLf2E5AE8GveQKkTbOedkhAJtYw= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-wr1-x42e.google.com with SMTP id ffacd0b85a97d-3686b554cfcso455062f8f.1; Sat, 27 Jul 2024 10:23:27 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1722101006; x=1722705806; darn=gcc.gnu.org; h=content-transfer-encoding:mime-version:message-id:references :in-reply-to:subject:to:from:date:from:to:cc:subject:date:message-id :reply-to; bh=4EGBb+r0hBxLM8axz7f8dJPNOVUkj3T+lvZwJ9OFPv8=; b=STZGdRdR1DZNZArjzUnq3/aFUU1E1D3NZkTv2eKDLysRNzeXGO2z11lUdzsRk9k+GX qEaruKtYDjEBSQ2btHSYl1Nf0bBe3cqJxdUcFuwwKje42o240tDwermO5+dn1gCKhI16 rn2SVKmh6F8nMGTRuKndbMpC1QM4DjMKcLg1tZNzG06pyr7JpfdztMfZWUB59mioiB5O yQsv7qbCf596SaX0BbRt/rIulk1/U0PDSINEfcEI68XtojdbVP+Nzg4HsnH9ukWgakpx OEkms6rR8T69x9u4t1jdKhM7jbchdWrhtcWYkP23875znix4pgKD2T+mVXID1kj8k/FI A5bg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1722101006; x=1722705806; h=content-transfer-encoding:mime-version:message-id:references :in-reply-to:subject:to:from:date:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=4EGBb+r0hBxLM8axz7f8dJPNOVUkj3T+lvZwJ9OFPv8=; b=dEtHPyhU4vThtNme/YsL5rSbFx6CdhKhnktlnfmwT3YKpAuNomDXeMgsyNqsi/TSXh 6BCnpoqE0NKGHyqj/7BBQmvlvcp5ow0kF/1ojgRvZtxPaNtMcSa5E1zeJy+AHQCz8hUS KGbJEDzVnaKDCd19QwTKmQy33VEwR8NUx3V81M9BHKn/Xqf+/t4d99e6whNHvxWO96/X 5DzVQ5ZROdv+kG90FMG3bBswn3b/vHntpZVReD5rouswQnz8ucvV15L2MB29fn5cbW/2 kzaXj9vgZ/0cKVFjkEXVcrbvGGI2mQPX/yjAEpm/8ca46b/5+DpwxgubHANnaiLeRcxh 6QFQ== X-Forwarded-Encrypted: i=1; AJvYcCWW3W1U2G9o2Ukh9AAvGrTcJHkytWkIxIkHLNGCtNvlY00hneUCwfEhm1887CNXF2sfVHB2ZVMKkasYlT/Om5FdKFDDmw8+Yw== X-Gm-Message-State: AOJu0YzQn4vBTZv3ljuuvRgMaO1SaExZp8Wx+07s/VrnuadLWr+bJ6h9 VsmUR2/diuifx+lDyOjGgpHklPCBKrd8bV5GP0xi6mvjyIaNetI1YdzVxQ== X-Google-Smtp-Source: AGHT+IHNbbAVacGdTek9PNxEg3qpPxatMUmAvrGj0xEuLFDekBClvT/h3spT2HgHMn4I8T7D/E6vzQ== X-Received: by 2002:a05:6000:1868:b0:36b:3384:40e5 with SMTP id ffacd0b85a97d-36b5cefdc3cmr1815847f8f.24.1722101005473; Sat, 27 Jul 2024 10:23:25 -0700 (PDT) Received: from ?IPv6:::1? ([2001:871:227:e712:b96:3ea4:c4a9:54a1]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-42809eb192csm60432905e9.1.2024.07.27.10.23.24 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Sat, 27 Jul 2024 10:23:24 -0700 (PDT) Date: Sat, 27 Jul 2024 19:23:18 +0200 From: rep.dot.nop@gmail.com To: fortran@gcc.gnu.org, Mikael Morin , gcc-patches@gcc.gnu.org Subject: =?US-ASCII?Q?Re=3A_=5BPATCH=5D_fortran=3A_Support_optional_d?= =?US-ASCII?Q?ummy_as_BACK_argument_of_MINLOC/MAXLOC=2E?= In-Reply-To: <20240722185318.770942-1-morin-mikael@orange.fr> References: <20240722185318.770942-1-morin-mikael@orange.fr> Message-ID: <2C7F693E-C3DF-44D6-B258-18327C118F75@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-10.4 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,GIT_PATCH_0,RCVD_IN_DNSWL_NONE,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: On 22 July 2024 20:53:18 CEST, Mikael Morin wrot= e: >From: Mikael Morin > >Hello, > >this fixes a null pointer dereference with absent optional dummy passed >as BACK argument of MINLOC/MAXLOC=2E > >Tested for regression on x86_64-linux=2E >OK for master? > >-- >8 -- > >Protect the evaluation of BACK with a check that the reference is non-nul= l >in case the expression is an optional dummy, in the inline code generated >for MINLOC and MAXLOC=2E > >This change contains a revert of the non-testsuite part of commit >r15-1994-ga55d24b3cf7f4d07492bb8e6fcee557175b47ea3, which factored the >evaluation of BACK out of the loop using the scalarizer=2E It was a bad = idea, >because delegating the argument evaluation to the scalarizer makes it >cumbersome to add a null pointer check next to the evaluation=2E > >Instead, evaluate BACK at the beginning, before scalarization, add a chec= k >that the argument is present if necessary, and evaluate the resulting >expression to a variable, before using the variable in the inline code=2E > >gcc/fortran/ChangeLog: > > * trans-intrinsic=2Ecc (maybe_absent_optional_variable): New function=2E > (gfc_conv_intrinsic_minmaxloc): Remove BACK from scalarization and > evaluate it before=2E Add a check that BACK is not null if the > expression is an optional dummy=2E Save the resulting expression to a > variable=2E Use the variable in the generated inline code=2E > >gcc/testsuite/ChangeLog: > > * gfortran=2Edg/maxloc_6=2Ef90: New test=2E > * gfortran=2Edg/minloc_7=2Ef90: New test=2E >--- > gcc/fortran/trans-intrinsic=2Ecc | 81 +++++- > gcc/testsuite/gfortran=2Edg/maxloc_6=2Ef90 | 366 +++++++++++++++++++++++= ++ > gcc/testsuite/gfortran=2Edg/minloc_7=2Ef90 | 366 +++++++++++++++++++++++= ++ > 3 files changed, 799 insertions(+), 14 deletions(-) > create mode 100644 gcc/testsuite/gfortran=2Edg/maxloc_6=2Ef90 > create mode 100644 gcc/testsuite/gfortran=2Edg/minloc_7=2Ef90 > >diff --git a/gcc/fortran/trans-intrinsic=2Ecc b/gcc/fortran/trans-intrins= ic=2Ecc >index 180d0d7a88c=2E=2E9f3c3ce47bc 100644 >--- a/gcc/fortran/trans-intrinsic=2Ecc >+++ b/gcc/fortran/trans-intrinsic=2Ecc >@@ -5209,6 +5209,50 @@ gfc_conv_intrinsic_dot_product (gfc_se * se, gfc_e= xpr * expr) > } >=20 >=20 >+/* Tells whether the expression E is a reference to an optional variable= whose >+ presence is not known at compile time=2E Those are variable referenc= es without >+ subreference; if there is a subreference, we can assume the variable = is >+ present=2E We have to special case full arrays, which we represent w= ith a fake >+ "full" reference, and class descriptors for which a reference to data= is not >+ really a subreference=2E */ >+ >+bool >+maybe_absent_optional_variable (gfc_expr *e) >+{ >+ if (!(e && e->expr_type =3D=3D EXPR_VARIABLE)) >+ return false; [] >+} >+ >+ > /* Remove unneeded kind=3D argument from actual argument list when the > result conversion is dealt with in a different place=2E */ >=20 >@@ -5321,11 +5365,11 @@ gfc_conv_intrinsic_minmaxloc (gfc_se * se, gfc_ex= pr * expr, enum tree_code op) > tree nonempty; > tree lab1, lab2; > tree b_if, b_else; >+ tree back; > gfc_loopinfo loop; > gfc_actual_arglist *actual; > gfc_ss *arrayss; > gfc_ss *maskss; >- gfc_ss *backss; > gfc_se arrayse; > gfc_se maskse; > gfc_expr *arrayexpr; >@@ -5391,10 +5435,27 @@ gfc_conv_intrinsic_minmaxloc (gfc_se * se, gfc_ex= pr * expr, enum tree_code op) > && maskexpr->symtree->n=2Esym->attr=2Edummy > && maskexpr->symtree->n=2Esym->attr=2Eoptional; > backexpr =3D actual->next->next->expr; >- if (backexpr) >- backss =3D gfc_get_scalar_ss (gfc_ss_terminator, backexpr); >+ >+ gfc_init_se (&backse, NULL); >+ if (backexpr =3D=3D nullptr) >+ back =3D logical_false_node; >+ else if (maybe_absent_optional_variable (backexpr)) >+ { /* if this fires, some wonky race is going on=2E=2E */ >+ gcc_assert (backexpr->expr_type =3D=3D EXPR_VARIABLE); drop it, downgrade to checking, or is it worth? >+ >+ gfc_conv_expr (&backse, backexpr); >+ tree present =3D gfc_conv_expr_present (backexpr->symtree->n=2Esym= , false); >+ back =3D fold_build2_loc (input_location, TRUTH_ANDIF_EXPR, >+ logical_type_node, present, backse=2Eexpr); >+ } > else >- backss =3D nullptr; >+ { >+ gfc_conv_expr (&backse, backexpr); >+ back =3D backse=2Eexpr; >+ } >+ gfc_add_block_to_block (&se->pre, &backse=2Epre); >+ back =3D gfc_evaluate_now_loc (input_location, back, &se->pre); >+ gfc_add_block_to_block (&se->pre, &backse=2Epost); >=20 > nonempty =3D NULL [] thanks