Re: [patch, fortran] Warn about out-of-bounds access with DO subscripts

[Thomas Schwinge]

Hi!

On Tue, 26 Sep 2017 18:51:52 +0200, Thomas Koenig  wrote:
> > On Mon, 25 Sep 2017 18:50:49 +0200, Thomas Koenig  
> > wrote:
> >> Thanks for the review, committed as r253156.
> >>
> >> Now, on to some other bugs...
> > 
> > No, back to this one please.  ;-)
> 
> OK, if you insist :-)

;-)


> > And the following gets highlighted, too:
> > 
> >  FAIL: compiler driver --help=fortran option(s): "^ +-.*[^:.]$" absent 
> > from output: "  -Wdo-subscript  Warn about possibly incorrect 
> > subscripts in do loops"
> >  FAIL: compiler driver --help=warnings option(s): "^ +-.*[^:.]$" absent 
> > from output: "  -Wdo-subscript  Warn about possibly incorrect 
> > subscripts in do loops"
> 
> This I don't understand.

Hmm, don't you see that in your test logs?

> Was there anything wrong with my
> change to fortran/lang.opt?

Easy enough to fix, so as obvious, committed to trunk in r253225:

commit a7717725d0bed402378b085bcda8fa3fe337fae9
Author: tschwinge 
Date:   Wed Sep 27 08:35:05 2017 +

Placate gcc.misc-tests/help.exp regarding -Wdo-subscript

gcc/fortran/
* lang.opt : End help text with a period.

git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@253225 
138bc75d-0d04-0410-961f-82ee72b054a4
---
 gcc/fortran/ChangeLog | 4 
 gcc/fortran/lang.opt  | 2 +-
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git gcc/fortran/ChangeLog gcc/fortran/ChangeLog
index d0eef84..c698014 100644
--- gcc/fortran/ChangeLog
+++ gcc/fortran/ChangeLog
@@ -1,3 +1,7 @@
+2017-09-27  Thomas Schwinge  
+
+   * lang.opt : End help text with a period.
+
 2017-09-26  Thomas Koenig  
 
* frontend-passes.c (do_subscript): Don't do anything
diff --git gcc/fortran/lang.opt gcc/fortran/lang.opt
index 37ed4a3..88f6af5 100644
--- gcc/fortran/lang.opt
+++ gcc/fortran/lang.opt
@@ -239,7 +239,7 @@ Warn about most implicit conversions.
 
 Wdo-subscript
 Fortran Var(warn_do_subscript) Warning LangEnabledBy(Fortran,Wextra)
-Warn about possibly incorrect subscripts in do loops
+Warn about possibly incorrect subscripts in do loops.
 
 Wextra
 Fortran Warning


Grüße
 Thomas

Re: [patch, fortran] Warn about out-of-bounds access with DO subscripts

[Thomas Koenig]

Hi Jakub,


   associate(k => v, l => a(i, j), m => a(i, :))

And I don't really see a bug in the testcase...


Hm, I will look at this. Maybe some strange
interaction with associate here...

Regards

Thomas

Re: [patch, fortran] Warn about out-of-bounds access with DO subscripts

[Thomas Koenig]

Hi!


On Mon, 25 Sep 2017 18:50:49 +0200, Thomas Koenig  wrote:

Thanks for the review, committed as r253156.

Now, on to some other bugs...


No, back to this one please.  ;-)


OK, if you insist :-)


Apparently, the changes you prepared for existing testcases did not get
committed, so I'm now seeing some FAILs there.  See also recent posts on
the  mailing list:

 FAIL: gfortran.dg/gomp/associate1.f90   -O  (test for excess errors)
 FAIL: gfortran.dg/predcom-1.f   -O  (test for excess errors)
 FAIL: gfortran.dg/unconstrained_commons.f   -O  (test for excess errors)


I have committed those changes, so that should be gone.


And the following gets highlighted, too:

 FAIL: compiler driver --help=fortran option(s): "^ +-.*[^:.]$" absent from output: 
"  -Wdo-subscript  Warn about possibly incorrect subscripts in do loops"
 FAIL: compiler driver --help=warnings option(s): "^ +-.*[^:.]$" absent from output: 
"  -Wdo-subscript  Warn about possibly incorrect subscripts in do loops"


This I don't understand.  Was there anything wrong with my
change to fortran/lang.opt?

Regards

Thomas

Re: [patch, fortran] Warn about out-of-bounds access with DO subscripts

[Jakub Jelinek]

