* [PATCH] fortran/50514 -- Fix static chekcing of ISHFT[C] arguments.
@ 2011-10-16 7:59 Steve Kargl
2011-10-17 10:30 ` Tobias Burnus
0 siblings, 1 reply; 4+ messages in thread
From: Steve Kargl @ 2011-10-16 7:59 UTC (permalink / raw)
To: fortran, gcc-patches
[-- Attachment #1: Type: text/plain, Size: 560 bytes --]
This patch fixes the checking of constant SHIFT values against
the bitsize of the argument to be shifted. I've been running
this patch on amd64-*-freebsd for sevearl days without a
problem (ie., multiple 'gmake check-gfortran' invocations).
OK for trunk?
2011-10-15 Steven G. Kargl <kargl@gcc.gnu.org>
* gfortran.dg/ishft_3.f90: Update test.
2011-10-15 Steven G. Kargl <kargl@gcc.gnu.org>
* check.c (less_than_bitsize1): Check |shift| <= bit_size(i).
(gfc_check_ishftc): Check |shift| <= bit_size(i) and check
that size is positive.
--
Steve
[-- Attachment #2: ishft.diff --]
[-- Type: text/x-diff, Size: 3663 bytes --]
Index: testsuite/gfortran.dg/ishft_3.f90
===================================================================
--- testsuite/gfortran.dg/ishft_3.f90 (revision 179208)
+++ testsuite/gfortran.dg/ishft_3.f90 (working copy)
@@ -1,11 +1,38 @@
! { dg-do compile }
+! PR fortran/50514
program ishft_3
- integer i, j
- write(*,*) ishftc( 3, 2, 3 )
- write(*,*) ishftc( 3, 2, i )
- write(*,*) ishftc( 3, i, j )
- write(*,*) ishftc( 3, 128 ) ! { dg-error "exceeds BIT_SIZE of first" }
- write(*,*) ishftc( 3, 0, 128 ) ! { dg-error "exceeds BIT_SIZE of first" }
- write(*,*) ishftc( 3, 0, 0 ) ! { dg-error "Invalid third argument" }
- write(*,*) ishftc( 3, 3, 2 ) ! { dg-error "exceeds third argument" }
+
+ implicit none
+
+ integer j, m
+
+ m = 42
+ !
+ ! These should compile.
+ !
+ j = ishft(m, 16)
+ j = ishft(m, -16)
+ j = ishftc(m, 16)
+ j = ishftc(m, -16)
+ !
+ ! These should issue an error.
+ !
+ j = ishft(m, 640) ! { dg-error "absolute value of SHIFT" }
+ j = ishftc(m, 640) ! { dg-error "absolute value of SHIFT" }
+ j = ishft(m, -640) ! { dg-error "absolute value of SHIFT" }
+ j = ishftc(m, -640) ! { dg-error "absolute value of SHIFT" }
+
+ ! abs(SHIFT) must be <= SIZE
+
+ j = ishftc(m, 1, 2)
+ j = ishftc(m, 1, 2)
+ j = ishftc(m, -1, 2)
+ j = ishftc(m, -1, 2)
+
+ j = ishftc(m, 10, 2)! { dg-error "absolute value of SHIFT" }
+ j = ishftc(m, 10, 2)! { dg-error "absolute value of SHIFT" }
+ j = ishftc(m, -10, 2)! { dg-error "absolute value of SHIFT" }
+ j = ishftc(m, -10, 2)! { dg-error "absolute value of SHIFT" }
+
+ j = ishftc(m, 1, -2) ! { dg-error "must be positive" }
end program
Index: fortran/check.c
===================================================================
--- fortran/check.c (revision 179208)
+++ fortran/check.c (working copy)
@@ -318,6 +318,22 @@ less_than_bitsize1 (const char *arg1, gf
{
gfc_extract_int (expr2, &i2);
i3 = gfc_validate_kind (BT_INTEGER, expr1->ts.kind, false);
+
+ /* For ISHFT[C], |shift| <= bit_size(i). */
+ if (strncmp (arg2, "ISHFT", 5) == 0)
+ {
+ if (i2 < 0)
+ i2 = -i2;
+
+ if (i2 > gfc_integer_kinds[i3].bit_size)
+ {
+ gfc_error ("The absolute value of SHIFT at %L must be less "
+ "than or equal to BIT_SIZE('%s')",
+ &expr2->where, arg1);
+ return FAILURE;
+ }
+ }
+
if (or_equal)
{
if (i2 > gfc_integer_kinds[i3].bit_size)
@@ -1961,6 +1977,9 @@ gfc_check_ishft (gfc_expr *i, gfc_expr *
|| type_check (shift, 1, BT_INTEGER) == FAILURE)
return FAILURE;
+ if (less_than_bitsize1 ("I", i, "ISHFT", shift, true) == FAILURE)
+ return FAILURE;
+
return SUCCESS;
}
@@ -1972,7 +1991,35 @@ gfc_check_ishftc (gfc_expr *i, gfc_expr
|| type_check (shift, 1, BT_INTEGER) == FAILURE)
return FAILURE;
- if (size != NULL && type_check (size, 2, BT_INTEGER) == FAILURE)
+ if (size != NULL)
+ {
+ int i2, i3;
+
+ if (type_check (size, 2, BT_INTEGER) == FAILURE)
+ return FAILURE;
+
+ if (less_than_bitsize1 ("I", i, "SIZE", size, true) == FAILURE)
+ return FAILURE;
+
+ gfc_extract_int (size, &i3);
+ if (i3 <= 0)
+ {
+ gfc_error ("SIZE at %L must be positive", &size->where);
+ return FAILURE;
+ }
+
+ gfc_extract_int (shift, &i2);
+ if (i2 < 0)
+ i2 = -i2;
+
+ if (i2 > i3)
+ {
+ gfc_error ("The absolute value of SHIFT at %L must be less than "
+ "or equal to SIZE at %L", &shift->where, &size->where);
+ return FAILURE;
+ }
+ }
+ else if (less_than_bitsize1 ("I", i, "ISHFTC", shift, true) == FAILURE)
return FAILURE;
return SUCCESS;
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] fortran/50514 -- Fix static chekcing of ISHFT[C] arguments.
2011-10-16 7:59 [PATCH] fortran/50514 -- Fix static chekcing of ISHFT[C] arguments Steve Kargl
@ 2011-10-17 10:30 ` Tobias Burnus
2011-10-17 17:35 ` Steve Kargl
0 siblings, 1 reply; 4+ messages in thread
From: Tobias Burnus @ 2011-10-17 10:30 UTC (permalink / raw)
To: Steve Kargl; +Cc: fortran, gcc-patches
On 10/15/2011 11:21 PM, Steve Kargl wrote:
> OK for trunk?
>
> 2011-10-15 Steven G. Kargl<kargl@gcc.gnu.org>
>
> * gfortran.dg/ishft_3.f90: Update test.
I am not so happy with complete test replacements. How about adding it
as new test case?
> 2011-10-15 Steven G. Kargl<kargl@gcc.gnu.org>
>
> * check.c (less_than_bitsize1): Check|shift| <= bit_size(i).
> (gfc_check_ishftc): Check|shift| <= bit_size(i) and check
> that size is positive.
I somehow find less_than_bitsize1's
+ if (strncmp (arg2, "ISHFT", 5) == 0)
not that elegant and would prefer another argument, which tells the
function that it should take the absolute value of the argument;
however, given that ISHFT seems to be the only function which allows
negative arguments, one could also leave it.
OK with considering the two remarks. Thanks for the patch!
Tobias
PS: As you have probably seen, I found a related issue regarding
DSHIFTL/DSHIFTR: PR 50753.
> Index: testsuite/gfortran.dg/ishft_3.f90
> ===================================================================
> --- testsuite/gfortran.dg/ishft_3.f90 (revision 179208)
> +++ testsuite/gfortran.dg/ishft_3.f90 (working copy)
> @@ -1,11 +1,38 @@
> ! { dg-do compile }
> +! PR fortran/50514
> program ishft_3
> - integer i, j
> - write(*,*) ishftc( 3, 2, 3 )
> - write(*,*) ishftc( 3, 2, i )
> - write(*,*) ishftc( 3, i, j )
> - write(*,*) ishftc( 3, 128 ) ! { dg-error "exceeds BIT_SIZE of first" }
> - write(*,*) ishftc( 3, 0, 128 ) ! { dg-error "exceeds BIT_SIZE of first" }
> - write(*,*) ishftc( 3, 0, 0 ) ! { dg-error "Invalid third argument" }
> - write(*,*) ishftc( 3, 3, 2 ) ! { dg-error "exceeds third argument" }
> +
> + implicit none
> +
> + integer j, m
> +
> + m = 42
> + !
> + ! These should compile.
> + !
> + j = ishft(m, 16)
> + j = ishft(m, -16)
> + j = ishftc(m, 16)
> + j = ishftc(m, -16)
> + !
> + ! These should issue an error.
> + !
> + j = ishft(m, 640) ! { dg-error "absolute value of SHIFT" }
> + j = ishftc(m, 640) ! { dg-error "absolute value of SHIFT" }
> + j = ishft(m, -640) ! { dg-error "absolute value of SHIFT" }
> + j = ishftc(m, -640) ! { dg-error "absolute value of SHIFT" }
> +
> + ! abs(SHIFT) must be<= SIZE
> +
> + j = ishftc(m, 1, 2)
> + j = ishftc(m, 1, 2)
> + j = ishftc(m, -1, 2)
> + j = ishftc(m, -1, 2)
> +
> + j = ishftc(m, 10, 2)! { dg-error "absolute value of SHIFT" }
> + j = ishftc(m, 10, 2)! { dg-error "absolute value of SHIFT" }
> + j = ishftc(m, -10, 2)! { dg-error "absolute value of SHIFT" }
> + j = ishftc(m, -10, 2)! { dg-error "absolute value of SHIFT" }
> +
> + j = ishftc(m, 1, -2) ! { dg-error "must be positive" }
> end program
>
> Index: fortran/check.c
> ===================================================================
> --- fortran/check.c (revision 179208)
> +++ fortran/check.c (working copy)
> @@ -318,6 +318,22 @@ less_than_bitsize1 (const char *arg1, gf
> {
> gfc_extract_int (expr2,&i2);
> i3 = gfc_validate_kind (BT_INTEGER, expr1->ts.kind, false);
> +
> + /* For ISHFT[C],|shift| <= bit_size(i). */
> + if (strncmp (arg2, "ISHFT", 5) == 0)
> + {
> + if (i2< 0)
> + i2 = -i2;
> +
> + if (i2> gfc_integer_kinds[i3].bit_size)
> + {
> + gfc_error ("The absolute value of SHIFT at %L must be less "
> + "than or equal to BIT_SIZE('%s')",
> + &expr2->where, arg1);
> + return FAILURE;
> + }
> + }
> +
> if (or_equal)
> {
> if (i2> gfc_integer_kinds[i3].bit_size)
> @@ -1961,6 +1977,9 @@ gfc_check_ishft (gfc_expr *i, gfc_expr *
> || type_check (shift, 1, BT_INTEGER) == FAILURE)
> return FAILURE;
>
> + if (less_than_bitsize1 ("I", i, "ISHFT", shift, true) == FAILURE)
> + return FAILURE;
> +
> return SUCCESS;
> }
>
> @@ -1972,7 +1991,35 @@ gfc_check_ishftc (gfc_expr *i, gfc_expr
> || type_check (shift, 1, BT_INTEGER) == FAILURE)
> return FAILURE;
>
> - if (size != NULL&& type_check (size, 2, BT_INTEGER) == FAILURE)
> + if (size != NULL)
> + {
> + int i2, i3;
> +
> + if (type_check (size, 2, BT_INTEGER) == FAILURE)
> + return FAILURE;
> +
> + if (less_than_bitsize1 ("I", i, "SIZE", size, true) == FAILURE)
> + return FAILURE;
> +
> + gfc_extract_int (size,&i3);
> + if (i3<= 0)
> + {
> + gfc_error ("SIZE at %L must be positive",&size->where);
> + return FAILURE;
> + }
> +
> + gfc_extract_int (shift,&i2);
> + if (i2< 0)
> + i2 = -i2;
> +
> + if (i2> i3)
> + {
> + gfc_error ("The absolute value of SHIFT at %L must be less than "
> + "or equal to SIZE at %L",&shift->where,&size->where);
> + return FAILURE;
> + }
> + }
> + else if (less_than_bitsize1 ("I", i, "ISHFTC", shift, true) == FAILURE)
> return FAILURE;
>
> return SUCCESS;
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] fortran/50514 -- Fix static chekcing of ISHFT[C] arguments.
2011-10-17 10:30 ` Tobias Burnus
@ 2011-10-17 17:35 ` Steve Kargl
2011-10-18 11:02 ` Tobias Burnus
0 siblings, 1 reply; 4+ messages in thread
From: Steve Kargl @ 2011-10-17 17:35 UTC (permalink / raw)
To: Tobias Burnus; +Cc: fortran, gcc-patches
On Mon, Oct 17, 2011 at 12:22:03PM +0200, Tobias Burnus wrote:
> On 10/15/2011 11:21 PM, Steve Kargl wrote:
> >OK for trunk?
> >
> >2011-10-15 Steven G. Kargl<kargl@gcc.gnu.org>
> >
> > * gfortran.dg/ishft_3.f90: Update test.
>
> I am not so happy with complete test replacements. How about adding it
> as new test case?
>
Well, the old testcase is
%cat ishft_3.f90
! { dg-do compile }
program ishft_3
integer i, j
write(*,*) ishftc( 3, 2, 3 )
write(*,*) ishftc( 3, 2, i )
write(*,*) ishftc( 3, i, j )
write(*,*) ishftc( 3, 128 ) ! { dg-error "exceeds BIT_SIZE of first" }
write(*,*) ishftc( 3, 0, 128 ) ! { dg-error "exceeds BIT_SIZE of first" }
write(*,*) ishftc( 3, 0, 0 ) ! { dg-error "Invalid third argument" }
write(*,*) ishftc( 3, 3, 2 ) ! { dg-error "exceeds third argument" }
end program
1) It's misnamed. There is no testing of ishft.
2) i and j are undefined, so lines 4 and 5 are invalid.
Even if i and j were defined, there is nothing special
about those lines in that gfortran generates a function
call to a runtime routine. Note, this a "dg-do compile"
test.
3) The four dg-error strings would need to be updated to
the new error messages.
The only line that would survive is the first line, which
is covered in the updated testcase.
> >2011-10-15 Steven G. Kargl<kargl@gcc.gnu.org>
> >
> > * check.c (less_than_bitsize1): Check|shift| <= bit_size(i).
> > (gfc_check_ishftc): Check|shift| <= bit_size(i) and check
> > that size is positive.
>
> I somehow find less_than_bitsize1's
>
> + if (strncmp (arg2, "ISHFT", 5) == 0)
>
> not that elegant and would prefer another argument, which tells the
> function that it should take the absolute value of the argument;
> however, given that ISHFT seems to be the only function which allows
> negative arguments, one could also leave it.
>
In looking at the other uses of less_than_bitsize1() I
could pass arg2=NULL for ISHFT[C], and then the code
would become
if (arg2 == NULL) { /* Special case for ISHFT[C]. */
Would that be better?
--
Steve
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] fortran/50514 -- Fix static chekcing of ISHFT[C] arguments.
2011-10-17 17:35 ` Steve Kargl
@ 2011-10-18 11:02 ` Tobias Burnus
0 siblings, 0 replies; 4+ messages in thread
From: Tobias Burnus @ 2011-10-18 11:02 UTC (permalink / raw)
To: Steve Kargl; +Cc: gfortran, gcc patches
On 10/17/2011 07:02 PM, Steve Kargl wrote:
> On Mon, Oct 17, 2011 at 12:22:03PM +0200, Tobias Burnus wrote:
>> I am not so happy with complete test replacements. How about adding it
>> as new test case?
> Well, the old testcase is
> [...]
>
> The only line that would survive is the first line, which
> is covered in the updated testcase.
Convinced: Do the replacement.
>> I somehow find less_than_bitsize1's
>>
>> + if (strncmp (arg2, "ISHFT", 5) == 0)
>>
>> not that elegant and would prefer another argument, which tells the
>> function that it should take the absolute value of the argument;
>> however, given that ISHFT seems to be the only function which allows
>> negative arguments, one could also leave it.
> In looking at the other uses of less_than_bitsize1() I
> could pass arg2=NULL for ISHFT[C], and then the code
> would become
>
> if (arg2 == NULL) { /* Special case for ISHFT[C]. */
>
> Would that be better?
It would be faster - but not necessarily better. I was rather thinking
of adding another argument like "bool abs_ok".
However, I think all three solutions are OK. Pick one.
Tobias
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2011-10-18 9:58 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-10-16 7:59 [PATCH] fortran/50514 -- Fix static chekcing of ISHFT[C] arguments Steve Kargl
2011-10-17 10:30 ` Tobias Burnus
2011-10-17 17:35 ` Steve Kargl
2011-10-18 11:02 ` Tobias Burnus
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).