Skip to content

Validate source snippet when format input is raw string#152277

Open
gurry wants to merge 1 commit intorust-lang:mainfrom
gurry:114865-ice-format-args
Open

Validate source snippet when format input is raw string#152277
gurry wants to merge 1 commit intorust-lang:mainfrom
gurry:114865-ice-format-args

Conversation

@gurry
Copy link
Contributor

@gurry gurry commented Feb 7, 2026

Fixes #114865

The issue occurred because the user's proc macro respanned the format arg to an unrelated multi-byte string and we ICE'd by landing in the middle of a multi-byte char.

This PR adds validation that prevents the parser from trying to walk such obviously wrong snippets. Such validation already existed for non-raw strings. This PR adds it for raw strings as well.

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Feb 7, 2026
@gurry gurry force-pushed the 114865-ice-format-args branch from 70647fa to 4483628 Compare February 7, 2026 09:59
@gurry gurry marked this pull request as ready for review February 7, 2026 12:56
@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 7, 2026
@rustbot rustbot removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Feb 7, 2026
@rustbot
Copy link
Collaborator

rustbot commented Feb 7, 2026

r? @TaKO8Ki

rustbot has assigned @TaKO8Ki.
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

Why was this reviewer chosen?

The reviewer was selected based on:

  • Owners of files modified in this PR: compiler
  • compiler expanded to 21 candidates
  • Random selection from 12 candidates

Copy link
Member

@TaKO8Ki TaKO8Ki left a comment

Choose a reason for hiding this comment

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

Thank you. I found another related ICE while reviewing the PR, so if possible, could you take a look at fixing that as well?

The implementation might have a somewhat broader impact, so I’d like to review it more carefully on my side. Please give me a little more time.

View changes since this review

let suffix_len = nr_hashes + 1; // closing " + hashes
if snippet.len() >= prefix_len + suffix_len // is sufficiently long
&& snippet.starts_with('r')
&& snippet[1..1 + nr_hashes].chars().all(|c| c == '#')
Copy link
Member

@TaKO8Ki TaKO8Ki Feb 14, 2026

Choose a reason for hiding this comment

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

I found foo!(r字字); causes an ICE with your implementation in this line. Could you check it? It pre-existed, but the ICE error message changed in the PR.

Copy link
Contributor Author

@gurry gurry Feb 17, 2026

Choose a reason for hiding this comment

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

@TaKO8Ki I have pushed a fix for this. No longer trying to convert bytes to chars while doing comparisons.

@gurry
Copy link
Contributor Author

gurry commented Feb 15, 2026

Thank you. I found another related ICE while reviewing the PR, so if possible, could you take a look at fixing that as well?

The implementation might have a somewhat broader impact, so I’d like to review it more carefully on my side. Please give me a little more time.

View changes since this review

No worries. Please take your time. I'll fix the second ICE in the meanwhile.

@gurry gurry force-pushed the 114865-ice-format-args branch from a1cee4e to 42a2960 Compare February 16, 2026 13:08
@gurry gurry force-pushed the 114865-ice-format-args branch from 42a2960 to 6b3d04e Compare February 16, 2026 13:11
@TaKO8Ki
Copy link
Member

TaKO8Ki commented Feb 26, 2026

@bors try jobs=x86_64-gnu

@rust-bors

This comment has been minimized.

rust-bors bot pushed a commit that referenced this pull request Feb 26, 2026
Validate source snippet when format input is raw string


try-job: x86_64-gnu
@rust-bors
Copy link
Contributor

rust-bors bot commented Feb 26, 2026

☀️ Try build successful (CI)
Build commit: 2546166 (254616605b508b115246302fd7e3fd0b12bf7af1, parent: 1ed488274bec5bf5cfe6bf7a1cc089abcc4ebd68)

@TaKO8Ki
Copy link
Member

TaKO8Ki commented Feb 26, 2026

@bors try @rust-timer queue

@rust-timer

This comment has been minimized.

@rust-bors

This comment has been minimized.

rust-bors bot pushed a commit that referenced this pull request Feb 26, 2026
Validate source snippet when format input is raw string
@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Feb 26, 2026
@rust-bors
Copy link
Contributor

rust-bors bot commented Feb 26, 2026

☀️ Try build successful (CI)
Build commit: 0421599 (04215991e86652a3338382526b1fbca9130d42c2, parent: 1ed488274bec5bf5cfe6bf7a1cc089abcc4ebd68)

@rust-timer

This comment has been minimized.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (0421599): comparison URL.

Overall result: ❌ regressions - no action needed

Benchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf.

@bors rollup=never
@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

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

Max RSS (memory usage)

Results (primary -2.5%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

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

Cycles

Results (primary -2.8%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

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

Binary size

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

Bootstrap: 492.161s -> 479.296s (-2.61%)
Artifact size: 395.78 MiB -> 395.80 MiB (0.00%)

@rustbot rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Feb 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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.

Projects

None yet

4 participants