Review of MR 9579

HIGH

1. src/ksp/pc/impls/asm/asm.c:662 — PCReset_ASM() frees osm->scaling without NULLing it

Description: The added reset block destroys each scaling Vec and frees the array but leaves osm->scaling dangling. Every other consumer tests osm->scaling for presence (PCApply_ASM(), PCMatApply_ASM_Private(), PCApplyTranspose_ASM() error-check, PCASMSetLocalScaling_ASM() replacement path, PCASMGetLocalScaling() return).

Trigger: PCSetUp(), PCASMSetLocalScaling(), PCReset(), then PCSetUp() again followed by PCApply()/PCApplyTranspose() without installing new weights; or two consecutive PCReset() calls (explicit reset followed by PCDestroy(), which calls PCReset_ASM() again).

Impact: Use-after-free on freed scaling Vecs instead of the intended PETSC_ERR_ARG_WRONGSTATE (“Call PCASMSetLocalScaling()…”), or a double-free crash on the second reset. With a re-setup that changes n_local_true, the stale array length also mismatches the new subdomain count.

Suggested fix: NULL the pointer after freeing, matching the osm->is = NULL; osm->is_local = NULL; pattern in the same routine:

  if (osm->scaling) {
    for (PetscInt i = 0; i < osm->n_local_true; i++) PetscCall(VecDestroy(&osm->scaling[i]));
    PetscCall(PetscFree(osm->scaling));
    osm->scaling = NULL;
  }

MEDIUM

2. src/ksp/pc/impls/asm/ftn-custom/zasmf.c:46 — pcasmgetlocalscaling_() builds a Fortran array descriptor from NULL when no weights are set

Description: The new getter wrapper calls F90Array1dCreate(tscaling, MPIU_FORTRANADDR, 1, nloc, scaling) unconditionally. The C getter documents scaling == NULL as the valid “none supplied” state and still returns n = n_local_true (> 0 after setup), so a valid query produces a length-nloc descriptor over a NULL base.

Trigger: Fortran caller invokes PCASMGetLocalScaling() after PCSetUp() but before PCASMSetLocalScaling() (or after PCReset()), which is the documented way to test presence.

Impact: The resulting Fortran pointer references address zero with nonzero extent; any access segfaults, and descriptor creation itself is invalid. Sibling wrappers (pcasmgetlocalsubmatrices_, pcasmgetlocalsubdomains_) never face a NULL-valid-after-setup case, so the copied pattern is unsafe here.

Suggested fix: Guard the NULL case before descriptor creation, e.g. return n = 0 with an empty descriptor when tscaling == NULL, or only call F90Array1dCreate() when tscaling is non-NULL and otherwise return a zero-length array.

Coverage

Reviewed mr-9579-diff.txt through line 818: 15 files. No binary files (Binary files ... differ / GIT binary patch) encountered, so no paths excluded as not covered. .out reference bodies (src/ksp/ksp/tutorials/output/ex8_weighted.out, ex8_weighted_zero.out) were not reviewed for content per procedure; code-to-output consistency (shared basic/weighted output, zero-weights test) was checked by test-block inspection only.

(1 LOW finding suppressed; ask to show them.)

Footnote: AI_CLI=openCode, model=muse-spark-1.3-contributor-free (Meta Muse Spark via OpenCode); effort ($CLAUDE_EFFORT)=unavailable; date=2026-09-09, time=04:52 UTC; MR_IID=9579; CI_PIPELINE_ID=unavailable; CI_JOB_ID=unavailable. DRIFT=unknown (no local ref for source branch; continued per procedure). No WARNING lines were printed by the fetch script.