Bail out in gfc_dep_compare_expr for a NULL argument.
diff mbox series

Message ID ce5f3834-ae3e-910d-c491-57be64a04188@suse.cz
State New
Headers show
Series
  • Bail out in gfc_dep_compare_expr for a NULL argument.
Related show

Commit Message

Martin Liška Dec. 10, 2019, 1:04 p.m. UTC
Hi.

The patch is about early bail out in gfc_dep_compare_expr function
when one of the arguments (and not both at the same time) is null.
I'm not sure, it's a proper fix however.

Patch can bootstrap on x86_64-linux-gnu and survives regression tests.

Ready to be installed?
Thanks,
Martin

gcc/fortran/ChangeLog:

2019-12-10  Martin Liska  <mliska@suse.cz>

	PR fortran/92874
	* dependency.c (gfc_dep_compare_expr): Bail out
	when one of the arguments is null.

gcc/testsuite/ChangeLog:

2019-12-10  Martin Liska  <mliska@suse.cz>

	PR fortran/92874
	* gfortran.dg/pr92874.f90: New test.
---
  gcc/fortran/dependency.c              |  2 ++
  gcc/testsuite/gfortran.dg/pr92874.f90 | 11 +++++++++++
  2 files changed, 13 insertions(+)
  create mode 100644 gcc/testsuite/gfortran.dg/pr92874.f90

Comments

Tobias Burnus Dec. 10, 2019, 2 p.m. UTC | #1
Hi Martin,

On 12/10/19 2:04 PM, Martin Liška wrote:
> The patch is about early bail out in gfc_dep_compare_expr function
> when one of the arguments (and not both at the same time) is null.
> I'm not sure, it's a proper fix however.

I think it is the best one can do. Technically, one could replace the 
comparison by
"length == 1 || "" == x(1:)" but I am not sure whether that would be be 
advantageous in general (esp. when taking implementation time into 
account); in any case, that's independent of the ICE.

The caller in this example is "are_identical_variables":

           if (gfc_dep_compare_expr (r1->u.ss.end, r2->u.ss.end) != 0)
             return false;
comparing the string end.

And "-2 if the relationship could not be determined".


> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
>
> Ready to be installed?

Yes; it looks good to me – thanks!

Tobias

> gcc/fortran/ChangeLog:
>
> 2019-12-10  Martin Liska  <mliska@suse.cz>
>
>     PR fortran/92874
>     * dependency.c (gfc_dep_compare_expr): Bail out
>     when one of the arguments is null.
>
> gcc/testsuite/ChangeLog:
>
> 2019-12-10  Martin Liska  <mliska@suse.cz>
>
>     PR fortran/92874
>     * gfortran.dg/pr92874.f90: New test.
> ---
>  gcc/fortran/dependency.c              |  2 ++
>  gcc/testsuite/gfortran.dg/pr92874.f90 | 11 +++++++++++
>  2 files changed, 13 insertions(+)
>  create mode 100644 gcc/testsuite/gfortran.dg/pr92874.f90
>
>

Patch
diff mbox series

diff --git a/gcc/fortran/dependency.c b/gcc/fortran/dependency.c
index 394d85b48a2..eb741137d60 100644
--- a/gcc/fortran/dependency.c
+++ b/gcc/fortran/dependency.c
@@ -319,6 +319,8 @@  gfc_dep_compare_expr (gfc_expr *e1, gfc_expr *e2)
 
   if (e1 == NULL && e2 == NULL)
     return 0;
+  else if (e1 == NULL || e2 == NULL)
+    return -2;
 
   e1 = gfc_discard_nops (e1);
   e2 = gfc_discard_nops (e2);
diff --git a/gcc/testsuite/gfortran.dg/pr92874.f90 b/gcc/testsuite/gfortran.dg/pr92874.f90
new file mode 100644
index 00000000000..a1fca3d7718
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/pr92874.f90
@@ -0,0 +1,11 @@ 
+! { dg-do compile }
+! { dg-options "-O2" }
+! PR fortran/92874
+program p
+   call s('a')
+   call s('abc')
+end
+subroutine s(x)
+   character(*) :: x
+   print *, (x(1:1) == x(1:))
+end