Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Re-enable/fix non-argv[0] exe name tests #1046

Merged
merged 8 commits into from
Dec 31, 2023
Merged

Conversation

mrdomino
Copy link
Collaborator

@mrdomino mrdomino commented Dec 31, 2023

These tests should now pass in all cases and if they don't it's a bug.

One buggy case is if they're run against the short-lived loader that used a stubbed RealPath on FreeBSD and NetBSD; if they fail on those platforms it's likely for that reason.

Still TODO, preferably in a future PR:

  • Make GetProgramExecutableName reliable for assimilated XNU binaries; it seems like this might be doable with what dyld::getExecutablePath (and therefore _NSGetExecutablePath) uses.
  • Enable argv[0] tests on all platforms, probably by using the to-be-written cosmo_execve.

There is probably nothing for assimilated OpenBSD binaries, so the check introduced will probably never completely go away.

These should always pass no matter what and if they don't it's a bug
somewhere.

Cases where they fail:

- assimilated binary on OpenBSD or x86_64 XNU (no `KERN_PROC_PATHNAME`)

- bad loader that did not implement `RealPath` on Free/Net BSD
Properly tests the behavior on Metal now, which is to always return
`APE_COM_NAME`.

Ensures that the path of the test itself as returned by
`GetProgramExecutableName` is executable.
GetProgramExecutableName does not work right on assimilated OpenBSD or
non-silicon XNU binaries. If we are on one of those platforms, look for
ape magic in ourselves and quit if none is found.

If `argv[0]` is not reliable for the test invocation itself (e.g.
because the test is run out of `PATH`), the test assert-fails.
@mrdomino mrdomino marked this pull request as ready for review December 31, 2023 17:46
Copy link
Owner

@jart jart left a comment

Choose a reason for hiding this comment

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

Looks good. As of the APE 1.10 deployment, this PR passes fleet.

@jart jart merged commit b02d13c into jart:master Dec 31, 2023
5 checks passed
@mrdomino mrdomino deleted the exename-tests branch December 31, 2023 19:53
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