Opened 14 years ago

Closed 14 years ago

#221 closed defect (fixed)

IF/AND short-circuit bug

Reported by: Dave Offiler Owned by: Dave Offiler
Priority: normal Milestone: 5.0
Component: ROPP (all) Version: 4.1
Keywords: IF, AND, short-circuit Cc: Ian Culverwell

Description

The GRAS SAF Helpdesk has received a question from Jason Lin

The helpdesk subject is: bug report
The helpdesk description is:

Hi,
There's a bug I found in ROPP and would like to report it back to you.

The issue is about some IF statements in various ROPP modules
assuming "Short-Circuit Evaluation" support by FORTRAN compiler. 
As far as I know, FORTRAN does not guarantee short-circuit
evaluation of .AND. operator. Even though it looks right
grammar-wise to the compiler and does get compiled, different
compilers may give different results. We compiled it with PGI 10.9,
and was given "segmentation fault" error when running the binaries.

The solution is to use "nested" IF statements. So far I've found
and fixed 4 of this kind of bugs in different modules:

In ropp_pp/preprocess/ropp_pp_radiooptic_analysis.f90:
<snip>
And in ropp_io/ropp/ropp_io_read.f90:
<snip>

Jason has spotted some legacy cases (which we should have cleared up in a previous review) of a construct like:

IF ( PRESENT(x) .AND. sometest(x) ) THEN

As Jason notes, the FORTRAN standard does not guarantee that just because the first test fails, the second will not be tested. This is a bad one, because if x is not present, it clearly should not be tested. As it happens, all of our current compilers probably do implement this 'short-circuit', but Jason's PGI v10 does not. Unfortunately, our PGI v10 is still broken due to a license manager issue.

All ROPP code should be scanned for these and similar dangerous constructs and corrected e.g. by nesting the IF statements as appropriate, for ROPP-5 (or potentially as a patch for v4.1 if other users report similar run-time problems.)

Jason asked to confirm that the local changes he suggests allow the code to run normally.

Change history (1)

comment:1 by Ian Culverwell, 14 years ago

Cc: Ian Culverwell added
Resolution: fixed
Status: newclosed

This bug has been fixed in the prototype ROPP50 branch (and therefore in ROPP5.0).

Removed LOGICAL clash IF (PRESENT(ranchk) .AND. ranchk) from ropp_io/ropp/ropp_io_read.f90 (twice).

Removed LOGICAL clash IF ((PRESENT(OutRO)) .AND. (OutRO)) from ropp_pp_radiooptic_analysis.f90 (thrice).

These were the only occurences I (idculv) could find.

Note: See TracTickets for help on using tickets.