compiletest crashes on CI with: expected success, got: signal: 11 (core dumped)

[klensy]

runner: x86_64-gnu-llvm-12, ubuntu-20.04-xl
Occured in multiple pr across CI:
#93361: https://github.com/rust-lang/rust/runs/4969749566?check_suite_focus=true
#93381: https://github.com/rust-lang/rust/runs/4969258442?check_suite_focus=true
#93375: https://github.com/rust-lang/rust/runs/4967313356?check_suite_focus=true
#93239: https://github.com/rust-lang/rust/runs/4963643970?check_suite_focus=true
and others

[JakobDegen]

Possibly this was #91840 ? Looking at what's been merged in the last day, that seems to be the most relevant thing

[matthiaskrgr]

You mean it might have introduced a new miscompilation that made compiletest crash?

[JakobDegen]

Maybe. Its also possible that there was some existing UB elsewhere that happened to get set off by this, don't really know. My observation just mostly comes from where the segfault is happening and what commits in the last day have touched nearby code

[klensy]

Warning: Skipping "src/test/ui": not a regular file or directory
Warning: Skipping "src/test/mir-opt": not a regular file or directory

Why this even triggered? Looks unrelated, but still suspicious.

[ehuss]

Those warnings are unrelated. I'll post a patch soon to fix it.

EDIT: #93399

[klensy]

Funny thing: #93352 been merged even with crashed compiletest: #93352 (comment), how this works?

[fee1-dead]

Funny thing: #93352 been merged even with crashed compiletest: #93352 (comment), how this works?

The comment by @rust-log-analyzer is about the CI run not the bors run.

[klensy]

The comment by @rust-log-analyzer is about the CI run not the bors run.

Yes, it failed on PR check, not on check-all-platforms-and-merge.
So fast approving PR, so it merged even with failed PR check :-(

[klensy]

@JakobDegen can you try to revert that #91840 pr and see, will it fail or not on test?

[JakobDegen]

Yeah, I can do that before the end of the day

[JakobDegen]

Reverting #91840 didn't fix it. Its still very suspicious that this started right after that got merged though. I'll leave the branch up on my fork for a little in case anyone else wants to re-run things or something.

[tmiasko]

The issue appears to be caused by a copy of dirent made in <ReadDir as Iterator>::next. Only a part of the structure as described by d_reclen is actually dereferenceable:

rust/library/std/src/sys/unix/fs.rs

Lines 492 to 497 in ca43894

	let ret = DirEntry {
	entry: *entry_ptr,
	// d_name is guaranteed to be null-terminated.
	name: CStr::from_ptr((*entry_ptr).d_name.as_ptr()).to_owned(),
	dir: Arc::clone(&self.inner),
	};

Which was recently changed for Linux and Android in #92778.

[Mark-Simulacrum]

It sounds like reverting that PR would be a good first step, since the bugs it fixes look less critical to me given that diagnosis.

(Of course, maybe there is a more forward-moving fix, but a revert seems like something we can merge with less caution).

cc @joshtriplett @tavianator

[tavianator]

I'll take a look. @tmiasko do you mean the entry: *entry_ptr part? I didn't expect d_reclen < sizeof(struct dirent) but maybe it's possible.

[tavianator]

Oh indeed that seems to happen all the time. One sec I'll put up a patch