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

RFC: bin2cfetbl.py tool (ELF and Mach-O support) #34

Closed
wants to merge 1 commit into from

Conversation

stanislaw
Copy link

@stanislaw stanislaw commented Feb 21, 2020

Describe the contribution

This work complements the work I did for CFS/OSAL in nasa/osal#352. In order to run CFS on macOS the elf2cfetbl tool has to be updated to support Mach-O.

This work is based on the following comment by @jphickey :

When I said there is no requirement to use this specific tool, you will need to somehow generate appropriate binary .tbl files for your target if you want to use apps that load runtime tables. The format is pretty simple (C struct encapsulated with a FS+TBL header) so its not hard to hack out a python script to create these files, there is just no python tool already written AFAIK.

I have decided to rewrite the tool in Python and made it to support both ELF and Mach-O files.

I would like to get feedback about the approach: if the direction is approved I would to complete the implementation and add all missing test cases.

This is a small plan of what is done and what still has to be done:

  • Reading Mach-O 64-bit and writing .tbl file (little endian binaries)
  • Reading ELF 64-bit and writing .tbl file (little endian binaries)
  • Integration tests using LLVM Integrated Tester (LIT)
  • Integration of the Python tool and integration tests into CMake tree
  • Supporting reading command-line arguments as supported by current elf2cfetbl
  • Big endian binaries
  • 32-bit binaries
  • Supporting more output of what the current elf2cfetbl does (example: Original Source File Modification Time:)
  • Add more test fixtures. For now I am only using sample_table.tbl and sch_lab_table which are built by CFS by default.

Testing performed

pip install lief lit filecheck

Run the bin2cfetbl-tests-integration CMake target. The LIT tests should pass.

Expected behavior changes

  • I have tried to preserve existing behavior 1 to 1. The only difference I am seeing is in what gets printed.
  • I consider my implementation cleaner and easier to follow.
    • Current C implementation uses global variables a lot and that makes the code harder to follow. I have tried to package all functions in such a way that the reasoning would be easier to follow.

System(s) tested on

  • Hardware: MacBook Pro
  • OS: macOS Mojave 10.14.6
  • Versions: The latest master branch of this repository.

Additional context

None.

Third party code

Contributor Info - All information REQUIRED for consideration of pull request

Stanislav Pankevich. Personal.

The individual CLA has been signed and sent to the GSFC-SoftwareRelease.

@skliper
Copy link
Contributor

skliper commented Feb 21, 2020

Thanks @stanislaw. One note on the CLA, I just checked on this side and the only personal CLA that has been received is specifically for the OSAL repository (18.370.1). The rest of the framework (cFE, PSP, ci_lab, sample_app, sample_lib, sch_lab, to_lab, cFS-GroundSystem, elf2cfetbl, and tblCRCtool) is covered by CLA form 18.128-1

Could you provide the 18.128.1 form that covers this contribution?

Note we are hoping in the next round to consolidate to one form to avoid this complexity, but at this point we are stuck managing two.

@stanislaw
Copy link
Author

stanislaw commented Feb 21, 2020

I am pretty sure I have signed and sent the NASA/CFE 18.128-1 ~15 min ago. The email should be Individual CLA signed for CFS/CFE project: Stanislav Pankevich.

Note we are hoping in the next round to consolidate to one form to avoid this complexity, but at this point we are stuck managing two.

No worries. I am used to processes. Happy to participate in testing and improvement.

@skliper
Copy link
Contributor

skliper commented Feb 21, 2020

