[sunnyrishy]

Summary

test_package crashes on Windows before it can run a single assertion. The
cause is a stack overflow: EosPackageSet is ~3.8 MB and six of the seven test
functions declared one as an ordinary stack local. Linux's 8 MB default stack
absorbs it, which is why CI has never seen this; Windows gives a thread 1 MB.

Type of Change

• fix — Bug fix
• test — Add or fix tests

Changes

• tests/test_package.c — declare the six EosPackageSet locals static, and
  document why in one comment at the top of the file.

The defect

ctest reports a bare SEGFAULT with no output, since the fault happens on entry
to the first test:

21/21 Test #21: test_package .....................***Exception: SegFault

gdb puts the fault in the stack-probe helper, which is the signature of a stack
overflow rather than a bad pointer:

Thread 1 received signal SIGSEGV, Segmentation fault.
#0  ___chkstk_ms ()
#1  test_package_register ()
#2  main ()

The size is the whole story. EosPackageSet embeds EOS_MAX_PACKAGES (128)
EosPackage entries at 31072 bytes each:

EosPackage=31072  EosPackageSet=3977224 bytes (3.79 MB)

3.79 MB against a 1 MB stack faults immediately and unconditionally.

Approach

Declare the sets static.

This is not an arbitrary choice — it is the convention the codebase already
uses. Production code in cmd/eos/main.c declares static EosConfig and
static EosPackageSet at all eleven sites it touches these types. Whoever wrote
that already knew these structs are too large for the stack; the tests are the
only place that omits it, and the tests are exactly where it crashes.

static is safe here because every affected function already begins with
memset(&set, 0, sizeof(set)). Re-initialization is explicit and
unconditional, so no state carries between tests and no test ordering
dependency is introduced. I verified all six sites individually rather than
assuming.

Testing

• Unit tests pass
• New tests added for new functionality — n/a, this repairs an existing test

Native build, GCC 15.2 (MinGW-w64), -DEOS_BUILD_TESTS=ON:

	Before	After
test_package	SIGSEGV, 0 assertions run	all 7 pass
Full suite	20/21	21/21

=== EoS Package Tests ===
[PASS] package register
[PASS] package find
[PASS] package find not found
[PASS] package find null set
[PASS] package register multiple
[PASS] package init all
[PASS] package build type

=== ALL 7 PACKAGE TESTS PASSED ===

The test itself is the regression test: on any 1 MB-stack host it fails before
this change and passes after, with no new test scaffolding required.

Considerations and limitations

• This fixes the crash, not the underlying design question. A ~3.8 MB
  by-value struct is worth a second look in a project whose target is embedded
  systems, where a task stack is measured in kilobytes rather than megabytes.
  Anything that puts an EosPackageSet on a thread stack will fault on real
  hardware far more readily than it does on a desktop. The host tooling
  (cmd/eos) can afford static; a device may not be able to afford the object
  at all.
• I deliberately kept that out of this PR. Shrinking EosPackage, reducing
  EOS_MAX_PACKAGES, or moving to a caller-provided/heap allocation are all
  public-API changes with real ABI consequences, and they should be a designed
  decision by the maintainers rather than a side effect of a test fix. Glad to
  open a separate issue to discuss it if that is useful.
• Why CI did not catch this: the crash needs a stack smaller than 3.8 MB, so
  it is invisible to Linux runners at default limits. A Windows job, or
  ulimit -s 1024 on the existing Linux job, would surface it.

Pre-Submission Checklist

• Code compiles without warnings — no new warnings introduced
• All existing tests pass — 21/21, up from 20/21
• New tests added for new functionality — n/a, this repairs an existing test
• Documentation updated if API changed — n/a, no API change
• Commit message follows <type>(<scope>): <description>
• Branch is rebased on latest master (0 commits behind)

Related Issues

None. Found while building and running the suite on Windows.

[srpatcha]

Review — verified, ready to merge

Built merged onto current master: 24/24 ctest pass.

Correct diagnosis and the right fix. sizeof(EosPackageSet) is ~3.9 MB, which overflows the default 1 MB thread stack on Windows and any small-stack host, and cmd/eos/main.c already declares the same type static for the same reason.

Of the three PRs that make this change, yours is the one I'd merge, because it is the only one that explains why in a comment — including the note that each test memsets on entry so nothing carries over between tests. That comment is what stops someone "cleaning up" the static in six months.

#52 and the same hunk in #60 are byte-identical to yours minus the comment (I compared blob hashes). See my notes there — neither needs to be closed, they just rebase to nothing once this lands.

[sunnyrishy]

Thanks for the detailed review, and for taking the time to diff the three versions. Glad the comment earned its place.

Small heads-up: this one is still showing as open on my end. It looks like the verdict came through as an issue comment rather than a submitted review, so GitHub still has it at REVIEW_REQUIRED and is blocking the merge. The other two (ebuild#45, eBoot#36) went through with a submitted approval, which is what makes me think this one just slipped through.

No rush at all, flagging it in case it wasn't intentional. The branch still merges cleanly against current master (no conflicts), so nothing needs rebasing on my side.

[srpatcha]

Verified locally against current master: merges clean, builds with no new errors, ctest 24/24.

Same change as #52, and I am taking this one because of the comment. The code edit is identical and self-evidently mechanical; what is not self-evident to the next reader is why — that EosPackageSet is EOS_MAX_PACKAGES * sizeof(EosPackage) at roughly 3.8 MB against a 1 MB default thread stack, so the crash happens before main() runs any assertion. Without that written down, someone reasonably "cleans up" the static back to a local and reintroduces it.

The note that each test memsets its set on entry is the other half worth stating: static normally raises a fair question about state carrying between tests, and it is answered here rather than left to be rediscovered.