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

Multiple updates - see individual commits #101

Merged
merged 9 commits into from
Nov 24, 2024
Merged

Conversation

fhgwright
Copy link
Contributor

Notably:

  • More test variants for scandir and fdopendir
  • fdopendir test now defends againstfstatat buffer overrun bug.
  • Eliminates opportunistic dependencies on MacPorts cctools and ld64.
  • fdopendir now handles variants directly, without ugly Makefile kludge.
  • New DEBUG and OPT variables help with debug builds.

Tested (both from repo and via temp Portfile) on:

Mac OS X 10.4.11 8S165, PPC, Xcode 2.5 8M2558
Mac OS X 10.4.11 8S2167, i386, Xcode 2.5 8M2558
Mac OS X 10.5.8 9L31a, PPC, Xcode 3.1.4 9M2809
Mac OS X 10.5.8 9L31a, i386, Xcode 3.1.4 9M2809
Mac OS X 10.5.8 9L31a, x86_64, Xcode 3.1.4 9M2809
Mac OS X 10.5.8 9L31a, PPC (i386 Rosetta), Xcode 3.1.4 9M2809
Mac OS X 10.6.8 10K549, i386, Xcode 3.2.6 10M2518
Mac OS X 10.6.8 10K549, x86_64, Xcode 3.2.6 10M2518
Mac OS X 10.6.8 10K549, PPC (i386 Rosetta), Xcode 3.2.6 10M2518
Mac OS X 10.7.5 11G63, x86_64, Xcode 4.6.3 4H1503
OS X 10.8.5 12F2560, x86_64, Xcode 5.1.1 5B1008
OS X 10.9.5 13F1911, x86_64, Xcode 6.2 6C131e
OS X 10.10.5 14F2511, x86_64, Xcode 7.2 7C68
OS X 10.11.6 15G22010, x86_64, Xcode 8.1 8B62
macOS 10.12.6 16G2136, x86_64, Xcode 9.2 9C40b
macOS 10.13.6 17G14042, x86_64, Xcode 10.1 10B61
macOS 10.14.6 18G9323, x86_64, Xcode 11.3.1 11C505
macOS 10.15.7 19H15, x86_64, Xcode 12.4 12D4e
macOS 11.7.10 20G1427, x86_64, Xcode 13.2.1 13C100
macOS 11.7.10 20G1427, arm64, Xcode 13.2.1 13C100
macOS 12.7.6 21H1320, x86_64, Xcode 14.2 14C18
macOS 12.7.6 21H1320, arm64, Xcode 14.2 14C18
macOS 13.7.1 22H221, arm64, Xcode 15.2 15C500b
macOS 14.7.1 23H222, arm64, Xcode 16.1 16B40
macOS 15.1 24B83, arm64, Xcode 16.1 16B40

The OPT value (default -Os) is included in compile commands, and the
DEBUG value is included in both compile and link commands.

TESTED:
Both default and debug settings work.
Also fixes 32-bit wrapper to use proper config flag, and to disable it
on arm64, where it doesn't work.

TESTED:
Passes on all platforms with all applicable SDKs.
This ensures that both 32- and 64-bit inode cases are tested,
regardless of the platform default.

TESTED:
Pass (except known ino32 bug) on all platforms, with all applicable
SDKs.
This simply turns the old 'FEEDBACK' compile-time option into a
runtime option.

Also adds a one-line message on success.
At the time of this writing, fstatat() may choose the wrong variant of
stat to return, possibly overflowing the provided buffer.  To defend
against this, we make the buffer the larger of the two possible sizes,
and also move it off the stack for less verbosity when debugging.  See
the comment in the code.

TESTED:
All versions pass, including previously crashing ino32 cases.
Referencing the "bare" ar, ld, lipo, and install_name_tool
opportunistically uses the MacPorts cctools and ld64 versions, without
explicit dependencies.  This adds the explicit /usr/bin/ prefix to all
but lipo to avoid that.  These are still overridable defaults.

Since LD and AR are standard make defaults, defaulting them doesn't
work.  We switch to LDX and ARX to avoid that.

The lipo reference is left as is (since the system lipo is
inadequate), but is expected to go away in the near future.

TESTED:
Builds (including tests) on all platforms, with all applicable SDKs.
This reworks the fdopendir code to directly build all the needed
variants, instead of the horrible splitandfilterandmergemultiarch
kludge in the Makefile.  Not only does it reduce code duplication, but
it also drastically improves readability.  It does not change the
underlying logic in the "guts", but merely changes the interfacing to
the different variants.

This was the only use of splitandfilterandmergemultiarch, which will
be excised in a subsequent commit.

A subtle problem that this introduced is that the empty object filter
for the static library was previously failing to filter fdopendir.o
due to the way it was constructed, but fixing that resulted in a truly
empty static library on 10.15+ platforms.  This is actually illegal,
so there's now a hack to put one dummy object into the archive in this
case.  The dummy object defines a single global symbol, thereby
avoiding the "has no symbols" warning from 'ar' and other tools.

TESTED:
Builds and passes all tests (including the new one in a subsequent
commit), on all platforms.
Now that splitandfilterandmergemultiarch and its callers are no longer
used (solely to build fdopendir), they can be removed.  This also
removed the only references to LDX (formerly LD), LIPO, and PLATFORM,
so their default definitions are removed as well.

TESTED:
Build still works (and tests pass) on all platforms.
This provides coverage for the non-UNIX2003 variant on 32-bit
platforms.

TESTED:
Passes on all platforms.  Generates expected references.
@fhgwright
Copy link
Contributor Author

@mascguy
I'm still down the rabbit hole with some *stat*() issues, but I figured I might as well submit what's "blessed" so far (fully tested, of course). I'm not sure it makes sense to do a -devel update yet.

@mascguy
Copy link
Member

mascguy commented Nov 24, 2024

@mascguy I'm still down the rabbit hole with some *stat*() issues, but I figured I might as well submit what's "blessed" so far (fully tested, of course). I'm not sure it makes sense to do a -devel update yet.

No worries, though legacy-support-devel is already a few commits behind. Would it be worth creating a new release with everthing so far, other than this PR?

@mascguy mascguy merged commit 3fc1856 into macports:master Nov 24, 2024
@fhgwright
Copy link
Contributor Author

@mascguy

@mascguy I'm still down the rabbit hole with some *stat*() issues, but I figured I might as well submit what's "blessed" so far (fully tested, of course). I'm not sure it makes sense to do a -devel update yet.

No worries, though legacy-support-devel is already a few commits behind. Would it be worth creating a new release with everthing so far, other than this PR?

Thanks for the quick review and merge.

I wasn't suggesting that there was any reason to hold merged commits back from a -devel update; I was just thinking of the cost/benefit of the "churn" when the number of user-impacting changes is small. I expect the next round to include the *stat*() fixes, including the fstatat() buffer overrun fix (which is admittedly limited to an obscure circumstance), so that would be a good candidate for an update.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants