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

tests.cc-wrapper: skip known-broken sanitizer cases #41284

Merged
merged 1 commit into from
May 31, 2018

Conversation

Ralith
Copy link
Contributor

@Ralith Ralith commented May 30, 2018

Motivation for this change

Fixes overly-broad testing due to #41065. Let me know if there's a better way to test for libc++ being in use.

CC @Ericson2314 @LnL7

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option build-use-sandbox in nix.conf on non-NixOS)
  • 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 nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.

@Ralith Ralith requested review from Ericson2314 and orivej as code owners May 30, 2018 20:24
@@ -90,7 +90,7 @@ stdenv.mkDerivation {
# Binutils, and Apple's "cctools"; "bintools" as an attempt to find an
# unused middle-ground name that evokes both.
inherit bintools;
inherit libc nativeTools nativeLibc nativePrefix isGNU isClang default_cxx_stdlib_compile;
inherit libc nativeTools nativeLibc nativePrefix isGNU isClang isLibCxx default_cxx_stdlib_compile;
Copy link
Member

Choose a reason for hiding this comment

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

What about introducing libcxx similar to libc we have now? I think we want to add that in the future anyway to get rid of the default_cxx_stdlib_compile hack.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is there documentation anywhere of what exactly the libc we have now is?

@Ericson2314
Copy link
Member

Hmm so this doesn't affect the libstdc++ ones? That's very very odd, as the sanitizer runtimes don't come from either.

@Ralith
Copy link
Contributor Author

Ralith commented May 31, 2018

Whoops, looks like that's an issue too.

Is there any way to run all the affected tests, so I don't have to keep making slightly inaccurate guesses about what to try?

@Ericson2314
Copy link
Member

Sadly I don't know of an easy way to replicate what hydra does, as conceptually simple as it is.

@Ericson2314
Copy link
Member

Ericson2314 commented May 31, 2018

Anyways looks good now. The condition makes sense in theory to me. Thanks for fixing.

@Ericson2314 Ericson2314 merged commit ccd3f98 into NixOS:master May 31, 2018
@Ralith Ralith deleted the sanitizer-fix-redux branch May 31, 2018 07:06
@matthewbauer
Copy link
Member

@Ralith Do you know what's going on with this latest error:

https://hydra.nixos.org/build/75406498/nixlog/1

ld: file not found: /nix/store/wdiibxx7221iiv8cdgdafgbmkvb58bw9-clang-wrapper-6.0.0/resource-root/lib/darwin/libclang_rt.asan_osx_dynamic.dylib
clang-6.0: error: linker command failed with exit code 1 (use -v to see invocation)
builder for '/nix/store/ih843k6q8xv3xbakp7r370377z18wnci-cc-wrapper-test.drv' failed with exit code 1

@Ralith
Copy link
Contributor Author

Ralith commented Jun 4, 2018

Looks like the test is doing its job and identifying that asan is broken under that configuration, perhaps (but not necessarily) due to changes introduced by #41065. In particular, runtime libraries that at least on linux are provided by compiler-rt aren't being found, though it appears to be looking in at least approximately the right place. The output of find -L /nix/store/wdiibxx7221iiv8cdgdafgbmkvb58bw9-clang-wrapper-6.0.0/resource-root/ would be interesting. I don't have a darwin machine to test on myself.

@orivej
Copy link
Contributor

orivej commented Jun 6, 2018

I have verified that sanitizers did not work on Darwin neither when #39743 was merged in 713d580 [1], nor directly before that in ae99562 [2]. Therefore it is reasonable to disable sanitizer tests on Darwin for now.

[1] With tests.cc-wrapper-clang-6 the error is ld: file not found: /nix/store/2qfdihv2x3m6da4bpv9jpi1qwlnzkns6-clang-wrapper-6.0.0/resource-root/lib/darwin/libclang_rt.asan_osx_dynamic.dylib. File tree:

/nix/store/2qfdihv2x3m6da4bpv9jpi1qwlnzkns6-clang-wrapper-6.0.0/resource-root/lib
└── darwin
    ├── libclang_rt.10.4.a
    ├── libclang_rt.cc_kext.a
    └── libclang_rt.osx.a

With tests.cc-wrapper-gcc, the error is the the same as in [2].

[2] The error is:

/nix/store/3c3vrabg4yy87y4qx8zsy5x2vdg132j5-sanitizers.c:1:10: fatal error: 'sanitizer/asan_interface.h' file not found
#include <sanitizer/asan_interface.h>

@orivej
Copy link
Contributor

orivej commented Jun 6, 2018

Disabled sanitizer tests on Darwin in 07ebb8b.

orivej added a commit that referenced this pull request Jun 6, 2018
@Ralith
Copy link
Contributor Author

Ralith commented Jun 6, 2018

Thanks for handling that! I wonder why the asan stuff is missing. Maybe some option we need to pass to the compiler-rt build, or perhaps one of our darwin-specific patches or options is disabling it.

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.

6 participants