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

gcc12: disable libsanitizer for mips64 #238715

Closed
wants to merge 3 commits into from
Closed

gcc12: disable libsanitizer for mips64 #238715

wants to merge 3 commits into from

Conversation

ghost
Copy link

@ghost ghost commented Jun 20, 2023

Description of changes

Resubmitting @trofi's 6cec030 from #237815 which was the right solution to this problem.

Cc: @trofi @alyssais

Things done
  • Built on platform(s)
    • mips64el-linux (cross from x86_64-linux)
    • mips64-linux (cross from x86_64-linux)
  • Fits CONTRIBUTING.md.

Adam Joseph and others added 3 commits June 20, 2023 00:07
This commit moves `canExecute`, `emulator`, and `emulatorAvailable`
out of `{host,build,target}Platform` and lifts them into
`lib.systems`.  Stubs have been left behind to assist with migration
of out-of-tree code.

Nix's function equality is unpredictable and error-prone; we should
not rely on it.  Since we need to be able to test
`{host,build,target}Platform` attrsets for equality, the most
sustainable long-term solution is to forbid functions from these
attrsets, much like we do for the `meta` attrset.

The stubs which have been left behind are a narrowly-defined case
where Nix's function equality *does* behave predictably.
Comparing platform sets using `==` works for most cases since the in the
native case, the platform sets would not only be equal, but the same
identical value (in terms of memory location). This is, however,
technically not always the case, e.g. when specifying crossSystem
explicitly, cases can arise when we should use a native stdenv, but
don't since the functions wouldn't compare as equal due to them being
constructed in separate invocations of `lib.system.elaborate`. This
problem is described in #237427 and motivated #237512 as an equality
predicate that ignors the functions.

Since the functions in the platform attribute set no longer relate to
the platform set at all, we can use the same function value for every
placeholder deprecation error function in the attribute set—consequently
they will all compare as equal even for independently constructed
platform sets.

This is achieved by floating the funtions into a let binding in
`lib/systems/default.nix`, ensuring that they are only constructed once
per nixpkgs instance, i.e. when that file is imported. (Consequently,
comparison of platform sets should be reliable as long as they come from
the same instance of nixpkgs.)

    let
      inherit (
        import <nixpkgs> {
          localSystem = "x86_64-linux";
          crossSystem.system = "x86_64-linux";
        }
      ) buildPlatform
        hostPlatform
        ;
    in

    buildPlatform == hostPlatform
    # => true

This property is also verified with new test cases.
    Without the change build on mips64-unknown-linux-gnu fails as:

        $ nix-build -A buildPackages.gcc12 --argstr crossSystem mips64-linux

        In file included from ...-glibc-mips64-unknown-linux-gnu-2.37-8-dev/include/bits/stat.h:25,
                         from ...-glibc-mips64-unknown-linux-gnu-2.37-8-dev/include/fcntl.h:78,
                         from ../../../../gcc-12.3.0/libsanitizer/sanitizer_common/sanitizer_linux.cpp:55:
        ...-glibc-mips64-unknown-linux-gnu-2.37-8-dev/include/bits/struct_stat.h:190:8: error: redefinition of 'struct stat64'
          190 | struct stat64
              |        ^~~~~~
@ghost ghost closed this Jun 20, 2023
@ghost ghost deleted the pr/gcc12/mips-libsanitizer branch June 20, 2023 07:18
@ghost ghost restored the pr/gcc12/mips-libsanitizer branch June 20, 2023 07:18
@NixOS NixOS locked as too heated and limited conversation to collaborators Jun 20, 2023
This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants