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

Improve testability #468

Open
brenoguim opened this issue Feb 23, 2023 · 6 comments
Open

Improve testability #468

brenoguim opened this issue Feb 23, 2023 · 6 comments

Comments

@brenoguim
Copy link
Collaborator

Hi! I would like to suggest (and implement if agreed) an improvement to the testability of the project.

I noticed that it's hard to exercise a specific part of the tool which I'll call the Layout Engine. To me, it's everything in rewriteSectionsExecutable and rewriteSectionLibrary except for the writeReplacedSections and writeHeaders.

The current approach to testing is to put whole ELF files and run through the whole flow - reading the elf file, understanding its contents, looking at .dynamic, .dynstr and so on, then doing layout and finally writing.

But the Layout Engine, is largely independent of the content of the elf files. All this engine needs is:

  • What are the sections offsets/addr/size?
  • What are the segment offsets/vaddr/paddr
  • Restrictions (content that can't be moved in the address space, sections that must come first...)
  • What layout strategy you want: Put sections in the end, or put sections in the beginning
  • What section you want to grow/shrink

So, my suggestion (and desire to implement) is a separate class that takes these inputs, calculates the new layout and spits it out.

Then the caller will use this result to write the appropriate data, adjust special offsets and the more elf specific stuff.

This will improve the testing in the following ways:

  1. Human readable test: We will be able to store text layout files that are easy to construct by hand. These layout files would be used to test the LayoutEngine in isolation and verify properties about its results. For example, sections in the file should never intersect (like it happened for a bug I investigated).
  2. Encourage more tests: We will be able to request users to --generate-layout-dump if they can't share the whole binary. They would be able to read the text file and see there is nothing confidential there and would give us something to work with. This would make it much easier to add tests from bugs.
  3. Fuzz testing - Allow us to easily generate random scenarios to make sure the engine never crashes or spits something with broken invariants (like overlapping sections).

I would love to implement something like that, and I'm confident that I can do it in a way that you are comfortable receiving it (that means no big rewrites, focus on small incremental changes, tests...).

But before I write any of that, I would like to hear if this makes sense for the project, if someone is already working on it, or if it's too out there.

Thanks!

@Mic92
Copy link
Member

Mic92 commented Feb 24, 2023

I like the idea. Thumbs up from my side.

@Mic92
Copy link
Member

Mic92 commented Feb 24, 2023

cc @cgzones

@brenoguim
Copy link
Collaborator Author

I'm confident that I can do it in a way that you are comfortable receiving it (that means no big rewrites, focus on small incremental changes, tests...).

I wrote the whole thing twice and I still failed at this part :)
Maybe a better idea is to have infrastructure to auto-generate true ELF files from a high level description.

@Mic92
Copy link
Member

Mic92 commented Mar 7, 2023

Sorry, I have been a bit radio silent the last two weeks. I have just accumulated different projects over the years and now I am cycling through them for reviews. Would you be open to also be added as a maintainer so we can share the review load? The only constraint that this project has is that needs to work for the usecases in nixpkgs and that we can easily bootstrap it from a small set of dependencies (i.e. c++ compiler + coreutils + linker).

@brenoguim
Copy link
Collaborator Author

Hey @Mic92 , I would love to participate as a maintainer! The constraints are fair and pretty much what I would expect from patchelf anyways.
I think my contributions goin forward will be on adding more tests (so that later one day we can confidently take bigger changes), review PRs and address crashes and urgent bugs.

@Mic92
Copy link
Member

Mic92 commented Mar 8, 2023

Welcome to the team. You should have received an invite.

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

No branches or pull requests

2 participants