Fix ICE when there is a non-Unicode entry in the incremental crate directory #124112

beetrees · 2024-04-18T07:00:36Z

Fix the ICE that occurs when there is a non-Unicode entry in the incremental crate directory by replacing uses of to_string_lossy + assert_no_characters_lost with to_str. The added test would cause the compiler to ICE before this PR.

rustbot · 2024-04-18T07:00:43Z

r? @Nadrieril

rustbot has assigned @Nadrieril.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

rustbot · 2024-04-18T07:00:45Z

Some changes occurred in run-make tests.

cc @jieyouxu

Nadrieril · 2024-04-18T18:43:59Z

Thanks!

@bors r+ rollup

bors · 2024-04-18T18:44:01Z

📌 Commit ed8f351 has been approved by Nadrieril

It is now in the queue for this repository.

…rieril Fix ICE when there is a non-Unicode entry in the incremental crate directory Fix the ICE that occurs when there is a non-Unicode entry in the incremental crate directory by replacing uses of `to_string_lossy` + `assert_no_characters_lost` with `to_str`. The added test would cause the compiler to ICE before this PR.

workingjubilee · 2024-04-18T23:03:29Z

I believe this is failing rollups:

#124133 (comment)

@bors r-

workingjubilee · 2024-04-18T23:53:42Z

I tested my hypothesis and was wrong.

@bors r=Nadrieril

bors · 2024-04-18T23:53:45Z

📌 Commit ed8f351 has been approved by Nadrieril

It is now in the queue for this repository.

…rieril Fix ICE when there is a non-Unicode entry in the incremental crate directory Fix the ICE that occurs when there is a non-Unicode entry in the incremental crate directory by replacing uses of `to_string_lossy` + `assert_no_characters_lost` with `to_str`. The added test would cause the compiler to ICE before this PR.

…kingjubilee Rollup of 9 pull requests Successful merges: - rust-lang#117919 (Introduce perma-unstable `wasm-c-abi` flag) - rust-lang#123406 (Force exhaustion in iter::ArrayChunks::into_remainder) - rust-lang#123752 (Properly handle emojis as literal prefix in macros) - rust-lang#123935 (Don't inline integer literals when they overflow - new attempt) - rust-lang#123980 ( Add an opt-in to store incoming edges in `VecGraph` + misc) - rust-lang#124019 (Use raw-dylib for Windows synchronization functions) - rust-lang#124110 (Fix negating `f16` and `f128` constants) - rust-lang#124112 (Fix ICE when there is a non-Unicode entry in the incremental crate directory) - rust-lang#124116 (when suggesting RUST_BACKTRACE=1, add a special note for Miri's env var isolation) r? `@ghost` `@rustbot` modify labels: rollup

beetrees · 2024-04-19T04:16:33Z

I've fixed the test failure that was caused by some filesystems not supporting non-Unicode paths.

rustbot · 2024-04-19T04:30:26Z

The run-make-support library was changed

cc @jieyouxu

These commits modify the Cargo.lock file. Unintentional changes to Cargo.lock can be introduced when switching branches and rebasing PRs.

If this was unintentional then you should revert the changes before this PR is merged.
Otherwise, you can ignore this comment.

ChrisDenton · 2024-04-19T04:59:43Z

tests/run-make/non-unicode-in-incremental-dir/rmake.rs

