Skip to content

Fix unit test dependencies on Neko library - #2755

Open
timfelle wants to merge 5 commits into
ExtremeFLOW:developfrom
timfelle:worktree-fix-unit-test-libneko-dependency
Open

Fix unit test dependencies on Neko library#2755
timfelle wants to merge 5 commits into
ExtremeFLOW:developfrom
timfelle:worktree-fix-unit-test-libneko-dependency

Conversation

@timfelle

@timfelle timfelle commented Sep 3, 2026

Copy link
Copy Markdown
Collaborator

Update unit test Makefiles to ensure test objects rebuild when the Neko library changes, preventing undefined behavior from stale objects. Add self-remake rules to Makefiles for proper regeneration upon changes.

timfelle and others added 4 commits September 3, 2026 08:38
The per-directory unit-test makefiles built the test objects with a bare
`%.o : %.F90` rule and the pFUnit rules only track the `.pf` sources, so
nothing tied a test object to Neko itself. After a change to a Neko
derived type, `make check` therefore kept the previously compiled test
object and linked it against the rebuilt library. The test frame then
lays out its own `type(x_t)` locals with the old component offsets while
the library reads and writes them at the new ones, which is undefined
behaviour.

This is what the reported `gs_free` segfault was: `gs_t` gained
`shared_gs_v`/`shared_gs_v_d` in the middle of its component list, so a
stale test object made `gs_init`'s opening `call gs%free()` deallocate
components through shifted, garbage descriptors. Reproduced exactly, by
padding `gs_t` and rebuilding only `src`:

  #3 __gather_scatter_MOD_gs_free at gs/gather_scatter.f90:602
  #4 __gather_scatter_MOD_gs_init at gs/gather_scatter.f90:199

The gather-scatter code itself is correct; the stale objects are not.

Give the pattern rule a dependency on the Neko library so the test
objects, and hence the suite binaries, follow any change to Neko. The
dependency is taken through `$(wildcard ...)` so it is simply absent,
as before, when the library has not been built yet.

Each of these makefiles also gains the standard autoconf self-remake
rule, which they lacked entirely. Without it a fix to a hand-maintained
`Makefile.in` under `tests/unit` — including this one — never reaches a
build tree that was already configured, because nothing regenerates the
`Makefile` from it. That is also why this commit cannot bootstrap
itself into an existing tree: such a tree has no self-remake rule to
run yet, so it needs `make refresh-unit-test-makefiles` (or a
reconfigure) once, as noted in the CHANGELOG.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The self-remake rule added in the previous commit spelled out each test
directory's own path in its recipe:

    cd @top_builddir@ && \
        $(SHELL) ./config.status tests/unit/point_interpolation/Makefile

That is 46 near-copies of one rule differing only in a string, and the
string is a trap: a new test directory made by copying an existing one
keeps the original's path, so building it would regenerate a different
directory's Makefile while its own stayed stale. Spelling the path out
also pushed thirteen of the recipes past the 80-column limit, which
needed a line continuation to stay inside it.

Derive the subdirectory from the paths make already knows instead. The
block is now byte-identical in all 46 files, the recipe fits on one
line, and it stays correct wherever it is copied.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Merging develop brought in three test directories that postdate the
earlier commits on this branch -- ax, bc_projector and wall_sampling --
so they still had the bare `%.o : %.F90` rule and no self-remake rule.
The fix covered 46 of the 49 pFUnit test directories; a merge cannot
apply it to files it has just introduced.

Give the three the same two blocks as the rest. All 49 are now
byte-identical in both.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants