Rustc memory exhaustion when formatting short code suggestion

[ad-anssi]

Code

fn f(
                                     x: u8
                                     y: u8,
) {}

fn main() {}

Meta

rustc --version --verbose:

rustc 1.48.0-dev
binary: rustc
commit-hash: 94b4de0e0793c8921d30e0fb886be712d17db6e5
commit-date: Fri Sep 11 02:35:01 2020 +0000
host: x86_64-unknown-linux-gnu
release: 1.48.0-dev
LLVM version: 11.0

Stable channel seems to be also affected from version 1.46.
The bug is reproducible in rust playground.

Error output

When trying to compile this code with rustc, nothing is displayed, and memory exhaustion happens, leading to either SIGABRT if user virtual memory is (u)limited, or memory swapping and eventually freezing the machine.
One would have expected syntax error as follows:

error: expected one of `!`, `(`, `)`, `+`, `,`, `::`, or `<`, found `y`
 --> ../bug.rs:3:38
  |
2 |   ...                   x: u8
  |                              - expected one of 7 possible tokens
  |  ____________________________|
  | |
3 | | ...                   y: u8,
| | |                       ^ unexpected token
| | |_
  |   help: missing `,`

error: aborting due to previous error

Analysis

Backtrace

Here is a gdb backtrace after SIGABRT:

#1  0x00007fffefd52535 in __GI_abort () at abort.c:79
#2  0x00007fffeff8ad47 in std::sys::unix::abort_internal ()
#3  0x00007fffeffac946 in std::process::abort ()
#4  0x00007fffeffb16ce in rust_oom ()
#5  0x00007ffff0018f27 in alloc::alloc::handle_alloc_error ()
#6  0x00007ffff57cde5b in alloc::raw_vec::RawVec<T,A>::reserve ()
#7  0x00007ffff579fdf3 in rustc_errors::styled_buffer::StyledBuffer::putc ()
#8  0x00007ffff57af21b in rustc_errors::emitter::EmitterWriter::emit_message_default ()
#9  0x00007ffff57aa3ac in <rustc_errors::emitter::EmitterWriter as rustc_errors::emitter::Emitter>::emit_diagnostic ()
#10 0x00007ffff57a698b in rustc_errors::HandlerInner::emit_diagnostic ()
#11 0x00007ffff57a658b in rustc_errors::Handler::emit_diagnostic ()
#12 0x00007ffff57b69e4 in rustc_errors::diagnostic_builder::DiagnosticBuilder::emit ()
#13 0x00007ffff4bbb03c in rustc_parse::parser::Parser::parse_paren_comma_seq ()
#14 0x00007ffff4c0ed9e in rustc_parse::parser::item::<impl rustc_parse::parser::Parser>::parse_fn_decl ()
#15 0x00007ffff4c032b5 in rustc_parse::parser::item::<impl rustc_parse::parser::Parser>::parse_item_kind ()
#16 0x00007ffff4c0253d in rustc_parse::parser::item::<impl rustc_parse::parser::Parser>::parse_item_common_ ()
#17 0x00007ffff4c02035 in rustc_parse::parser::item::<impl rustc_parse::parser::Parser>::parse_item_common ()
#18 0x00007ffff4c01def in rustc_parse::parser::item::<impl rustc_parse::parser::Parser>::parse_item ()
#19 0x00007ffff4c0183a in rustc_parse::parser::item::<impl rustc_parse::parser::Parser>::parse_mod ()
#20 0x00007ffff4c015e5 in rustc_parse::parser::item::<impl rustc_parse::parser::Parser>::parse_crate_mod ()
#21 0x00007ffff4baa8cd in rustc_parse::parse_crate_from_file ()
#22 0x00007ffff0bb717d in rustc_session::utils::<impl rustc_session::session::Session>::time ()
#23 0x00007ffff0adad51 in rustc_interface::passes::parse ()
#24 0x00007ffff0b94e0a in rustc_interface::queries::Query<T>::compute ()
#25 0x00007ffff0ae4544 in rustc_interface::queries::Queries::parse ()
#26 0x00007ffff09e2ef8 in rustc_interface::queries::<impl rustc_interface::interface::Compiler>::enter ()
#27 0x00007ffff09b4530 in rustc_span::with_source_map ()
#28 0x00007ffff09e3c95 in rustc_interface::interface::create_compiler_and_run ()
#29 0x00007ffff09d51e1 in scoped_tls::ScopedKey<T>::set ()
#30 0x00007ffff09b4c37 in rustc_span::with_session_globals ()
#31 0x00007ffff09b2aa1 in std::sys_common::backtrace::__rust_begin_short_backtrace ()
#32 0x00007ffff09a9476 in std::panicking::try ()
#33 0x00007ffff096a4fe in core::ops::function::FnOnce::call_once{{vtable-shim}} ()
#34 0x00007fffeff92788 in <alloc::boxed::Box<F> as core::ops::function::FnOnce<A>>::call_once ()
#35 0x00007fffeff86b4a in std::sys::unix::thread::Thread::new::thread_start ()
#36 0x00007fffefce5fb7 in start_thread (arg=<optimized out>) at pthread_create.c:486
#37 0x00007fffefe271af in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:95

Finding regression

After ulimit -v 8000000, we used cargo-bisect-rustc with the following script:

#!/bin/sh

! cargo build 2>&1 | grep "SIGABRT"

Here is the execution result:

[...]
installing nightly-2020-07-07
std for x86_64-unknown-linux-gnu: 16.03 MB / 16.03 MB [==========================================] 100.00 % 14.01 MB/s testing...
RESULT: nightly-2020-07-07, ===> No
uninstalling nightly-2020-07-07

installing nightly-2020-07-08
std for x86_64-unknown-linux-gnu: 16.02 MB / 16.02 MB [==========================================] 100.00 % 13.87 MB/s testing...
RESULT: nightly-2020-07-08, ===> Yes
uninstalling nightly-2020-07-08

searched toolchains nightly-2020-01-02 through nightly-2020-09-10


********************************************************************************
Regression in nightly-2020-07-08
********************************************************************************

fetching https://static.rust-lang.org/dist/2020-07-07/channel-rust-nightly-git-commit-hash.txt
nightly manifest 2020-07-07: 40 B / 40 B [======================================================] 100.00 % 521.03 KB/s converted 2020-07-07 to 0c03aee8b81185d65b5821518661c30ecdb42de5
fetching https://static.rust-lang.org/dist/2020-07-08/channel-rust-nightly-git-commit-hash.txt
nightly manifest 2020-07-08: 40 B / 40 B [======================================================] 100.00 % 585.71 KB/s converted 2020-07-08 to 8ac1525e091d3db28e67adcbbd6db1e1deaa37fb
looking for regression commit between 2020-07-07 and 2020-07-08
opening existing repository at "rust.git"
refreshing repository
fetching (via local git) commits from 0c03aee8b81185d65b5821518661c30ecdb42de5 to 8ac1525e091d3db28e67adcbbd6db1e1deaa37fb
opening existing repository at "rust.git"
refreshing repository
looking up first commit
looking up second commit
checking that commits are by bors and thus have ci artifacts...
finding bors merge commits
found 6 bors merge commits in the specified range
  commit[0] 2020-07-05UTC: Auto merge of #74073 - Manishearth:rollup-faqo9lx, r=Manishearth
  commit[1] 2020-07-06UTC: Auto merge of #73978 - Mark-Simulacrum:shrink-paramenv, r=nnethercote
  commit[2] 2020-07-07UTC: Auto merge of #74117 - Manishearth:rollup-ds7z0kx, r=Manishearth
  commit[3] 2020-07-07UTC: Auto merge of #73562 - poliorcetics:e0432-to-edition2018, r=GuillaumeGomez
  commit[4] 2020-07-07UTC: Auto merge of #74059 - RalfJung:miri-uninit-validation, r=oli-obk
  commit[5] 2020-07-07UTC: Auto merge of #74006 - euclio:sys-unix-static-mut, r=oli-obk
validated commits found, specifying toolchains

installing 0c03aee8b81185d65b5821518661c30ecdb42de5
std for x86_64-unknown-linux-gnu: 16.03 MB / 16.03 MB [===========================================] 100.00 % 5.26 MB/s testing...
RESULT: 0c03aee8b81185d65b5821518661c30ecdb42de5, ===> No
uninstalling 0c03aee8b81185d65b5821518661c30ecdb42de5

installing 8ac1525e091d3db28e67adcbbd6db1e1deaa37fb
std for x86_64-unknown-linux-gnu: 16.02 MB / 16.02 MB [===========================================] 100.00 % 4.22 MB/s testing...
RESULT: 8ac1525e091d3db28e67adcbbd6db1e1deaa37fb, ===> Yes
uninstalling 8ac1525e091d3db28e67adcbbd6db1e1deaa37fb

installing 70f9d23b916f2db7da711aa4a0317a218997ba42
std for x86_64-unknown-linux-gnu: 16.01 MB / 16.01 MB [===========================================] 100.00 % 5.46 MB/s testing...
RESULT: 70f9d23b916f2db7da711aa4a0317a218997ba42, ===> Yes
uninstalling 70f9d23b916f2db7da711aa4a0317a218997ba42

installing 8981dbbc36f1575b0a417b6849767bde29e7c6b4
std for x86_64-unknown-linux-gnu: 16.02 MB / 16.02 MB [===================================================================================================================================================================] 100.00 % 5.25 MB/s testing...
RESULT: 8981dbbc36f1575b0a417b6849767bde29e7c6b4, ===> No
uninstalling 8981dbbc36f1575b0a417b6849767bde29e7c6b4

searched toolchains 0c03aee8b81185d65b5821518661c30ecdb42de5 through 8ac1525e091d3db28e67adcbbd6db1e1deaa37fb


********************************************************************************
Regression in 70f9d23b916f2db7da711aa4a0317a218997ba42
********************************************************************************

==================================================================================
= Please file this regression report on the rust-lang/rust GitHub repository     =
=        New issue: https://github.com/rust-lang/rust/issues/new                 =
=     Known issues: https://github.com/rust-lang/rust/issues                     =
= Copy and paste the text below into the issue report thread.  Thanks!           =
==================================================================================

searched nightlies: from nightly-2020-01-02 to nightly-2020-09-10
regressed nightly: nightly-2020-07-08
searched commits: from https://github.com/rust-lang/rust/commit/0c03aee8b81185d65b5821518661c30ecdb42de5 to https://github.com/rust-lang/rust/commit/8ac1525e091d3db28e67adcbbd6db1e1deaa37fb
regressed commit: https://github.com/rust-lang/rust/commit/70f9d23b916f2db7da711aa4a0317a218997ba42

bisected with <a href='https://github.com/rust-lang/cargo-bisect-rustc'>cargo-bisect-rustc</a> v0.5.2

Host triple: x86_64-unknown-linux-gnu
Reproduce with:

cargo bisect-rustc --start=2020-01-02 --script=./script.sh 

The bug was introduced when auditting hidden/short code suggestions #73953 with the change of the span used when reporting a missing ',' suggestion (https://github.com/rust-lang/rust/pull/73953/files#diff-da8eccd50b4abe306482925ebf6e6a90R702).

After further investigation, we found that the problem resides in the construction of the error message, in the module rustc_errors. When applying left margin shift in the function FileWithAnnotatedLines::render_source_line, the column number can wrap over usize (emitter.rs#962), thus the function draw_range tries to putc way too many '_' chars into the StyledBuffer.

A fix would be to saturating_sub when computing column number (in emitter.rs, at line 962, and also at lines 969 and 970), in the same way it is already done at different points of the same function.
For completion, further inspection of the multispan data would be required to check whether they are correct.

Aurélien Deharbe and Vincent Dagonneau

[tesuji]

@rustbot prioritize

[jyn514]

It sounds like there are two issues here:

1. a regression from stable to stable, the diagnostic added in Audit hidden/short code suggestions #73953 and present in 1.46.
2. A bug in the rustc_errors library which allows normal diagnostics to exhaust memory, which has possible existed much longer.

Does that sound accurate?

[ad-anssi]

Yes exactly. (2) being discovered thanks to (1).
The PR #76598 tries to fix (2) (the memory bug in rustc_errors).
I'd like to participate to fixing (1) but I don't fully understand yet why this line about span has been patched in #73953.

[jyn514]

It seems like 1) should be fixed automatically if you fix 2), no? Since there's nothing wrong with the diagnostic itself, it just hit an edge case in the library.

[ad-anssi]

I agree with that. Once the edge case will be correctly handled, the diagnostic itself should be OK. And this is validated according to the test case which prints the diagnostic in stderr with the patch, but consumes memory in master.
Still, is this the correct and expected form of such a diagnostic message?

[ad-anssi]

Well, when adding rustfix (and corresponding .fixed file) for this new test case, it passes only when reverting identified regression line from #73953. But then another test case breaks (similar_tokens.rs). I guess that:

• the issue with 'similar_token' test case was the initial motivation of the patch,
• the difference between the two is that in one case the expected token should replace bad token whereas in the other case the expected token should be inserted by rustfix.

I'll continue to explore this path.

[ad-anssi]

I finally managed to pass all tests with rustfix, after the addition of a test for multiline span when calling span_suggestion_short function. Should I push this patch in the same PR that fixes the memory bug? Since there are two distinct problems, what is the preferred way to treat this case?

[jyn514]

I'm confused by what problem you're trying to solve. I guess open a new PR, but I don't know what it's fixing.

[ad-anssi]

I'm sorry if I wasn't clear.

The PR #76598 fixes the memory bug in rustc_errors, but the new test case cannot pass properly. As you can see in the expected stderr, the error message gets strangely formatted (note the starting '|' at lines 9 and 10). Moreover, rustfix is not able to fix the missing ',' by following, since the spanning of the suggestion is incorrect.

But I have a new patch to submit to solve these problems. It's about rustc_parse and the way span is selected when reporting a missing char suggestion.

[jyn514]

I'd say add it on to your existing PR, since it sounds like it depends on those changes.

[ad-anssi]

Indeed, I just pushed it. Thank you for your answers.

[jyn514]

Fixed by #76598