+    #[cfg(windows)]
+    let non_unicode: std::ffi::OsString = std::os::windows::ffi::OsStringExt::from_wide(&[0xD800]);
+    match std::fs::create_dir(tmp_dir().join(&non_unicode)) {
+        Err(e) if e.raw_os_error() == Some(libc::EILSEQ) => {


That error code assumes create_dir is implemented using a POSIX API. That isn't true on Windows and may not even be true on Unix OSes (depending on how the create_dir is implemented).

I don't see EILSEQ referenced anywhere in std so I don't think there's a portable way to detect this. You can always try to create a file with a unicode path first to ensure the test doesn't silently fail for other permissions reasons.

If we fail here, then surely we also fail in incremental when trying to create a directory with non-Unicode characters? Would that also ICE?

Do we even want to support non-Unicode incremental directory paths, given it is not at all portable (seeing that there doesn't seem to be an easy way to even test this behavior portably)? Would it make more sense to convert the ICE to a fatal error?

Incremental will never create a directory with a non-Unicode name - session directory names have a specific format and are always valid Unicode. The ICE would occur (before this PR) if something else had created a file/directory with a non-Unicode name in the incremental crate directory. When find_source_directory iterates over the incremental crate directory to find the most recent published session directory, it ignores any entries that do not have names in the expected format (checked with is_session_directory() etc.). However, before this PR, instead of ignoring entry names with non-Unicode characters in them (as session directories always have valid Unicode names) the compiler would instead ICE in the assert_no_characters_lost() function.

I've gone with something similar to @Nadrieril's suggestion and checked if creating a directory with a valid Unicode name would succeed if creating a directory with a non-Unicode name failed.

Nadrieril · 2024-04-19T06:08:13Z

@bors r-

…rectory

Nadrieril · 2024-04-21T12:27:09Z

Thanks!

@bors r+

bors · 2024-04-21T12:27:11Z

📌 Commit 71f751d has been approved by Nadrieril

It is now in the queue for this repository.

…eril Fix ICE when there is a non-Unicode entry in the incremental crate directory Fix the ICE that occurs when there is a non-Unicode entry in the incremental crate directory by replacing uses of `to_string_lossy` + `assert_no_characters_lost` with `to_str`. The added test would cause the compiler to ICE before this PR.

bors · 2024-04-21T15:11:32Z

⌛ Testing commit 71f751d with merge a55d3d7...

rust-log-analyzer · 2024-04-21T17:00:30Z

The job x86_64-gnu-aux failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

bors · 2024-04-21T17:00:55Z

💔 Test failed - checks-actions

Nadrieril · 2024-04-21T20:38:20Z

Looks unrelated

@bors retry

bors · 2024-04-22T04:01:02Z

⌛ Testing commit 71f751d with merge 3288583...

bors · 2024-04-22T06:05:59Z

☀️ Test successful - checks-actions
Approved by: Nadrieril
Pushing 3288583 to master...

rust-timer · 2024-04-22T08:39:50Z

Finished benchmarking commit (3288583): comparison URL.

Overall result: ✅ improvements - no action needed

@rustbot label: -perf-regression

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-0.2%	[-0.2%, -0.2%]	1
All ❌✅ (primary)	-	-	0

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	4.1%	[4.1%, 4.1%]	1
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	4.1%	[4.1%, 4.1%]	1

Cycles

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-2.5%	[-2.5%, -2.5%]	1
All ❌✅ (primary)	-	-	0

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 672.434s -> 671.523s (-0.14%)
Artifact size: 315.47 MiB -> 315.42 MiB (-0.02%)

rustbot assigned Nadrieril Apr 18, 2024

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Apr 18, 2024

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 18, 2024

GuillaumeGomez mentioned this pull request Apr 18, 2024

Rollup of 10 pull requests #124131

Closed

GuillaumeGomez mentioned this pull request Apr 18, 2024

Rollup of 7 pull requests #124133

Closed

bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Apr 18, 2024

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Apr 18, 2024

workingjubilee mentioned this pull request Apr 18, 2024

Rollup of 9 pull requests #124142

Closed

beetrees force-pushed the incremental-os-str branch from ed8f351 to c71cfee Compare April 19, 2024 04:14

beetrees force-pushed the incremental-os-str branch from c71cfee to 6b9767d Compare April 19, 2024 04:30

ChrisDenton reviewed Apr 19, 2024

View reviewed changes

bors removed the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Apr 19, 2024

bors added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Apr 19, 2024

Fix ICE when there is a non-Unicode entry in the incremental crate di…

71f751d

…rectory

beetrees force-pushed the incremental-os-str branch from 6b9767d to 71f751d Compare April 21, 2024 10:28

beetrees requested a review from Nadrieril April 21, 2024 10:29

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Apr 21, 2024

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 21, 2024

Nadrieril removed their request for review April 21, 2024 12:27

bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Apr 21, 2024

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 21, 2024

bors added the merged-by-bors This PR was explicitly merged by bors. label Apr 22, 2024

bors merged commit 3288583 into rust-lang:master Apr 22, 2024
13 checks passed

rustbot added this to the 1.79.0 milestone Apr 22, 2024

beetrees deleted the incremental-os-str branch April 22, 2024 06:23

bors mentioned this pull request Apr 22, 2024

Stabilize the size of incr comp object file names #123441

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix ICE when there is a non-Unicode entry in the incremental crate directory #124112

Fix ICE when there is a non-Unicode entry in the incremental crate directory #124112

beetrees commented Apr 18, 2024

rustbot commented Apr 18, 2024

rustbot commented Apr 18, 2024

Nadrieril commented Apr 18, 2024

bors commented Apr 18, 2024

workingjubilee commented Apr 18, 2024

workingjubilee commented Apr 18, 2024

bors commented Apr 18, 2024

beetrees commented Apr 19, 2024

rustbot commented Apr 19, 2024

ChrisDenton Apr 19, 2024

Nadrieril Apr 20, 2024

jieyouxu Apr 20, 2024

beetrees Apr 21, 2024 •

edited

Loading

beetrees Apr 21, 2024

Nadrieril commented Apr 19, 2024

Nadrieril commented Apr 21, 2024

bors commented Apr 21, 2024

bors commented Apr 21, 2024

rust-log-analyzer commented Apr 21, 2024

bors commented Apr 21, 2024

Nadrieril commented Apr 21, 2024

bors commented Apr 22, 2024

bors commented Apr 22, 2024

rust-timer commented Apr 22, 2024

Fix ICE when there is a non-Unicode entry in the incremental crate directory #124112

Fix ICE when there is a non-Unicode entry in the incremental crate directory #124112

Conversation

beetrees commented Apr 18, 2024

rustbot commented Apr 18, 2024

rustbot commented Apr 18, 2024

Nadrieril commented Apr 18, 2024

bors commented Apr 18, 2024

workingjubilee commented Apr 18, 2024

workingjubilee commented Apr 18, 2024

bors commented Apr 18, 2024

beetrees commented Apr 19, 2024

rustbot commented Apr 19, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

beetrees Apr 21, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Nadrieril commented Apr 19, 2024

Nadrieril commented Apr 21, 2024

bors commented Apr 21, 2024

bors commented Apr 21, 2024

rust-log-analyzer commented Apr 21, 2024

bors commented Apr 21, 2024

Nadrieril commented Apr 21, 2024

bors commented Apr 22, 2024

bors commented Apr 22, 2024

rust-timer commented Apr 22, 2024

Overall result: ✅ improvements - no action needed

beetrees Apr 21, 2024 •

edited

Loading