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

run-make-fulldeps/linker-output-non-utf8 failure #63520

Closed
nikomatsakis opened this issue Aug 13, 2019 · 8 comments · Fixed by #65321
Closed

run-make-fulldeps/linker-output-non-utf8 failure #63520

nikomatsakis opened this issue Aug 13, 2019 · 8 comments · Fixed by #65321
Labels
A-linkage Area: linking into static, shared libraries and binaries A-testsuite Area: The testsuite used to check the correctness of rustc P-medium Medium priority T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@nikomatsakis
Copy link
Contributor

when running x.py test locally I get errors in the test run-make-fulldeps/linker-output-non-utf8:

---- [run-make] run-make-fulldeps/linker-output-non-utf8 stdout ----

error: make failed
status: exit code: 2
command: "make"
stdout:
------------------------------------------
LD_LIBRARY_PATH="/home/nmatsakis/versioned/rust-6/build/x86_64-unknown-linux-gnu/test/run-make-fulldeps/linker-output-non-utf8/linker-output-non-utf8:/home/nmatsakis/versioned/rust-6/build/x86_64-unknown-linux-gnu/stage2/lib:/home/nmatsakis/versioned/rust-6/build/x86_64\
-unknown-linux-gnu/stage0-bootstrap-tools/x86_64-unknown-linux-gnu/release/deps:/home/nmatsakis/versioned/rust-6/build/x86_64-unknown-linux-gnu/stage0/lib" '/home/nmatsakis/versioned/rust-6/build/x86_64-unknown-linux-gnu/stage2/bin/rustc' --out-dir /home/nmatsakis/versi\
oned/rust-6/build/x86_64-unknown-linux-gnu/test/run-make-fulldeps/linker-output-non-utf8/linker-output-non-utf8 -L /home/nmatsakis/versioned/rust-6/build/x86_64-unknown-linux-gnu/test/run-make-fulldeps/linker-output-non-utf8/linker-output-non-utf8  library.rs
mkdir /home/nmatsakis/versioned/rust-6/build/x86_64-unknown-linux-gnu/test/run-make-fulldeps/linker-output-non-utf8/linker-output-non-utf8/zzz$'\xff'
mv /home/nmatsakis/versioned/rust-6/build/x86_64-unknown-linux-gnu/test/run-make-fulldeps/linker-output-non-utf8/linker-output-non-utf8/liblibrary.a /home/nmatsakis/versioned/rust-6/build/x86_64-unknown-linux-gnu/test/run-make-fulldeps/linker-output-non-utf8/linker-outp\
ut-non-utf8/zzz$'\xff'
LD_LIBRARY_PATH="/home/nmatsakis/versioned/rust-6/build/x86_64-unknown-linux-gnu/test/run-make-fulldeps/linker-output-non-utf8/linker-output-non-utf8:/home/nmatsakis/versioned/rust-6/build/x86_64-unknown-linux-gnu/stage2/lib:/home/nmatsakis/versioned/rust-6/build/x86_64\
-unknown-linux-gnu/stage0-bootstrap-tools/x86_64-unknown-linux-gnu/release/deps:/home/nmatsakis/versioned/rust-6/build/x86_64-unknown-linux-gnu/stage0/lib" '/home/nmatsakis/versioned/rust-6/build/x86_64-unknown-linux-gnu/stage2/bin/rustc' --out-dir /home/nmatsakis/versi\
oned/rust-6/build/x86_64-unknown-linux-gnu/test/run-make-fulldeps/linker-output-non-utf8/linker-output-non-utf8 -L /home/nmatsakis/versioned/rust-6/build/x86_64-unknown-linux-gnu/test/run-make-fulldeps/linker-output-non-utf8/linker-output-non-utf8  -L /home/nmatsakis/ve\
rsioned/rust-6/build/x86_64-unknown-linux-gnu/test/run-make-fulldeps/linker-output-non-utf8/linker-output-non-utf8/zzz$'\xff' exec.rs 2>&1 | "/home/nmatsakis/versioned/rust-6/src/etc/cat-and-grep.sh" this_symbol_not_defined
[[[ begin stdout ]]]
^[[90merror: Argument 6 is not valid Unicode: "/home/nmatsakis/versioned/rust-6/build/x86_64-unknown-linux-gnu/test/run-make-fulldeps/linker-output-non-utf8/linker-output-non-utf8/zzz\xFF"

^[[0m
[[[ end stdout ]]]
^[[1;31mError: cannot match: this_symbol_not_defined^[[0m

------------------------------------------
stderr:
------------------------------------------
make: *** [Makefile:23: all] Error 1

------------------------------------------

The most notable part being this:

error: Argument 6 is not valid Unicode: "/home/nmatsakis/versioned/rust-6/build/x86_64-unknown-linux-gnu/test/run-make-fulldeps/linker-output-non-utf8/linker-output-non-utf8/zzz\xFF"

perhaps related to the linker on my system?

@nikomatsakis nikomatsakis added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Aug 13, 2019
@Centril Centril added T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) A-testsuite Area: The testsuite used to check the correctness of rustc A-linkage Area: linking into static, shared libraries and binaries I-nominated labels Aug 13, 2019
@nikomatsakis
Copy link
Contributor Author

Compiler team pre-triage: marking as P-medium, this is probably something wacky in my local setup. We'll see if others complain about it.

@nikomatsakis nikomatsakis added P-medium Medium priority and removed I-nominated labels Aug 15, 2019
@cuviper
Copy link
Member

cuviper commented Aug 15, 2019

I have also seen this in Fedora and RHEL builds, but I didn't worry enough to chase it down yet.

This error is actually from rustc itself:

let args = env::args_os().enumerate()
.map(|(i, arg)| arg.into_string().unwrap_or_else(|arg| {
early_error(ErrorOutputType::default(),
&format!("Argument {} is not valid Unicode: {:?}", i, arg))
}))
.collect::<Vec<_>>();

Arguments 5 and 6 from the rustc command in your report are:

-L /home/nmatsakis/versioned/rust-6/build/x86_64-unknown-linux-gnu/test/run-make-fulldeps/linker-output-non-utf8/linker-output-non-utf8/zzz$'\xff'

That line is deliberately added in the linker-output-non-utf8 test:

bad_dir := $(TMPDIR)/zzz$$'\xff'
all:
$(RUSTC) library.rs
mkdir $(bad_dir)
mv $(TMPDIR)/liblibrary.a $(bad_dir)
$(RUSTC) -L $(bad_dir) exec.rs 2>&1 | $(CGREP) this_symbol_not_defined

I have no idea why this isn't an issue in CI though...

@cuviper
Copy link
Member

cuviper commented Aug 15, 2019

I have no idea why this isn't an issue in CI though...

Oh, I think I do know why. That bad_dir in the Makefile is assuming shell expansion of that $$'\xff' (with doubled $$ to escape from make itself). Fedora and RHEL use sh -> bash which does expand it, but Debian and Ubuntu (used in CI) have sh -> dash, which is a more limited shell.

bash:

$ echo -n $'\xff' | xxd
00000000: ff                                       .

dash:

$ echo -n $'\xff' | xxd
00000000: 245c 7866 66                             $\xff

So I think this test hasn't really been working as desired on dash-using platforms at all, since the bad_dir isn't really "bad" when not escaped. And it can't work even then because rustc won't accept non-utf8 arguments in the first place, never mind reaching the linker.

@cuviper
Copy link
Member

cuviper commented Aug 15, 2019

Note, the dist-i686-linux and dist-x86_64-linux builders do use CentOS 5 as the base image, which would have this issue too, but we don't actually run tests there. It would be nice to add some diversity to the testing images, either with Fedora or perhaps the redistributable RHEL8 UBI.

@mati865
Copy link
Contributor

mati865 commented Aug 27, 2019

Any idea what to do with this test?
Having to use --exclude src/test/run-make-fulldeps for over two weeks isn't very practical.

@Mark-Simulacrum
Copy link
Member

We can probably just remove that test, it doesn't seem worth the pain of having it if it's causing people problems. It sounds like we don't have a test case that would work universally even on *nix platforms, right? Or that's my interpretation of #63520 (comment) at least.

It's also pretty unclear to me that ICE-ing on non-utf8 output from linkers is something we need to test for. That sounds both incredibly uncommon, and also pretty easily fixable if we need to.

One thought for testing this is to override the linker rustc uses with a shell script or even a rust program that runs the linker internally but just does something like println or eprintln with some non-utf8 output.

@nikomatsakis
Copy link
Contributor Author

This test continues to be super annoying. I'm happy to just remove it, per @Mark-Simulacrum's suggestion.

@Mark-Simulacrum
Copy link
Member

Okay I've filed #65321 to remove the test -- seems like everyone is in agreement it does not pull its weight.

tmandry added a commit to tmandry/rust that referenced this issue Oct 11, 2019
…f8-test, r=nikomatsakis

Remove painful test that is not pulling its weight

Research suggests that we are not properly testing this case anyway, and
even if we were, it is unlikely that we will regress here -- or, perhaps
more accurately, if we do, I am uncertain that we care too much. It
definitely seems like an edge case, and one that is particularly
unlikely to occur as time goes on.

Fixes rust-lang#63520
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-linkage Area: linking into static, shared libraries and binaries A-testsuite Area: The testsuite used to check the correctness of rustc P-medium Medium priority T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants