Point at invalid utf-8 span on user's source code by estebank · Pull Request #135557 · rust-lang/rust

estebank · 2025-01-15T21:25:00Z

error: couldn't read `$DIR/not-utf8-bin-file.rs`: stream did not contain valid UTF-8
  --> $DIR/not-utf8-2.rs:6:5
   |
LL |     include!("not-utf8-bin-file.rs");
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   |
note: byte `193` is not valid utf-8
  --> $DIR/not-utf8-bin-file.rs:2:14
   |
LL |     let _ = "�|�␂!5�cc␕␂��";
   |              ^
   = note: this error originates in the macro `include` (in Nightly builds, run with -Z macro-backtrace for more info)

When we attempt to load a Rust source code file, if there is a OS file failure we try reading the file as bytes. If that succeeds we try to turn it into UTF-8. If that fails, we provide additional context about where the file has the first invalid UTF-8 character.

Fix #76869.

rustbot · 2025-01-15T21:25:09Z

r? @fee1-dead

rustbot has assigned @fee1-dead.
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 · 2025-01-15T21:25:12Z

Some changes occurred in src/tools/compiletest

cc @jieyouxu

compiler/rustc_builtin_macros/src/source_util.rs

compiler/rustc_parse/src/lib.rs

estebank · 2025-01-17T15:54:36Z

src/tools/compiletest/src/errors.rs

    rdr.lines()
        .enumerate()
+        // We want to ignore utf-8 failures in tests during collection of annotations.
+        .filter(|(_, line)| line.is_ok())


estebank · 2025-01-17T15:56:52Z

src/tools/tidy/src/tests_revision_unpaired_stdout_stderr.rs

            let mut expected_revisions = BTreeSet::new();

-            let contents = std::fs::read_to_string(test).unwrap();
+            let Ok(contents) = std::fs::read_to_string(test) else { continue };


...this, and another line in md-doc blow up if a test has non-utf-8 bytes.

I ended up removing a test of ".rs file with invalid few utf-8 bytes" because md-docs is in a separate repo. I feel we should make the entire test suite more resilient to these, but I think I can make that test I added and removed be in run-make...

compiler/rustc_builtin_macros/src/source_util.rs

fee1-dead · 2025-01-18T03:26:21Z

compiler/rustc_parse/src/lib.rs

    let source_file = psess.source_map().load_file(path).unwrap_or_else(|e| {
-        let msg = format!("couldn't read {}: {}", path.display(), e);
+        let msg = format!("couldn't read `{}`: {}", path.display(), e);


the error here could be an enum of an io error or a utf8 error, and store the information about the span and message to be reported, and that would probably help with deduplicating these code

fee1-dead · 2025-01-18T03:26:28Z

compiler/rustc_builtin_macros/src/source_util.rs

@@ -210,8 +210,34 @@ pub(crate) fn expand_include_str(
                MacEager::expr(cx.expr_str(cx.with_def_site_ctxt(bsp), interned_src))


maybe worth looking into unifying the logic here and the rustc_parse logic somehow?

I moved the logic to a separate function and called it from both places.

fee1-dead

r=me after squashing

``` error: couldn't read `$DIR/not-utf8-bin-file.rs`: stream did not contain valid UTF-8 --> $DIR/not-utf8-2.rs:6:5 | LL | include!("not-utf8-bin-file.rs"); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | note: `[193]` is not valid utf-8 --> $DIR/not-utf8-bin-file.rs:2:14 | LL | let _ = "�|�␂!5�cc␕␂��"; | ^ = note: this error originates in the macro `include` (in Nightly builds, run with -Z macro-backtrace for more info) ``` When we attempt to load a Rust source code file, if there is a OS file failure we try reading the file as bytes. If that succeeds we try to turn it into UTF-8. If *that* fails, we provide additional context about *where* the file has the first invalid UTF-8 character. Fix rust-lang#76869.

``` error: couldn't read `$DIR/not-utf8-bin-file.rs`: stream did not contain valid UTF-8 --> $DIR/not-utf8-2.rs:6:5 | LL | include!("not-utf8-bin-file.rs"); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | note: byte `193` is not valid utf-8 --> $DIR/not-utf8-bin-file.rs:2:14 | LL | let _ = "�|�␂!5�cc␕␂��"; | ^ = note: this error originates in the macro `include` (in Nightly builds, run with -Z macro-backtrace for more info) ``` CC rust-lang#76869, rust-lang#135557.

estebank · 2025-01-22T02:28:34Z

@bors r=fee1-dead

bors · 2025-01-22T02:28:36Z

📌 Commit 57dd42d has been approved by fee1-dead

It is now in the queue for this repository.

Point at invalid utf-8 span on user's source code ``` error: couldn't read `$DIR/not-utf8-bin-file.rs`: stream did not contain valid UTF-8 --> $DIR/not-utf8-2.rs:6:5 | LL | include!("not-utf8-bin-file.rs"); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | note: byte `193` is not valid utf-8 --> $DIR/not-utf8-bin-file.rs:2:14 | LL | let _ = "�|�␂!5�cc␕␂��"; | ^ = note: this error originates in the macro `include` (in Nightly builds, run with -Z macro-backtrace for more info) ``` When we attempt to load a Rust source code file, if there is a OS file failure we try reading the file as bytes. If that succeeds we try to turn it into UTF-8. If *that* fails, we provide additional context about *where* the file has the first invalid UTF-8 character. Fix rust-lang#76869.

Rollup of 8 pull requests Successful merges: - rust-lang#135557 (Point at invalid utf-8 span on user's source code) - rust-lang#135596 (Properly note when query stack is being cut off) - rust-lang#135638 (Make it possible to build GCC on CI) - rust-lang#135648 (support wasm inline assembly in `naked_asm!`) - rust-lang#135826 (Misc. `rustc_resolve` cleanups) - rust-lang#135827 (CI: free disk with in-tree script instead of GitHub Action) - rust-lang#135850 (Update the `wasm-component-ld` tool) - rust-lang#135855 (Only assert the `Parser` size on specific arches) r? `@ghost` `@rustbot` modify labels: rollup

Point at invalid utf-8 span on user's source code ``` error: couldn't read `$DIR/not-utf8-bin-file.rs`: stream did not contain valid UTF-8 --> $DIR/not-utf8-2.rs:6:5 | LL | include!("not-utf8-bin-file.rs"); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | note: byte `193` is not valid utf-8 --> $DIR/not-utf8-bin-file.rs:2:14 | LL | let _ = "�|�␂!5�cc␕␂��"; | ^ = note: this error originates in the macro `include` (in Nightly builds, run with -Z macro-backtrace for more info) ``` When we attempt to load a Rust source code file, if there is a OS file failure we try reading the file as bytes. If that succeeds we try to turn it into UTF-8. If *that* fails, we provide additional context about *where* the file has the first invalid UTF-8 character. Fix rust-lang#76869.

…iaskrgr Rollup of 10 pull requests Successful merges: - rust-lang#132983 (Edit dangling pointers ) - rust-lang#133154 (Reword resolve errors caused by likely missing crate in dep tree) - rust-lang#135409 (Fix ICE-133117: multiple never-pattern arm doesn't have false_edge_start_block) - rust-lang#135557 (Point at invalid utf-8 span on user's source code) - rust-lang#135596 (Properly note when query stack is being cut off) - rust-lang#135794 (Detect missing fields with default values and suggest `..`) - rust-lang#135814 (ci: use ghcr buildkit image) - rust-lang#135826 (Misc. `rustc_resolve` cleanups) - rust-lang#135837 (Remove test panic from File::open) - rust-lang#135856 (Library: Finalize dyn compatibility renaming) r? `@ghost` `@rustbot` modify labels: rollup

matthiaskrgr · 2025-01-22T19:35:32Z

@bors r-
#135891 (comment)

matthiaskrgr · 2025-01-22T19:36:04Z

oops wrong pr
@bors r=fee1-dead

bors · 2025-01-22T19:36:06Z

📌 Commit 57dd42d has been approved by fee1-dead

It is now in the queue for this repository.

estebank · 2025-01-22T19:37:01Z

oops wrong pr

You had me scared for a second there "oh, no! what fresh hell did this utf-8 handling cause during rollup?" XD

…iaskrgr Rollup of 9 pull requests Successful merges: - rust-lang#132983 (Edit dangling pointers ) - rust-lang#135409 (Fix ICE-133117: multiple never-pattern arm doesn't have false_edge_start_block) - rust-lang#135557 (Point at invalid utf-8 span on user's source code) - rust-lang#135596 (Properly note when query stack is being cut off) - rust-lang#135794 (Detect missing fields with default values and suggest `..`) - rust-lang#135814 (ci: use ghcr buildkit image) - rust-lang#135826 (Misc. `rustc_resolve` cleanups) - rust-lang#135837 (Remove test panic from File::open) - rust-lang#135856 (Library: Finalize dyn compatibility renaming) r? `@ghost` `@rustbot` modify labels: rollup

rustbot assigned fee1-dead Jan 15, 2025

This comment has been minimized.

Sign in to view

estebank force-pushed the wtf8 branch from 472d719 to ec81025 Compare January 15, 2025 22:13

rustbot added the A-tidy Area: The tidy tool label Jan 15, 2025

This comment has been minimized.

Sign in to view

estebank mentioned this pull request Jan 16, 2025

[ER] Line number for 'stream did not contain valid UTF-8' errors #76869

Closed

jieyouxu reviewed Jan 17, 2025

View reviewed changes

compiler/rustc_builtin_macros/src/source_util.rs Outdated Show resolved Hide resolved

compiler/rustc_parse/src/lib.rs Outdated Show resolved Hide resolved

estebank commented Jan 17, 2025

View reviewed changes

fee1-dead requested changes Jan 18, 2025

View reviewed changes

rustbot 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-review Status: Awaiting review from the assignee but also interested parties. labels Jan 18, 2025

estebank force-pushed the wtf8 branch from 0a30085 to 96f063e Compare January 18, 2025 21:19

This comment has been minimized.

Sign in to view

estebank force-pushed the wtf8 branch from 96f063e to 0b0d931 Compare January 18, 2025 21:31

estebank 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 Jan 18, 2025

fee1-dead approved these changes Jan 21, 2025

View reviewed changes

estebank force-pushed the wtf8 branch from 2cf9a61 to 57dd42d Compare January 22, 2025 00:52

estebank mentioned this pull request Jan 22, 2025

Add tests for source code with invalid utf-8 bytes #135854

Closed

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 Jan 22, 2025

jieyouxu mentioned this pull request Jan 22, 2025

Rollup of 8 pull requests #135862

Closed

matthiaskrgr mentioned this pull request Jan 22, 2025

Rollup of 10 pull requests #135891

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 Jan 22, 2025

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 Jan 22, 2025

matthiaskrgr mentioned this pull request Jan 22, 2025

Rollup of 9 pull requests #135896

Merged

bors merged commit b4266b0 into rust-lang:master Jan 23, 2025
6 checks passed

rustbot added this to the 1.86.0 milestone Jan 23, 2025

		@@ -210,8 +210,34 @@ pub(crate) fn expand_include_str(
		MacEager::expr(cx.expr_str(cx.with_def_site_ctxt(bsp), interned_src))

Uh oh!

Conversation

estebank commented Jan 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Jan 15, 2025

Uh oh!

rustbot commented Jan 15, 2025

Uh oh!

This comment has been minimized.

This comment has been minimized.

Uh oh!

Uh oh!

estebank Jan 17, 2025

Choose a reason for hiding this comment

Uh oh!

estebank Jan 17, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

fee1-dead Jan 18, 2025

Choose a reason for hiding this comment

Uh oh!

fee1-dead Jan 18, 2025

Choose a reason for hiding this comment

Uh oh!

estebank Jan 18, 2025

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

fee1-dead left a comment

Choose a reason for hiding this comment

Uh oh!

estebank commented Jan 22, 2025

Uh oh!

bors commented Jan 22, 2025

Uh oh!

matthiaskrgr commented Jan 22, 2025

Uh oh!

matthiaskrgr commented Jan 22, 2025

Uh oh!

bors commented Jan 22, 2025

Uh oh!

estebank commented Jan 22, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

estebank commented Jan 15, 2025 •

edited

Loading