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

Draft: Preview of WIP in refactoring ABI functions and related ELF/Mach-O changes. #2350

Closed
wants to merge 5 commits into from

Conversation

kevemueller
Copy link

@kevemueller kevemueller commented Nov 20, 2024

This is a preview of what hopefully may come soon: Making ELF and Mach-O co-exist with identical feature level across all platforms.

As this is not a trivial change, I am posting here partial results for initial feedback.
As detailed in the separate commits, I am:

  • getting rid of libmachista in favour of a clean (purpose-built) Mach-O parser.
  • starting changing the ABI unit tests, but the output is somewhat controversial (e.g. Linux:3.2.0:amd64?)
  • Refactored pkg_elf into pgk_abi and pkg(_binfmt)_elf. While this merely copies things around, I welcome a close look.
  • Finally I introduce pkg_abi_macho that parses Mach-O files on any platform.

Currently unsolved issues:

Work continues:

  • shared library extraction with Mach-O
  • error handling

Feedback welcome. Unit tests welcome.

Keve added 5 commits November 20, 2024 23:07
…g_macho

Remove libmachista in favour of own binfmt_macho.
Promote pkg_elf to all platforms and fix libelf building on Darwin.
Remove inactive branches in automake files.

NB: this temporarily disables Mach-O functionality in pkg.
@kevemueller kevemueller changed the title Dra Draft: Preview of WIP in refactoring ABI functions and related ELF/Mach-O changes. Nov 20, 2024
@bapt
Copy link
Member

bapt commented Nov 21, 2024

in order to be able to work without you on this (in particular adding the regression tests, I will create a dedicated branch and already merge this into that dedicated branch

@kevemueller
Copy link
Author

Hi Baptiste,

sounds great.
I will continue with the extraction of shlibs for Mach-O a bit later, as mentioned, I really need unit tests to be able to understand the intent of some of the code. Unit-tests cover intent very well, whereas code is often obfuscating it.

My thinking for Fat libraries, which is a wonderful concept and very nicely managed in Mach-O (btw. there was FatELF earlier under Linux), goes like this:
If ARCH is configured, use that to select the right entry, entries are distinguished by CPU type and subtype, i.e. there can be two AMD64 entries, one for baseline and one for x86-64-v4.

While bootstrapping the configuration with ABI_FILE, allow to use a descriptor like ABI_FILE=/bin/sh#amd64 and use that descriptor for selecting the right entry.
If no descriptor is given use ARCH (unlikely case), otherwise use the architecture pkg was built for.

In all cases proper debug1 messaging should ensure that any error condition becomes easily trace-able.

This feels like a natural extension to ABI_FILE to me.

@kevemueller
Copy link
Author

Thinking about the Linux 3.2.0 case, in fact this is a good choice for ABI, as that seems to be the last time Linux changed its ABI. I would prefer 3.2, though.

@bapt
Copy link
Member

bapt commented Nov 21, 2024

beside some corner cases which I haven't decided yet what to do about (old arm FreeBSD arm binaries) pkg should deal with os specificity dynamically, I have switch the linux ABI to be 3.2.
I do like your proposal about the ABI_FILE.

Is there any part in particular where you need me to add some regression tests ASAP?

@bapt
Copy link
Member

bapt commented Nov 21, 2024

I have added more tests for macho including macosfat binary (the ABI probably needs some work there :D )

@kevemueller
Copy link
Author

Well, the unit under test here is analyse_elf and analyse_macho. So there should be a suite dealing with creating packages from binaries and checking how the actual MANIFEST enrichment using the automatic analyze of the binary worked out. This would sort any regression with the current refactoring as well as the mach-o code that I will be adding.

Expectations on flags, shlibs_provided, shlibs_required, self-reference (provided==depends), annotation no_provide_shlib, interaction with ALLOW_BASE_SHLIBS.

I know this is quite complex and a lot of combinations are possible, moreover the tests are likely OS dependent.

Best would be to start agreeing on how to deal with binary input to the test cases in the first place. If building them is done as a subproject, there is full control on how they can be tuned to support the test cases above. GitHub/Sourcehut CI can build them on the major platforms and we can reference them as a sub-module to the tests.

@bapt
Copy link
Member

bapt commented Nov 21, 2024

right I will work on adding those tests, I think we can continue like with the .bin adding minimalistic libraries to the test suite is enough, I don't think it is worth building them and trying to setup some cross building.

I think we can merge the abi_refactoring branch, as it is stable for me right now, what do you think?

@kevemueller
Copy link
Author

I just ran through your changes in the branch. Impressive.

I honestly would not merge until I have confirmation that shlibs (on ELF) works the same on before/after (even if only with FreeBSD), but I might be too conservative here.

Obviously it would make the workflow smooth if you just merge and I continue with my small changes. Technically I would then just close/abandon this pull request, right?

@bapt
Copy link
Member

bapt commented Nov 21, 2024

A first naive test has been added, library tests are faster to run than frontend test, so I ended up using them ;)

@kevemueller
Copy link
Author

Just a small note pkg_macho.c is currently not in any build and will be completely removed. I will base my code in pkg_abi_macho.c for shlib handling on the way it is done in pkg_elf.c. The parsing code is trivial and will use already existing functionality from my binfmt_macho.c (it is already working outside of pkg).

@bapt
Copy link
Member

bapt commented Nov 21, 2024

this has been merged! thank you!

@bapt bapt closed this Nov 21, 2024
@kevemueller kevemueller deleted the abiref branch November 21, 2024 20:01
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