On Tue, Sep 26, 2017 at 09:17:40AM +0200, Thomas Schwinge wrote:
> Hi!
> 
> On Mon, 25 Sep 2017 18:50:49 +0200, Thomas Koenig  
> wrote:
> > Thanks for the review, committed as r253156.
> > 
> > Now, on to some other bugs...
> 
> No, back to this one please.  ;-)
> 
> Apparently, the changes you prepared for existing testcases did not get
> committed, so I'm now seeing some FAILs there.  See also recent posts on
> the  mailing list:
> 
> FAIL: gfortran.dg/gomp/associate1.f90   -O  (test for excess errors)
> FAIL: gfortran.dg/predcom-1.f   -O  (test for excess errors)
> FAIL: gfortran.dg/unconstrained_commons.f   -O  (test for excess errors)

At least the gfortran.dg/gomp testcase doesn't seem to be related to OpenMP
at all, it fails also with just:
program associate1
  integer :: v, i, j
  real :: a(3, 3)
  i = 1
  j = 2
  associate(k => v, l => a(i, j), m => a(i, :))
  k = 1
  do i = 1, 10
k = k + 2
  end do
  end associate
end program

And I don't really see a bug in the testcase...

Jakub

Re: [patch, fortran] Warn about out-of-bounds access with DO subscripts

[Thomas Schwinge]

Hi!

On Mon, 25 Sep 2017 18:50:49 +0200, Thomas Koenig  wrote:
> Thanks for the review, committed as r253156.
> 
> Now, on to some other bugs...

No, back to this one please.  ;-)

Apparently, the changes you prepared for existing testcases did not get
committed, so I'm now seeing some FAILs there.  See also recent posts on
the  mailing list:

FAIL: gfortran.dg/gomp/associate1.f90   -O  (test for excess errors)
FAIL: gfortran.dg/predcom-1.f   -O  (test for excess errors)
FAIL: gfortran.dg/unconstrained_commons.f   -O  (test for excess errors)

And the following gets highlighted, too:

FAIL: compiler driver --help=fortran option(s): "^ +-.*[^:.]$" absent from 
output: "  -Wdo-subscript  Warn about possibly incorrect subscripts 
in do loops"
FAIL: compiler driver --help=warnings option(s): "^ +-.*[^:.]$" absent from 
output: "  -Wdo-subscript  Warn about possibly incorrect subscripts 
in do loops"


Grüße
 Thomas

Re: [patch, fortran] Warn about out-of-bounds access with DO subscripts

[Thomas Koenig]

Hi Jerry,


Yes OK,


Thanks for the review, committed as r253156.

Now, on to some other bugs...

Regards

Thomas

Re: [patch, fortran] Warn about out-of-bounds access with DO subscripts

[Jerry DeLisle]

On 09/23/2017 05:33 AM, Thomas Koenig wrote:
> Hello world,
> 
> here is an update and a ping for my patch at
> 
> https://gcc.gnu.org/ml/gcc-patches/2017-09/msg01104.html
> 
> This patch warns unconditionally for
> 
>   REAL A(3)
>   DO I=1,4
>  A(I) = 42.
>   END DO
> 
> while only warning conditionally, dependent on a new flag,
> for when the statement containing the expression is hidden
> behind some IF or SELECT CASE statement or if there
> is something in the DO loop which could potentially exit the loop,
> so
> 
>   REAL A(3)
>   DO I=1,4
>     IF (CONDITON) A(I) = 42.
>   END DO
> 
> will require the new -Wdo-subscript or the -Wextra flag.
> 
> Regression-tested. OK for trunk?
> 
> Regards
> 
> Thomas

Yes OK,

Jerry

[patch, fortran] Warn about out-of-bounds access with DO subscripts

[Thomas Koenig]

Hello world,

here is an update and a ping for my patch at

https://gcc.gnu.org/ml/gcc-patches/2017-09/msg01104.html

This patch warns unconditionally for

  REAL A(3)
  DO I=1,4
 A(I) = 42.
  END DO

while only warning conditionally, dependent on a new flag,
for when the statement containing the expression is hidden
behind some IF or SELECT CASE statement or if there
is something in the DO loop which could potentially exit the loop,
so

  REAL A(3)
  DO I=1,4
IF (CONDITON) A(I) = 42.
  END DO

will require the new -Wdo-subscript or the -Wextra flag.

Regression-tested. OK for trunk?

Regards

Thomas

2017-09-23  Thomas Koenig  

