RE: [PATCH] PR fortran/77505 -- Treat negative character length as LEN=0
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 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 PR fortran/77505 * trans-array.c (trans_array_constructor): Treat negative character length as LEN = 0. 2016-12-01 Elizebeth Punnoose 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
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 PR fortran/77505 * trans-array.c (trans_array_constructor): Treat negative character length as LEN = 0. 2016-12-01 Elizebeth Punnoose 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 Index: gcc/fortran/trans-array.c === --- gcc/fortran/trans-array.c (revision 243134) +++ gcc/fortran/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,29 @@ trans_array_constructor (gfc_ss * ss, lo gfc_conv_expr_type (&length_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 it is, then + set 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 treated as LEN = 0"); + gfc_trans_runtime_check (false, true, neg_len, &length_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, + &length_se.pre); + gfc_add_block_to_block (&outer_loop->pre, &length_se.pre); gfc_add_block_to_block (&outer_loop->post, &length_se.post); } Index: gcc/testsuite/gfortran.dg/char_length_20.f90 === --- gcc/testsuite/gfortran.dg/char_length_20.f90 (nonexistent) +++ gcc/testsuite/gfortran.dg/char_length_20.f90 (working copy) @@ -0,0 +1,13 @@ +! { dg-do run } +! { dg-options "-fcheck=bounds" } +program rabbithole + implicit none + character(len=:), allocatable :: text_block(:) + integer i, ii + character(len=10) :: cten='abcdefghij' + character(len=20) :: ctwenty='abcdefghijabcdefghij' + ii = -6 + text_block=[ character(len=ii) :: cten, ctwenty ] + if (any(len_trim(text_block) /= 0)) call abort +end program rabbithole +! { dg-output "At line 10 of file .*char_length_20.f90.*Fortran runtime warning: Negative character length treated as LEN = 0" } Index: gcc/testsuite/gfortran.dg/char_length_21.f90 === --- gcc/testsuite/gfortran.dg/char_length_21.f90 (nonexistent) +++ gcc/testsuite/gfortran.dg/char_length_21.f90 (working copy) @@ -0,0 +1,11 @@ +! { dg-do run } +program rabbithole + implicit none + character(len=:), allocatable :: text_block(:) + integer i, ii + character(len=10) :: cten='abcdefghij' + character(len=20) :: ctwenty='abcdefghijabcdefghij' + ii = -6 + text_block = [character(len=ii) :: cten, ctwenty] + if (any(len_trim(text_block) /= 0)) call abort +end program rabbithole
RE: [PATCH] PR fortran/77505 -- Treat negative character length as LEN=0
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 (&length_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, &length_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, + &length_se.pre); + gfc_add_block_to_block (&outer_loop->pre, &length_se.pre); gfc_add_block_to_block (&outer_loop->post, &length_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