RE: [PATCH] PR fortran/77505 -- Treat negative character length as LEN=0

[Punnoose, Elizebeth]

Thank you Steve.

Thanks,
Elizebeth

-Original Message-
From: Steve Kargl [mailto:s...@troutmask.apl.washington.edu] 
Sent: 02 December 2016 04:54
To: Punnoose, Elizebeth <elizebeth.punno...@hpe.com>
Cc: fort...@gcc.gnu.org; gcc-patches@gcc.gnu.org
Subject: Re: [PATCH] PR fortran/77505 -- Treat negative character length as 
LEN=0

On Wed, Nov 30, 2016 at 05:13:28AM +, Punnoose, Elizebeth wrote:
> Please excuse the messy formatting in my initial mail.  Resending with 
> proper formatting.
> 
> This patch checks for negative character length in the array 
> constructor, and treats it as LEN=0.
> 
> A warning message is also printed if bounds checking is enabled.
> 
> Bootstrapped and regression tested the patch on x86_64-linux-gnu and 
> aarch64-linux-gnu.
> 

Thanks.  After regression testing on x86_64-*-freebsd, I committed the attached 
patch.  Not sure if the whitespace got messed up by my email agent, but I 
needed to reformat your testcases.  I took the opportunity to rename and 
improve the testcases.  The improvements check that in fact len=0 and that a 
warning is issued. 

Hopefully, you're inclined to submit additional patches in the future.
A few recommendations are to include the text of your ChangeLog entry in body 
of the email, for example,

2016-12-01  Elizebeth Punnoose  <elizebeth.punno...@hpe.com>

PR fortran/77505
* trans-array.c (trans_array_constructor): Treat negative character
length as LEN = 0.

2016-12-01  Elizebeth Punnoose  <elizebeth.punno...@hpe.com>

PR fortran/77505
* gfortran.dg/char_length_20.f90: New test.
* gfortran.dg/char_length_21.f90: Ditto.

(Note, 2 spaces before and after your name.)  Then attach the patch to the 
email.  This hopefully will prevent formatting issues with various email 
clients/servers. 

--
Steve

RE: [PATCH] PR fortran/77505 -- Treat negative character length as LEN=0

[Punnoose, Elizebeth]

Please excuse the messy formatting in my initial mail. Resending with proper 
formatting.

This patch checks for negative character length in the array constructor, and 
treats it as LEN=0. 

A warning message is also printed if bounds checking is enabled.

Bootstrapped and regression tested the patch on x86_64-linux-gnu and 
aarch64-linux-gnu.

Index: ChangeLog
===
--- ChangeLog   (revision 242906)
+++ ChangeLog   (working copy)
@@ -1,3 +1,9 @@
+2016-11-30  Elizebeth Punnoose 
+
+   PR fortran/77505
+   * trans-array.c (trans_array_constructor): Treat negative character
+   length as LEN=0.
+
 2016-11-27  Paul Thomas  
 
PR fortran/78474

