rustc driver: Remove argument 0 before at-expansion to prevent ICE #109084

dekrain · 2023-03-13T17:46:07Z

Under Unix-based operating systems, when I execute rustc by setting argv0 to @/dev/null, it will expand command-line arguments from this file, leading to an empty arglist, which then triggers an ICE by trying to remove first argument.

The panic message is this:

thread 'main' panicked at 'range start index 1 out of range for slice of length 0', compiler/rustc_driver/src/lib.rs:972:17

My fix is to remove the first argument before expanding arguments.

Full backtrace

% (exec -a @/dev/null `rustup which rustc`)
thread 'main' panicked at 'range start index 1 out of range for slice of length 0', compiler/rustc_driver/src/lib.rs:972:17
stack backtrace:
   0:     0x7fcec776659a - std::backtrace_rs::backtrace::libunwind::trace::h595f06c70adcc478
                               at /rustc/2c8cc343237b8f7d5a3c3703e3a87f2eb2c54a74/library/std/src/../../backtrace/src/backtrace/libunwind.rs:93:5
   1:     0x7fcec776659a - std::backtrace_rs::backtrace::trace_unsynchronized::h177a0149c76cdde9
                               at /rustc/2c8cc343237b8f7d5a3c3703e3a87f2eb2c54a74/library/std/src/../../backtrace/src/backtrace/mod.rs:66:5
   2:     0x7fcec776659a - std::sys_common::backtrace::_print_fmt::hc0701fd2c3530c58
                               at /rustc/2c8cc343237b8f7d5a3c3703e3a87f2eb2c54a74/library/std/src/sys_common/backtrace.rs:65:5
   3:     0x7fcec776659a - <std::sys_common::backtrace::_print::DisplayBacktrace as core::fmt::Display>::fmt::hd4cd115d8750fd6c
                               at /rustc/2c8cc343237b8f7d5a3c3703e3a87f2eb2c54a74/library/std/src/sys_common/backtrace.rs:44:22
   4:     0x7fcec77c839e - core::fmt::write::h93e2f5923c7eca08
                               at /rustc/2c8cc343237b8f7d5a3c3703e3a87f2eb2c54a74/library/core/src/fmt/mod.rs:1213:17
   5:     0x7fcec7756be5 - std::io::Write::write_fmt::h8162dbb45f0b9e62
                               at /rustc/2c8cc343237b8f7d5a3c3703e3a87f2eb2c54a74/library/std/src/io/mod.rs:1682:15
   6:     0x7fcec7766365 - std::sys_common::backtrace::_print::h1835ef8a8f9066da
                               at /rustc/2c8cc343237b8f7d5a3c3703e3a87f2eb2c54a74/library/std/src/sys_common/backtrace.rs:47:5
   7:     0x7fcec7766365 - std::sys_common::backtrace::print::hcb5e6388b9235f41
                               at /rustc/2c8cc343237b8f7d5a3c3703e3a87f2eb2c54a74/library/std/src/sys_common/backtrace.rs:34:9
   8:     0x7fcec776912f - std::panicking::default_hook::{{closure}}::h9c084969ccf9a722
                               at /rustc/2c8cc343237b8f7d5a3c3703e3a87f2eb2c54a74/library/std/src/panicking.rs:267:22
   9:     0x7fcec7768e6b - std::panicking::default_hook::h68fa2ba3c3c6c12f
                               at /rustc/2c8cc343237b8f7d5a3c3703e3a87f2eb2c54a74/library/std/src/panicking.rs:286:9
  10:     0x7fcecaab56e4 - <rustc_driver[f4ad927b3c57833d]::DEFAULT_HOOK::{closure#0}::{closure#0} as core[d16e85342ea223d9]::ops::function::FnOnce<(&core[d16e85342ea223d9]::panic::panic_info::PanicInfo,)>>::call_once::{shim:vtable#0}
  11:     0x7fcec776996a - <alloc::boxed::Box<F,A> as core::ops::function::Fn<Args>>::call::h4e6ced11e07d8b24
                               at /rustc/2c8cc343237b8f7d5a3c3703e3a87f2eb2c54a74/library/alloc/src/boxed.rs:2002:9
  12:     0x7fcec776996a - std::panicking::rust_panic_with_hook::h8d5c434518ef298c
                               at /rustc/2c8cc343237b8f7d5a3c3703e3a87f2eb2c54a74/library/std/src/panicking.rs:692:13
  13:     0x7fcec77696e9 - std::panicking::begin_panic_handler::{{closure}}::hf33414f5dabf6faf
                               at /rustc/2c8cc343237b8f7d5a3c3703e3a87f2eb2c54a74/library/std/src/panicking.rs:579:13
  14:     0x7fcec7766a4c - std::sys_common::backtrace::__rust_end_short_backtrace::hc50389427413bb75
                               at /rustc/2c8cc343237b8f7d5a3c3703e3a87f2eb2c54a74/library/std/src/sys_common/backtrace.rs:137:18
  15:     0x7fcec77693f2 - rust_begin_unwind
                               at /rustc/2c8cc343237b8f7d5a3c3703e3a87f2eb2c54a74/library/std/src/panicking.rs:575:5
  16:     0x7fcec77c4d43 - core::panicking::panic_fmt::h2de7a7938f816de8
                               at /rustc/2c8cc343237b8f7d5a3c3703e3a87f2eb2c54a74/library/core/src/panicking.rs:64:14
  17:     0x7fcec77cb492 - core::slice::index::slice_start_index_len_fail_rt::h0c87d85ce11d10f6
                               at /rustc/2c8cc343237b8f7d5a3c3703e3a87f2eb2c54a74/library/core/src/slice/index.rs:53:5
  18:     0x7fcec77cb416 - core::slice::index::slice_start_index_len_fail::h504609f2a6b168d1
                               at /rustc/2c8cc343237b8f7d5a3c3703e3a87f2eb2c54a74/library/core/src/slice/index.rs:41:9
  19:     0x7fceca0eca1f - rustc_driver[f4ad927b3c57833d]::handle_options
  20:     0x7fceca0e037f - <rustc_driver[f4ad927b3c57833d]::RunCompiler>::run
  21:     0x7fceca0dfd0d - <core[d16e85342ea223d9]::panic::unwind_safe::AssertUnwindSafe<rustc_driver[f4ad927b3c57833d]::main::{closure#0}> as core[d16e85342ea223d9]::ops::function::FnOnce<()>>::call_once
  22:     0x7fceca17ce89 - rustc_driver[f4ad927b3c57833d]::main
  23:     0x564f5f008a87 - rustc_main[f164605d1302e295]::main
  24:     0x564f5f008973 - std[3da461b304582a2c]::sys_common::backtrace::__rust_begin_short_backtrace::<fn(), ()>
  25:     0x564f5f008969 - <std[3da461b304582a2c]::rt::lang_start<()>::{closure#0} as core[d16e85342ea223d9]::ops::function::FnOnce<()>>::call_once::{shim:vtable#0}
  26:     0x7fcec774795c - core::ops::function::impls::<impl core::ops::function::FnOnce<A> for &F>::call_once::h699977d052768608
                               at /rustc/2c8cc343237b8f7d5a3c3703e3a87f2eb2c54a74/library/core/src/ops/function.rs:287:13
  27:     0x7fcec774795c - std::panicking::try::do_call::h4e121e623c70f903
                               at /rustc/2c8cc343237b8f7d5a3c3703e3a87f2eb2c54a74/library/std/src/panicking.rs:483:40
  28:     0x7fcec774795c - std::panicking::try::hf9d919e062bc178a
                               at /rustc/2c8cc343237b8f7d5a3c3703e3a87f2eb2c54a74/library/std/src/panicking.rs:447:19
  29:     0x7fcec774795c - std::panic::catch_unwind::h7a7b12272684cb97
                               at /rustc/2c8cc343237b8f7d5a3c3703e3a87f2eb2c54a74/library/std/src/panic.rs:140:14
  30:     0x7fcec774795c - std::rt::lang_start_internal::{{closure}}::hd96b0eb4844b8762
                               at /rustc/2c8cc343237b8f7d5a3c3703e3a87f2eb2c54a74/library/std/src/rt.rs:148:48
  31:     0x7fcec774795c - std::panicking::try::do_call::h1af1f88f4f92a22c
                               at /rustc/2c8cc343237b8f7d5a3c3703e3a87f2eb2c54a74/library/std/src/panicking.rs:483:40
  32:     0x7fcec774795c - std::panicking::try::hf20d7abea7f0f097
                               at /rustc/2c8cc343237b8f7d5a3c3703e3a87f2eb2c54a74/library/std/src/panicking.rs:447:19
  33:     0x7fcec774795c - std::panic::catch_unwind::hb0e084c3a9c042e4
                               at /rustc/2c8cc343237b8f7d5a3c3703e3a87f2eb2c54a74/library/std/src/panic.rs:140:14
  34:     0x7fcec774795c - std::rt::lang_start_internal::hca9d5c7277f5b67c
                               at /rustc/2c8cc343237b8f7d5a3c3703e3a87f2eb2c54a74/library/std/src/rt.rs:148:20
  35:     0x564f5f008ab7 - main
  36:     0x7fcec74a1790 - <unknown>
  37:     0x7fcec74a184a - __libc_start_main
  38:     0x564f5f00899e - <unknown>
  39:                0x0 - <unknown>

error: internal compiler error: unexpected panic

note: the compiler unexpectedly panicked. this is a bug.

note: we would appreciate a bug report: https://github.com/rust-lang/rust/issues/new?labels=C-bug%2C+I-ICE%2C+T-compiler&template=ice.md

note: rustc 1.68.0 (2c8cc3432 2023-03-06) running on x86_64-unknown-linux-gnu

query stack during panic:
end of query stack

I also checked if I can trigger a similar problem by passing empty argument list to execve, but at least under Linux, it seems to always insert an empty first argument if there are none.

rustbot · 2023-03-13T17:46:15Z

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @WaffleLapkin (or someone else) soon.

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

@rustbot author: the review is finished, PR author should check the comments and take action accordingly
@rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

compiler/rustc_driver_impl/src/lib.rs

WaffleLapkin · 2023-03-13T18:01:00Z

The fix looks fine to me, may I ask however how did you find this/what is the use-case that is broken by this ICE?

compiler/rustc_driver_impl/src/lib.rs

dekrain · 2023-03-13T18:05:22Z

The fix looks fine to me, may I ask however how did you find this/what is the use-case that is broken by this ICE?

I found it by following the flow from the entry point, and it popped up to me as a potential issue. And it turned out right.
I don't think it's gonna have a big impact, but it's better not to have such small issues lying around.

klensy · 2023-03-13T18:13:53Z

Backtrace stuff from slightly outdated version, there no compiler/rustc_driver/src/lib.rs:972:17, it's mostly empty.

Some error handling there, shouldn't it work?

rust/compiler/rustc_driver_impl/src/args.rs

Lines 6 to 33 in cf8d98b

    
           fn arg_expand(arg: String) -> Result<Vec<String>, Error> { 
        
               if let Some(path) = arg.strip_prefix('@') { 
        
                   let file = match fs::read_to_string(path) { 
        
                       Ok(file) => file, 
        
                       Err(ref err) if err.kind() == io::ErrorKind::InvalidData => { 
        
                           return Err(Error::Utf8Error(Some(path.to_string()))); 
        
                       } 
        
                       Err(err) => return Err(Error::IOError(path.to_string(), err)), 
        
                   }; 
        
                   Ok(file.lines().map(ToString::to_string).collect()) 
        
               } else { 
        
                   Ok(vec![arg]) 
        
               } 
        
           } 
        
           pub fn arg_expand_all(at_args: &[String]) -> Vec<String> { 
        
               let mut args = Vec::new(); 
        
               for arg in at_args { 
        
                   match arg_expand(arg.clone()) { 
        
                       Ok(arg) => args.extend(arg), 
        
                       Err(err) => rustc_session::early_error( 
        
                           rustc_session::config::ErrorOutputType::default(), 
        
                           &format!("Failed to load argument file: {err}"), 
        
                       ), 
        
                   } 
        
               } 
        
               args 
        
           }

dekrain · 2023-03-13T18:20:57Z

@klensy The bug is in run_compiler. It was caused by expanding argv[0] before removing it in handle_option, which can result in trying to take out first item from an empty slice, leading to an out-of-bounds crash.

compiler/rustc_driver_impl/src/lib.rs

klensy · 2023-03-13T18:51:09Z

@klensy The bug is in run_compiler. It was caused by expanding argv[0] before removing it in handle_option, which can result in trying to take out first item from an empty slice, leading to an out-of-bounds crash.

Sorry, i still don't understand why this shouldn't be in arg_expand/arg_expand_all, which exist exactly for checking errors in args.

WaffleLapkin · 2023-03-13T18:54:46Z

@klensy arg_expand expands @... to the contents of ... file, it may be useful for arguments, but allowing this as argv[0] doesn't make much sense, argv[0] is normally the name of the binary, there is nothing to expand there.

klensy · 2023-03-13T19:11:59Z

 pub fn arg_expand_all(at_args: &[String]) -> Vec<String> { 
     let mut args = Vec::new(); 
     // don't care about argv[0]
     for arg in at_args.iter().skip(1) {
     ...
    }
}

dekrain · 2023-03-13T19:17:58Z

Handling of argv[0] isn't a reponsibility of arg_expand_all, and it could conflict with more potential use cases, for example...
If it's decided at some point to also recursively expand @arguments in the argument file, it would call arg_expand_all on a list without argv[0].

klensy · 2023-03-13T19:35:51Z

Handling of argv[0] isn't a reponsibility of arg_expand_all, and it could conflict with more potential use cases, for example...

If it's decided at some point to also recursively expand @arguments in the argument file, it would call arg_expand_all on a list without argv[0].

Why not?
In that potential far future it would be enough to look at 3rd line of (arg_expand_all) function (or comment above)

rust/src/librustdoc/lib.rs

Lines 705 to 708 in cf8d98b

    
           fn main_args(at_args: &[String]) -> MainResult { 
        
               let args = rustc_driver::args::arg_expand_all(at_args); 
        
               let mut options = getopts::Options::new();

dekrain · 2023-03-13T23:15:39Z

Now that I see this function is called in more places (only librustdoc right now), it might be reasonable to add a variant in rustc_driver::args to specifically handle this. An alternative is to add this snippet at all use sites, potentially even for binaries not using rustc_driver::args. Both places which use it right now do remove the first argument manually, so it might be best to somehow unify them.

In any case, I'll wait to hear the team's opinion first.

apiraino · 2023-04-25T14:41:12Z

hello visiting this PR, checking progress. by reading this comment IIUC this is waiting on an input from the reviewer cc @WaffleLapkin ? thanks

WaffleLapkin · 2023-04-25T15:12:31Z

@apiraino I'm not sure how to answer the question, so maybe a reroll is appropriate here

apiraino · 2023-04-25T15:20:35Z

r? compiler

Any input about #109084 (comment) ?

petrochenkov · 2023-04-26T08:48:36Z

Both adding the snippet before every use of arg_expand_all and moving it inside arg_expand_all seems fine to me.

dekrain · 2023-04-26T09:02:07Z

So I can do either? (first option is already in place)
Or do you mean to move it inside arg_expand_all and add a comment on every call site?

petrochenkov · 2023-04-26T10:01:01Z

So I can do either?

Either

(first option is already in place)

No, the snippet is not added to the use of arg_expand_all in rustdoc.

jyn514 · 2023-05-13T21:17:53Z

Hi @dekrain, it's been a while - are you still planning to work on this? Do you know how to make progress? Feel free to ask for help here or on Zulip if you get stuck.

dekrain · 2023-05-14T11:40:01Z

Sorry, but the same day I posted my last comment, I lost Internet access for a quite long time, and since I focused on other things. I can take care of it now.

dekrain · 2023-05-16T17:12:27Z

@rustbot review

petrochenkov · 2023-05-17T11:17:25Z

r=me after squashing commits into one.
@rustbot author

dekrain · 2023-05-27T16:08:31Z

@rustbot review

jyn514 · 2023-05-27T16:49:57Z

@bors r=petrochenkov

bors · 2023-05-27T16:49:59Z

📌 Commit 6240d45 has been approved by petrochenkov

It is now in the queue for this repository.

…iaskrgr Rollup of 6 pull requests Successful merges: - rust-lang#108630 (Fix docs for `alloc::realloc`) - rust-lang#109084 (rustc driver: Remove argument 0 before at-expansion to prevent ICE) - rust-lang#111181 (fix(parse): return unpected when current token is EOF) - rust-lang#111656 (Use an unbounded lifetime in `String::leak`.) - rust-lang#111946 (rustdoc: Add `ItemTemplate` trait and related functions to avoid repetitively wrapping existing functions) - rust-lang#112018 (Clean up usage of `cx.tcx` when `tcx` is already set into a variable) r? `@ghost` `@rustbot` modify labels: rollup

rustbot assigned WaffleLapkin Mar 13, 2023

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 Mar 13, 2023

Noratrieb reviewed Mar 13, 2023

View reviewed changes

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

WaffleLapkin reviewed Mar 13, 2023

View reviewed changes

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

WaffleLapkin reviewed Mar 13, 2023

View reviewed changes

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

rustbot assigned petrochenkov and unassigned WaffleLapkin Apr 25, 2023

petrochenkov 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 Apr 26, 2023

This comment has been minimized.

Sign in to view

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 May 16, 2023

klensy mentioned this pull request May 16, 2023

Refactor pre-getopts command line argument handling #111658

Closed

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 May 17, 2023

Fix ICE caused by at-expanding argument 0 instead of removing it early

6240d45

dekrain force-pushed the fix-panic-arg0-expansion branch from 3016cbe to 6240d45 Compare May 27, 2023 16:04

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 May 27, 2023

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 May 27, 2023

matthiaskrgr mentioned this pull request May 27, 2023

Rollup of 6 pull requests #112025

Merged

bors merged commit 0b300a7 into rust-lang:master May 27, 2023

rustbot added this to the 1.71.0 milestone May 27, 2023

dekrain deleted the fix-panic-arg0-expansion branch May 28, 2023 01:40

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

rustc driver: Remove argument 0 before at-expansion to prevent ICE #109084

rustc driver: Remove argument 0 before at-expansion to prevent ICE #109084

dekrain commented Mar 13, 2023

rustbot commented Mar 13, 2023

WaffleLapkin commented Mar 13, 2023

dekrain commented Mar 13, 2023

klensy commented Mar 13, 2023 •

edited

Loading

dekrain commented Mar 13, 2023 •

edited

Loading

klensy commented Mar 13, 2023

WaffleLapkin commented Mar 13, 2023

klensy commented Mar 13, 2023

dekrain commented Mar 13, 2023

klensy commented Mar 13, 2023 •

edited

Loading

dekrain commented Mar 13, 2023

apiraino commented Apr 25, 2023

WaffleLapkin commented Apr 25, 2023

apiraino commented Apr 25, 2023

petrochenkov commented Apr 26, 2023

dekrain commented Apr 26, 2023

petrochenkov commented Apr 26, 2023

jyn514 commented May 13, 2023

dekrain commented May 14, 2023

This comment has been minimized.

dekrain commented May 16, 2023

petrochenkov commented May 17, 2023

dekrain commented May 27, 2023

jyn514 commented May 27, 2023

bors commented May 27, 2023

rustc driver: Remove argument 0 before at-expansion to prevent ICE #109084

rustc driver: Remove argument 0 before at-expansion to prevent ICE #109084

Conversation

dekrain commented Mar 13, 2023

rustbot commented Mar 13, 2023

WaffleLapkin commented Mar 13, 2023

dekrain commented Mar 13, 2023

klensy commented Mar 13, 2023 • edited Loading

dekrain commented Mar 13, 2023 • edited Loading

klensy commented Mar 13, 2023

WaffleLapkin commented Mar 13, 2023

klensy commented Mar 13, 2023

dekrain commented Mar 13, 2023

klensy commented Mar 13, 2023 • edited Loading

dekrain commented Mar 13, 2023

apiraino commented Apr 25, 2023

WaffleLapkin commented Apr 25, 2023

apiraino commented Apr 25, 2023

petrochenkov commented Apr 26, 2023

dekrain commented Apr 26, 2023

petrochenkov commented Apr 26, 2023

jyn514 commented May 13, 2023

dekrain commented May 14, 2023

This comment has been minimized.

dekrain commented May 16, 2023

petrochenkov commented May 17, 2023

dekrain commented May 27, 2023

jyn514 commented May 27, 2023

bors commented May 27, 2023

klensy commented Mar 13, 2023 •

edited

Loading

dekrain commented Mar 13, 2023 •

edited

Loading

klensy commented Mar 13, 2023 •

edited

Loading