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

arm-embedded: make gcc multi-lib (arm32 + thumb) #111321

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

cleverca22
Copy link
Contributor

@cleverca22 cleverca22 commented Jan 30, 2021

Motivation for this change

gcc lacked a thumb version of crtbegin.o
newlib lacked a thumb version of everything

Things done

tested the resulting binaries with the expr&commands in https://gist.github.com/cleverca22/9cb66a1182fba1d660bf1b970437fb7b
confirmed that its no longer leaking arm32 opcodes into an elf binary meant for a thumb-only cpu

todo:

get the newlib multilib build properly into the linker search path, i need to manually add a -L flag for it currently (see the above gist)
i suspect this change will also break x86 32+64bit multilib

  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@@ -8,7 +8,7 @@ in lib.concatLists [
(lib.optional (p ? cpu) "--with-cpu=${p.cpu}")
(lib.optional (p ? abi) "--with-abi=${p.abi}")
(lib.optional (p ? fpu) "--with-fpu=${p.fpu}")
(lib.optional (p ? float) "--with-float=${p.float}")
(lib.optional (!targetPlatform.gcc.enableMultilib or false && (p ? float)) "--with-float=${p.float}") # --with-multilib-list conflicts with --with-float
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
(lib.optional (!targetPlatform.gcc.enableMultilib or false && (p ? float)) "--with-float=${p.float}") # --with-multilib-list conflicts with --with-float
(lib.optional (!enableMultilib && (p ? float)) "--with-float=${p.float}") # --with-multilib-list conflicts with --with-float

I would make enableMultilib another parameter so things work even if it is overridden from the target platform

@stale
Copy link

stale bot commented Aug 4, 2021

I marked this as stale due to inactivity. → More info

@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Aug 4, 2021
@sigprof
Copy link
Contributor

sigprof commented Sep 1, 2021

Do you still plan to move this PR forward? In particular, did you figure out a way to set the library search path automatically instead of using environment variables to set it?

This change would also be useful for riscv64-embedded and riscv32-embedded (currently the compilers and libraries for those targets are built only for hard-float, but for embedded usage the soft-float version is likely to be needed).

@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Sep 1, 2021
@cleverca22
Copy link
Contributor Author

i do want to finish this PR, but i had forgotten about it for a while

@stale
Copy link

stale bot commented Apr 17, 2022

I marked this as stale due to inactivity. → More info

@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Apr 17, 2022
@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Mar 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants