fix(trbpsd): size temp/qp to plasmaf%nrmax to avoid OOB (#203)#210
Open
HengyuLi-Ozaki-lab wants to merge 1 commit into
Open
fix(trbpsd): size temp/qp to plasmaf%nrmax to avoid OOB (#203)#210HengyuLi-Ozaki-lab wants to merge 1 commit into
HengyuLi-Ozaki-lab wants to merge 1 commit into
Conversation
…-yoshimi#203 bug-B) tr_bpsd_get declared temp(nrmp,nsm,3) and tempx(nrmp,17) with the compile-time bound nrmp = NRMAX+1. For file-baked fixtures such as eqdata.TST-2, bpsd_get_data(plasmaf) returns plasmaf%nrmax = 52 while nrmp = 51, so the do nr=1,plasmaf%nrmax copy loop wrote temp(52,...) one row past the end. Likewise the do nr=2,plasmaf%nrmax loop wrote qp(51) while QP is allocated to size NRMAX (=50). Fix: make temp/tempx ALLOCATABLE and size them to max(plasmaf%nrmax, nrmp) after bpsd_get_data, and clamp the qp(nr-1) write loop to min(plasmaf%nrmax, nrmax+1). These were two of the upstream OOBs that silently corrupted the heap under release builds and produced the macOS SIGABRT signature in k-yoshimi#194. Verified: with this plus the bpsd_plasmaf intent(out) fix (bug-C), tr_tst2 runs cleanly under -fbounds-check -fcheck=all on macOS, the Newton solve converges, and the output matches the prior silent-OOB release build to rel err 1.25e-11 (pure float reassociation). Local patch on kyoshimi-develop; cherry-picked to a clean PR branch for k-yoshimi#203. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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
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.
Summary
Fixes the
trbpsd.f90half of the out-of-bounds cascade tracked in #203 (bug-B).tr_bpsd_getdeclared its work arrays with the compile-time boundnrmp = NRMAX + 1:For a file-baked equilibrium fixture such as
eqdata.TST-2,bpsd_get_data(plasmaf)returnsplasmaf%nrmax = 52whilenrmp = 51(default
NRMAX = 50). The copy loop then writestemp(52,...)one rowpast the end, and the
do nr=2,plasmaf%nrmaxloop writesqp(51)whileQPis allocated to sizeNRMAX(= 50).NRMAXis not exposed throughthe TR
set_paramregistry, so a caller cannot side-step this by shrinkingthe mesh.
Under release builds these writes corrupted the heap silently; on macOS
hardened-malloc they surfaced as the non-deterministic
free_list_checksum_botchSIGABRT in #194.Fix
temp/tempxALLOCATABLEand size them tomax(plasmaf%nrmax, nrmp)afterbpsd_get_data.qp(nr-1)write loop tomin(plasmaf%nrmax, nrmax+1)so itnever exceeds the
QP(NRMAX)allocation.Evidence it is numerically correct (not just crash-suppressing)
A controlled rebuild compared a reference build with this fix + the
bpsd_get_plasmafintent(out) fix (bug-C) and-fbounds-check -fcheck=allON, against a release build that lets the OOB go silent.
tr_tst2outputwas compared pointwise across the full state:
-O2/-O0float reassociation,not the OOB.
The one-past-end
temp(52)/qp(51)writes land in memory the physicsnever reads back, and the two extra boundary points were never consumed
(
mesh_convert_gtomuses only the inner mesh). With the fix,tr_tst2runs to completion under bounds-check on macOS with the Newton solve
converging normally.
Related
bpsdrepo (bpsd_get_plasmafunconditional intent(out)
nrmax); the two together close the tr/trbpsd.f90 + bpsd/bpsd_plasmaf.f90 OOB cascade with eqdata.TST-2 (follow-up to #194) #203cascade.
bpsd_specieskidstride-3 OOB) is separate and tracked atbpsd_setup_species_kdata writes past 'kid' upper bound when nsmax not multiple of 3 bpsd#6.
Test plan
-fbounds-check -fcheck=all.tr_tst2) and confirm it completeswith no Fortran runtime bounds error.
to ~1e-11).