@stanislaw Excellent! I checked with our folks yesterday (they don't give me direct access to that email account), so my information is old. Thanks!

@jphickey
Copy link
Contributor

This requires some CCB discussion. This strikes me as a complete alternative to elf2cfetbl rather than a change or update to it. Not that that is a bad thing; it looks like great work, but I think should be an entirely separate repo, not as an change to elf2cfetbl.

As far as I can tell there should be nothing preventing you from creating another github repo to contain this alternative table creation tool. It would also be a good fit for putting into the list of external tools and resources to raise awareness of it. It should be fairly easy for any interested user to pull in this tool instead of elf2cfetbl if they so choose.

@stanislaw
Copy link
Author

@jphickey thanks for the answer. It is very easy to create a separate repo to make it available as an external tool. I could also make this repository a submodule of that new repository and write integration tests that make sure that the Python tool always produces the same output compared to the elf2cfetbl tool.

Do you also consider the macOS OSAL port to be something you would prefer to keep externally? I am asking because there is a dependency between osal/posix-mac and this tool if you want to run CFS on macOS. Has there been any decision made about the macOS OSAL? Thank you.

@jphickey
Copy link
Contributor

Do you also consider the macOS OSAL port to be something you would prefer to keep externally? I am asking because there is a dependency between osal/posix-mac and this tool if you want to run CFS on macOS. Has there been any decision made about the macOS OSAL? Thank you.

There are multiple facets to this question... While it is true that a MacOS developer whom is also running table-based apps would (possibly) need both of these, they really address different issues and and can be approached differently.

The reason I propose putting this into a separate repo is because it really is an outright replacement of the elf2cfetbl tool. In contrast, for the OSAL side of things, I think it would be better to integrate because there is still high level of overlap between MacOS and POSIX and want to avoid having a complete for for every system variant if possible.

You are more than welcome to keep your complete MacOS OSAL changes in your local fork as long as you need them, but it will probably be a more gradual, itemized process to merge these into the OSAL.

Basically on the OSAL side, I would like to identify the specific areas where the current POSIX implementation is depending on features that other non Linux/Glibc environments may not consistently provide, and address them each independently with focused change sets. This would benefit not just MacOS, but also various BSD, Cygwin, and WSL use cases which have come up from time to time.

Note that IIRC the Cygwin environment (and maybe WSL too?) uses a COFF binary format, so it also doesn't work with elf2cfetbl. So for the longer-term solution for the table generation problem I would still advocate for something entirely different that is more system agnostic, not trying to chase every possible binary executable format.

@stanislaw
Copy link
Author

stanislaw commented Feb 27, 2020

Do you also consider the macOS OSAL port to be something you would prefer to keep externally? I am asking because there is a dependency between osal/posix-mac and this tool if you want to run CFS on macOS. Has there been any decision made about the macOS OSAL? Thank you.

There are multiple facets to this question... While it is true that a MacOS developer whom is also running table-based apps would (possibly) need both of these, they really address different issues and and can be approached differently.

The reason I propose putting this into a separate repo is because it really is an outright replacement of the elf2cfetbl tool.

I fully agree that it makes sense to create a separate repo. I will create it within 1-2 days.

In contrast, for the OSAL side of things, I think it would be better to integrate because there is still high level of overlap between MacOS and POSIX and want to avoid having a complete for for every system variant if possible.

You are more than welcome to keep your complete MacOS OSAL changes in your local fork as long as you need them, but it will probably be a more gradual, itemized process to merge these into the OSAL.

Basically on the OSAL side, I would like to identify the specific areas where the current POSIX implementation is depending on features that other non Linux/Glibc environments may not consistently provide, and address them each independently with focused change sets. This would benefit not just MacOS, but also various BSD, Cygwin, and WSL use cases which have come up from time to time.

I very much understand and share your point of view (general maintenance perspective). On my end though I need to have cFS running on Linux and macOS rather soon so I will keep working on my fork for now but also I want to extract some easy things from my macOS PR starting from this one: nasa/osal#363.

Note that IIRC the Cygwin environment (and maybe WSL too?) uses a COFF binary format, so it also doesn't work with elf2cfetbl. So for the longer-term solution for the table generation problem I would still advocate for something entirely different that is more system agnostic, not trying to chase every possible binary executable format.

Just a thought: I just checked LIEF again. They seem to not have COFF support but otherwise they seem to be rather advanced in the general support of ELF/Mach-O/PE so maybe people who need COFF could contribute its support there directly.

@stanislaw stanislaw closed this Feb 27, 2020
@stanislaw
Copy link
Author

The reason I propose putting this into a separate repo is because it really is an outright replacement of the elf2cfetbl tool.

I fully agree that it makes sense to create a separate repo. I will create it within 1-2 days.

Just FYI: I am still keeping it as a direct fork of elf2cfetbl but it is a separate repo now: https://github.com/stanislaw/bin2cfetbl.

@skliper
Copy link
Contributor

skliper commented Mar 2, 2020

Just FYI: I am still keeping it as a direct fork of elf2cfetbl but it is a separate repo now: https://github.com/stanislaw/bin2cfetbl.

Excellent! Would you be interested in adding it to https://github.com/nasa/cFS/blob/master/README.md under "Other Tools" to raise awareness of this new tool? If so please submit a pull request on the cFS repo.

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.

3 participants