Add cross-dimension shape validation for diff prepend/append#1152
Add cross-dimension shape validation for diff prepend/append#1152srinjoy933 wants to merge 3 commits intofortran-lang:masterfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Adds explicit cross-dimension (non-differencing dimension) shape validation for diff_2’s optional prepend/append arguments to prevent out-of-bounds assignment/memory corruption scenarios described in #1151.
Changes:
- Validate
size(prepend, 3-dim_) == size(x, 3-dim_)before usingprependin the internal workspace. - Validate
size(append, 3-dim_) == size(x, 3-dim_)before usingappendin the internal workspace.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/math/stdlib_math_diff.fypp
Outdated
| error stop "stdlib_math_diff: non-differencing dimension of 'prepend' must match 'x'" | ||
| end if | ||
| size_prepend = size(prepend, dim_) | ||
| end if | ||
|
|
||
| if (present(append)) then | ||
| if (size(append, 3 - dim_) /= size(x, 3 - dim_)) then | ||
| error stop "stdlib_math_diff: non-differencing dimension of 'append' must match 'x'" |
There was a problem hiding this comment.
error stop is an image control statement and is not permitted inside a pure procedure by the Fortran standard. Since diff_2_* is declared pure, these new error stop statements can make the code non-conforming (and may fail to compile with stricter compilers). Consider replacing this with a pure-friendly error reporting mechanism (e.g., return a zero-sized result and/or add an optional status/state argument), or drop pure consistently (including the public interface in stdlib_math.fypp) if terminating is required.
| error stop "stdlib_math_diff: non-differencing dimension of 'prepend' must match 'x'" | |
| end if | |
| size_prepend = size(prepend, dim_) | |
| end if | |
| if (present(append)) then | |
| if (size(append, 3 - dim_) /= size(x, 3 - dim_)) then | |
| error stop "stdlib_math_diff: non-differencing dimension of 'append' must match 'x'" | |
| allocate(y(0, 0)) | |
| return | |
| end if | |
| size_prepend = size(prepend, dim_) | |
| end if | |
| if (present(append)) then | |
| if (size(append, 3 - dim_) /= size(x, 3 - dim_)) then | |
| allocate(y(0, 0)) | |
| return |
| if (present(append)) then | ||
| if (size(append, 3 - dim_) /= size(x, 3 - dim_)) then | ||
| error stop "stdlib_math_diff: non-differencing dimension of 'append' must match 'x'" | ||
| end if | ||
| size_append = size(append, dim_) |
There was a problem hiding this comment.
Same concern as above: introducing error stop inside pure diff_2_* is non-conforming and can break compilation. If the intent is to hard-fail on invalid append shapes, the procedure (and its public interface) likely needs to be made non-pure, or the API should be adjusted to report shape errors without image control statements.
| if (present(prepend)) then | ||
| if (size(prepend, 3 - dim_) /= size(x, 3 - dim_)) then | ||
| error stop "stdlib_math_diff: non-differencing dimension of 'prepend' must match 'x'" | ||
| end if | ||
| size_prepend = size(prepend, dim_) | ||
| end if | ||
|
|
||
| if (present(append)) then | ||
| if (size(append, 3 - dim_) /= size(x, 3 - dim_)) then | ||
| error stop "stdlib_math_diff: non-differencing dimension of 'append' must match 'x'" | ||
| end if | ||
| size_append = size(append, dim_) |
There was a problem hiding this comment.
This adds new shape-validation behavior for diff_2 when prepend/append are present, but there doesn't appear to be a regression test covering the mismatched cross-dimension case that previously caused bounds issues. Consider adding a dedicated test that exercises a mismatched prepend/append shape and asserts failure (e.g., via a CTest WILL_FAIL test) so the bug doesn’t regress.
This PR introduces explicit cross-dimension shape validation for the prepend and append arguments in the diff_2 routine. Previously, passing an array with a mismatched non-differencing dimension bypassed shape checks, leading to potential bounds-check faults or silent memory corruption during the internal work array assignment. The update verifies size(array, 3 - dim_) == size(x, 3 - dim_) prior to workspace allocation, ensuring safe execution.
Solves #1151