Skip to content

tests: split elf coverage into no-reloc control and reloc gate#70

Draft
jlinenkohl wants to merge 1 commit intovarnish:masterfrom
jlinenkohl:pr-u6-elf-test-gating
Draft

tests: split elf coverage into no-reloc control and reloc gate#70
jlinenkohl wants to merge 1 commit intovarnish:masterfrom
jlinenkohl:pr-u6-elf-test-gating

Conversation

@jlinenkohl
Copy link
Copy Markdown
Contributor

@jlinenkohl jlinenkohl commented Apr 2, 2026

Summary

  • Split ELF tests into:
    • no-reloc control path ([ELF][no-reloc])
    • relocation capability gate ([ELF][reloc])

Why

  • Separates stable baseline behavior from dynamic relocation capability checks.

Validation

  • cd tests && bash run_unit_tests.sh -R test_elf

Depends on

Stack Context

  • Final capstone test-gate PR for the relocation stack.

Test Evidence

  • Date: 2026-04-02
  • Branch-level validation source: phase14_audit baseline sweep
  • Full unit harness: cd tests && bash run_unit_tests.sh -> 8/8 passed
  • Integration tinytest lane: (cd guest/tests && bash build.sh) && ./build/tinytest guest/tests/glibc_test -> passed

PR-Scoped Command

@jlinenkohl jlinenkohl marked this pull request as ready for review April 4, 2026 11:07
@fwsGonzo
Copy link
Copy Markdown
Member

fwsGonzo commented Apr 4, 2026

I don't quite understand this PR. Most unit tests are already statically linked binaries, so adding a return 123; test is not adding anything new or testing any new code paths.
The helper for current working directory is only useful to avoid a leak using https://linux.die.net/man/3/get_current_dir_name which needs to freed. However, it's used only once, and it's not a concerning issue. Unless the test-suite is run with lsan, I guess. I'd merge just the cwd change.

@fwsGonzo fwsGonzo self-assigned this Apr 7, 2026
Copy link
Copy Markdown
Member

@fwsGonzo fwsGonzo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding a new test that just returns 123 is not a new codepath. Most unit tests are static ELFs already. We could maybe merge the CWD change to avoid a leak with get_current_dir_name().

@jlinenkohl jlinenkohl marked this pull request as draft April 20, 2026 19:20
@jlinenkohl jlinenkohl force-pushed the pr-u6-elf-test-gating branch 2 times, most recently from a52f529 to 63dc20f Compare April 21, 2026 19:45
@jlinenkohl
Copy link
Copy Markdown
Contributor Author

I've pulled this back to draft for now as this was submitted too soon as a batch with the others. I had added some test gates for working on relocation support and this was not ready to go upstream. This was pushed up because of the CWD fix but was not supposed to include the rest.

I do appreciate your feedback though and have noted to include that minor fix in a subsequent update.

Also, about to propose a merge of a different fix in PR71 that caused a SIGSEGV. I discovered it while doing relocation work.

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