Silence two static array-bounds warnings on unreachable code - #65
Open
Sohex wants to merge 1 commit into
Open
Conversation
A stock build emits three `Array reference at (1) out of bounds` warnings from
gfortran's static analysis at -O3. Both sites are guarded by conditions that are
false at compile time for the default configuration, so neither reference is
reachable and neither is a bug -- but the warnings are the only ones the build
produces, so a genuinely new one would land in noise rather than in an empty
log.
oceanmod.f90, three warnings. The block
if(NLEV_OCE > 1) then
do jlev=1,nlem_oce
vdiffk(jlev)=(dlayer(jlev)*vdiffkl(jlev)
+dlayer(jlev+1)*vdiffkl(jlev+1))
/(dlayer(jlev)+dlayer(jlev+1))
enddo
endif
indexes dlayer(2) where dlayer is dlayer(NLEV_OCE) and NLEV_OCE is a parameter
equal to 1. It cannot execute: the guard is false at compile time, and
nlem_oce = NLEV_OCE - 1 = 0 makes the loop zero-trip as well. The body is
correct for any NLEV_OCE > 1; gfortran simply does not use the guard to prune it
before bounds analysis.
nlem_oce is derived from a parameter and is never assigned anywhere in the file,
so declaring it a parameter is what it already is, and it lets gfortran prove
the trip count is zero. The three warnings go.
plasim.f90, one warning. The neqsig==5 branch is guarded by NLEV > 10 but its
loop runs jlev = NLEV-10 .. NLEV, which reaches sigmah(0) when NLEV = 10 -- the
case the guard excludes. max(NLEV-10,1) on the lower bound never changes the
range where the branch runs, and is the idiom the same block already uses two
lines below:
zskf = sigmah(max(NLEV-10,1)) !Have to add this max statement so it'll compile with debug flag
With both, the model builds with zero warnings.
Neither change can alter behaviour: both touch code that does not execute in the
configurations that reach it. Verified at T42 L10 on 16 ranks, gfortran 16.2.1 --
the restart checksum is byte-identical to a build without these changes.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ET5NPfmc2UhKtbdLeuNaWH
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A stock build emits three
Array reference at (1) out of boundswarnings from gfortran's static analysis at-O3. They are the only warnings the build produces, so a genuinely new one would land in noise rather than in an empty log.Neither site is a bug. Both are guarded by conditions that are false at compile time for the default configuration, so neither reference is reachable — gfortran just doesn't use the guard to prune the body before bounds analysis.
oceanmod.f90— three warningsThis indexes
dlayer(2)wheredlayerisdlayer(NLEV_OCE)andNLEV_OCEis a parameter equal to 1. It cannot execute — the guard is false at compile time, andnlem_oce = NLEV_OCE - 1 = 0makes the loop zero-trip as well. The body is correct for anyNLEV_OCE > 1.nlem_oceis derived from a parameter and is never assigned anywhere in the file, so declaring itparameteris just stating what it already is, and it lets gfortran prove the trip count is zero.plasim.f90— one warningThe
neqsig==5branch is guarded byNLEV > 10, but its loop runsjlev = NLEV-10 .. NLEV, which reachessigmah(0)whenNLEV = 10— precisely the case the guard excludes.max(NLEV-10,1)on the lower bound never changes the range where the branch actually runs, and it is the idiom this same block already uses two lines below:Verification
Neither change can alter behaviour — both touch code that does not execute in the configurations that reach it. Verified at T42 L10 on 16 ranks, gfortran 16.2.1: the restart checksum is byte-identical to a build without these changes. With both applied, the model builds with zero warnings.
Independent of #64, which touches
legmod.f90only; the two can merge in either order.