Index: trans-array.c
===
--- trans-array.c   (revision 242906)
+++ trans-array.c   (working copy)
@@ -2226,6 +2226,8 @@ trans_array_constructor (gfc_ss * ss, lo
   gfc_ss_info *ss_info;
   gfc_expr *expr;
   gfc_ss *s;
+  tree neg_len;
+  char *msg;
 
   /* Save the old values for nested checking.  */
   old_first_len = first_len;
@@ -2271,6 +2273,28 @@ trans_array_constructor (gfc_ss * ss, lo
  gfc_conv_expr_type (_se, expr->ts.u.cl->length,
 gfc_charlen_type_node);
  ss_info->string_length = length_se.expr;
+
+ /* Check if the character length is negative,
+   if so consider it as LEN=0.  */
+ neg_len = fold_build2_loc (input_location, LT_EXPR,
+  boolean_type_node, 
ss_info->string_length,
+  build_int_cst (gfc_charlen_type_node, 
0));
+ /* Print a warning if bounds checking is enabled.  */
+ if (gfc_option.rtcheck & GFC_RTCHECK_BOUNDS)
+ {
+msg = xasprintf ("Negative character length will be treated"
+" as LEN=0");
+gfc_trans_runtime_check (false, true, neg_len, _se.pre,
+where, msg);
+free (msg);
+ }
+ ss_info->string_length = fold_build3_loc (input_location, COND_EXPR,
+  gfc_charlen_type_node, neg_len,
+  build_int_cst (gfc_charlen_type_node, 0),
+  ss_info->string_length);
+ ss_info->string_length = gfc_evaluate_now (ss_info->string_length,
+   _se.pre);
+
  gfc_add_block_to_block (_loop->pre, _se.pre);
  gfc_add_block_to_block (_loop->post, _se.post);
}


Index: ChangeLog
===
--- ChangeLog   (revision 242906)
+++ ChangeLog   (working copy)
@@ -1,3 +1,9 @@
+2016-11-30 Elizebeth Punnoose 
+
+   PR fortran/77505
+   * gfortran.dg/pr77505_1.f90: New test.
+   * gfortran.dg/pr77505_2.f90: New test.
+
 2016-11-27  Paul Thomas  
 
PR fortran/78474


Index: pr77505_1.f90
===
--- pr77505_1.f90 (nonexistent)
+++ pr77505_1.f90  (working copy)
@@ -0,0 +1,13 @@
+! { dg-do run }
+program rabbithole
+implicit none
+character(len=:),allocatable    :: text_block(:) integer   
  
+:: i integer :: ii
+character(len=10)   :: cten='abcdefghij'
+character(len=20)   :: ctwenty='abcdefghijabcdefghij'
+ii=-6
+text_block=[ character(len=ii) :: cten, ctwenty ] write(*,*)'WRITE IT'
+write(*,'(a)')(trim(text_block(i)),i=1,size(text_block))
+end program rabbithole


Index: pr77505_2.f90
===
--- pr77505_2.f90 (nonexistent)
+++ pr77505_2.f90  (working copy)
@@ -0,0 +1,14 @@
+! { dg-options "-fcheck=bounds" }
+! { dg-do run }
+program rabbithole
+implicit none
+character(len=:),allocatable    :: text_block(:)
+integer :: i
+integer :: ii
+character(len=10)   :: cten='abcdefghij'
+character(len=20)   :: ctwenty='abcdefghijabcdefghij'
+ii=-6
+text_block=[ character(len=ii) :: cten, ctwenty ]
+write(*,*)'WRITE IT'
+write(*,'(a)')(trim(text_block(i)),i=1,size(text_block))
+end program rabbithole


Thanks,
Elizebeth

[PATCH] PR fortran/77505 -- Treat negative character length as LEN=0

[Punnoose, Elizebeth]

This patch checks for negative character length in the array constructor,
and treats it as LEN=0. A warning message is also printed if bounds checking is 
enabled.

Bootstrapped and regression tested the patch on x86_64-linux-gnu and 
aarch64-linux-gnu.

Index: ChangeLog
===
--- ChangeLog    (revision 242906)
+++ ChangeLog (working copy)
@@ -1,3 +1,9 @@
+2016-11-30  Elizebeth Punnoose 
+
+ PR fortran/77505
+ * trans-array.c (trans_array_constructor): Treat negative 
character
+ length as LEN=0.
+
2016-11-27  Paul Thomas  

    PR fortran/78474


Index: trans-array.c
===
--- trans-array.c (revision 242906)
+++ trans-array.c (working copy)
@@ -2226,6 +2226,8 @@ trans_array_constructor (gfc_ss * ss, lo
   gfc_ss_info *ss_info;
   gfc_expr *expr;
   gfc_ss *s;
+  tree neg_len;
+  char *msg;

   /* Save the old values for nested checking.  */
   old_first_len = first_len;
@@ -2271,6 +2273,28 @@ trans_array_constructor (gfc_ss * ss, lo
     gfc_conv_expr_type (_se, expr->ts.u.cl->length,
     gfc_charlen_type_node);
     ss_info->string_length = length_se.expr;
+
+   /* Check if the character length is negative,
+  if so consider it as LEN=0.  */
+   neg_len = fold_build2_loc (input_location, LT_EXPR,
+  
boolean_type_node, ss_info->string_length,
+  
build_int_cst (gfc_charlen_type_node, 0));
+   /* Print a warning if bounds checking is enabled.  */
+   if (gfc_option.rtcheck & GFC_RTCHECK_BOUNDS)
+   {
+     msg = xasprintf ("Negative character length will be treated"
+  " as LEN=0");
+     gfc_trans_runtime_check (false, true, neg_len, _se.pre,
+  where, msg);
+     free (msg);
+   }
+   ss_info->string_length = fold_build3_loc (input_location, 
COND_EXPR,
+   
gfc_charlen_type_node, neg_len,
+   
build_int_cst (gfc_charlen_type_node, 0),
+   
ss_info->string_length);
+   ss_info->string_length = gfc_evaluate_now 
(ss_info->string_length,
+   
       _se.pre);
+
     gfc_add_block_to_block (_loop->pre, _se.pre);
     gfc_add_block_to_block (_loop->post, _se.post);
   }


Index: ChangeLog
===
--- ChangeLog    (revision 242906)
+++ ChangeLog (working copy)
@@ -1,3 +1,9 @@
+2016-11-30 Elizebeth Punnoose 
+
+ PR fortran/77505
+ * gfortran.dg/pr77505_1.f90: New test.
+ * gfortran.dg/pr77505_2.f90: New test.
+
2016-11-27  Paul Thomas  

    PR fortran/78474


Index: pr77505_1.f90
===
--- pr77505_1.f90 (nonexistent)
+++ pr77505_1.f90  (working copy)
@@ -0,0 +1,13 @@
+! { dg-do run }
+program rabbithole
+implicit none
+character(len=:),allocatable    :: text_block(:)
+integer :: i
+integer :: ii
+character(len=10)   :: cten='abcdefghij'
+character(len=20)   :: ctwenty='abcdefghijabcdefghij'
+ii=-6
+text_block=[ character(len=ii) :: cten, ctwenty ]
+write(*,*)'WRITE IT'
+write(*,'(a)')(trim(text_block(i)),i=1,size(text_block))
+end program rabbithole


Index: pr77505_2.f90
===
--- pr77505_2.f90 (nonexistent)
+++ pr77505_2.f90  (working copy)
@@ -0,0 +1,14 @@
+! { dg-options "-fcheck=bounds" }
+! { dg-do run }
+program rabbithole
+implicit none
+character(len=:),allocatable    :: text_block(:)
+integer :: i
+integer :: ii
+character(len=10)   :: cten='abcdefghij'
+character(len=20)   :: ctwenty='abcdefghijabcdefghij'
+ii=-6
+text_block=[ character(len=ii) :: cten, ctwenty ]
+write(*,*)'WRITE IT'
+write(*,'(a)')(trim(text_block(i)),i=1,size(text_block))
+end program rabbithole


Thanks,
Elizebeth