* lang.opt:  Add -Wdo-subscript.
* frontend-passes.c (do_t): New type.
(doloop_list): Use variable of do_type.
(if_level): Variable to track if levels.
(select_level): Variable to track select levels.
(gfc_run_passes): Initialize i_level and select_level.
(doloop_code): Record current level of if + select
level in doloop_list.  Add seen_goto if there could
be a branch outside the loop. Use different type for
doloop_list.
(doloop_function): Call do_intent and do_subscript; move
functionality of checking INTENT to do_intent.
(insert_index_t): New type, for callback_insert_index.
(callback_insert_index): New function.
(insert_index): New function.
(do_subscript): New function.
(do_intent): New function.
(gfc_code_walker): Keep track of if_level and select_level.
* invoke.texi: Document -Wdo-subscript.

2017-09-23  Thomas Koenig  

* gfortran.dg/do_subscript_1.f90: New test.
* gfortran.dg/do_subscript_2.f90: New test.
* gfortran.dg/gomp/associate1.f90: Add out of bounds warning.
* gfortran.dg/predcom-1.f: Adjust loop bounds.
* gfortran.dg/unconstrained_commons.f: Add out of bounds warning.
Index: gcc/fortran/frontend-passes.c
===
--- gcc/fortran/frontend-passes.c	(Revision 253076)
+++ gcc/fortran/frontend-passes.c	(Arbeitskopie)
@@ -39,6 +39,8 @@ static bool optimize_lexical_comparison (gfc_expr
 static void optimize_minmaxloc (gfc_expr **);
 static bool is_empty_string (gfc_expr *e);
 static void doloop_warn (gfc_namespace *);
+static int do_intent (gfc_expr **);
+static int do_subscript (gfc_expr **);
 static void optimize_reduction (gfc_namespace *);
 static int callback_reduction (gfc_expr **, int *, void *);
 static void realloc_strings (gfc_namespace *);
@@ -98,10 +100,20 @@ static int iterator_level;
 
 /* Keep track of DO loop levels.  */
 
-static vec doloop_list;
+typedef struct {
+  gfc_code *c;
+  int branch_level;
+  bool seen_goto;
+} do_t;
 
+static vec doloop_list;
 static int doloop_level;
 
+/* Keep track of if and select case levels.  */
+
+static int if_level;
+static int select_level;
+
 /* Vector of gfc_expr * to keep track of DO loops.  */
 
 struct my_struct *evec;
@@ -133,6 +145,8 @@ gfc_run_passes (gfc_namespace *ns)
  change.  */
 
   doloop_level = 0;
+  if_level = 0;
+  select_level = 0;
   doloop_warn (ns);
   doloop_list.release ();
   int w, e;
@@ -2231,6 +2245,8 @@ doloop_code (gfc_code **c, int *walk_subtrees ATTR
   gfc_formal_arglist *f;
   gfc_actual_arglist *a;
   gfc_code *cl;
+  do_t loop, *lp;
+  bool seen_goto;
 
   co = *c;
 
@@ -2239,16 +2255,67 @@ doloop_code (gfc_code **c, int *walk_subtrees ATTR
   if ((unsigned) doloop_level < doloop_list.length())
 doloop_list.truncate (doloop_level);
 
+  seen_goto = false;
   switch (co->op)
 {
 case EXEC_DO:
 
   if (co->ext.iterator && co->ext.iterator->var)
-	doloop_list.safe_push (co);
+	loop.c = co;
   else
-	doloop_list.safe_push ((gfc_code *) NULL);
+	loop.c = NULL;
+
+  loop.branch_level = if_level + select_level;
+  loop.seen_goto = false;
+  doloop_list.safe_push (loop);
   break;
 
+  /* If anything could transfer control away from a suspicious
+	 subscript, make sure to set seen_goto in the current DO loop
+	 (if any).  */
+case EXEC_GOTO:
+case EXEC_EXIT:
+case EXEC_STOP:
+case EXEC_ERROR_STOP:
+case EXEC_CYCLE:
+  seen_goto = true;
+  break;
+
+case EXEC_OPEN:
+  if (co->ext.open->err)
+	seen_goto = true;
+  break;
+
+case EXEC_CLOSE:
+  if (co->ext.close->err)
+	seen_goto = true;
+  break;
+
+case EXEC_BACKSPACE:
+case EXEC_ENDFILE:
+case EXEC_REWIND:
+case EXEC_FLUSH:
+
+  if (co->ext.filepos->err)
+	seen_goto = true;
+  break;
+
+case EXEC_INQUIRE:
+  if (co->ext.filepos->err)
+	seen_goto = true;
+  break;
+
+case EXEC_READ:
+case EXEC_WRITE:
+  if (co->ext.dt->err || co->ext.dt->end || co->ext.dt->eor)
+	seen_goto = true;
+  break;
+
+case EXEC_WAIT:
+  if (co->ext.wait->err || co->ext.wait->end || co->ext.wait->eor)
+