ICE with malformed `rustc_on_unimplemented` on `impl`

[jruderman]

Found with a modified fuzz-rustc

Code

#![feature(rustc_attrs)]

trait Foo {}

#[rustc_on_unimplemented]
impl Foo for u32 {}

fn main() {}

ICE output

error: internal compiler error: no errors encountered even though `delay_span_bug` issued

error: internal compiler error: of_item: neither meta_item_list nor value_str
  |
  = note: delayed at compiler/rustc_trait_selection/src/traits/on_unimplemented.rs:169:26

Backtrace

thread 'rustc' panicked at 'Box<dyn Any>', compiler/rustc_errors/src/lib.rs:1585:13
stack backtrace:
   0:        0x10b95d7a2 - <std::sys_common::backtrace::_print::DisplayBacktrace as core::fmt::Display>::fmt::hde9930b8710af522
   1:        0x10b9b55ea - core::fmt::write::hd25ec0bfc1849a2f
   2:        0x10b94f87c - std::io::Write::write_fmt::hca36d7396c5dc18b
   3:        0x10b95d56a - std::sys_common::backtrace::print::hc155979e9e33e658
   4:        0x10b960946 - std::panicking::default_hook::{{closure}}::h03db778ae686f347
   5:        0x10b960697 - std::panicking::default_hook::h0d9e8bcb55be071f
   6:        0x1199837dd - rustc_driver[77a613ab63d15aac]::DEFAULT_HOOK::{closure#0}::{closure#0}
   7:        0x10b961145 - std::panicking::rust_panic_with_hook::h758520931cfdfde5
   8:        0x11e400f27 - std[11391ffec397fd56]::panicking::begin_panic::<rustc_errors[ec87cacb996b1da1]::ExplicitBug>::{closure#0}
   9:        0x11e3fef59 - std[11391ffec397fd56]::sys_common::backtrace::__rust_end_short_backtrace::<std[11391ffec397fd56]::panicking::begin_panic<rustc_errors[ec87cacb996b1da1]::ExplicitBug>::{closure#0}, !>
  10:        0x11e7c7239 - std[11391ffec397fd56]::panicking::begin_panic::<rustc_errors[ec87cacb996b1da1]::ExplicitBug>
  11:        0x11e3f0059 - std[11391ffec397fd56]::panic::panic_any::<rustc_errors[ec87cacb996b1da1]::ExplicitBug>
  12:        0x11e3f83f5 - <rustc_errors[ec87cacb996b1da1]::HandlerInner>::flush_delayed::<alloc[a05f3e7fdaf4d80f]::vec::Vec<rustc_errors[ec87cacb996b1da1]::diagnostic::Diagnostic>, &str>
  13:        0x11e3f71b6 - <rustc_errors[ec87cacb996b1da1]::Handler>::flush_delayed
  14:        0x119ac9a94 - <rustc_interface[a0d981abc9377f30]::passes::QueryContext>::enter::<<rustc_interface[a0d981abc9377f30]::queries::Queries>::ongoing_codegen::{closure#0}::{closure#0}, core[cfd2f0869b685cd6]::result::Result<alloc[a05f3e7fdaf4d80f]::boxed::Box<dyn core[cfd2f0869b685cd6]::any::Any>, rustc_errors[ec87cacb996b1da1]::ErrorGuaranteed>>
  15:        0x119aaa3ac - <rustc_interface[a0d981abc9377f30]::queries::Queries>::ongoing_codegen
  16:        0x1199e3fa4 - rustc_span[48482e0f8f724117]::with_source_map::<core[cfd2f0869b685cd6]::result::Result<(), rustc_errors[ec87cacb996b1da1]::ErrorGuaranteed>, rustc_interface[a0d981abc9377f30]::interface::run_compiler<core[cfd2f0869b685cd6]::result::Result<(), rustc_errors[ec87cacb996b1da1]::ErrorGuaranteed>, rustc_driver[77a613ab63d15aac]::run_compiler::{closure#1}>::{closure#0}::{closure#1}>
  17:        0x1199d3a7c - <scoped_tls[46b3c18c0a12f0bc]::ScopedKey<rustc_span[48482e0f8f724117]::SessionGlobals>>::set::<rustc_interface[a0d981abc9377f30]::interface::run_compiler<core[cfd2f0869b685cd6]::result::Result<(), rustc_errors[ec87cacb996b1da1]::ErrorGuaranteed>, rustc_driver[77a613ab63d15aac]::run_compiler::{closure#1}>::{closure#0}, core[cfd2f0869b685cd6]::result::Result<(), rustc_errors[ec87cacb996b1da1]::ErrorGuaranteed>>
  18:        0x1199a33ca - std[11391ffec397fd56]::sys_common::backtrace::__rust_begin_short_backtrace::<rustc_interface[a0d981abc9377f30]::util::run_in_thread_pool_with_globals<rustc_interface[a0d981abc9377f30]::interface::run_compiler<core[cfd2f0869b685cd6]::result::Result<(), rustc_errors[ec87cacb996b1da1]::ErrorGuaranteed>, rustc_driver[77a613ab63d15aac]::run_compiler::{closure#1}>::{closure#0}, core[cfd2f0869b685cd6]::result::Result<(), rustc_errors[ec87cacb996b1da1]::ErrorGuaranteed>>::{closure#0}::{closure#0}, core[cfd2f0869b685cd6]::result::Result<(), rustc_errors[ec87cacb996b1da1]::ErrorGuaranteed>>
  19:        0x1199894cb - <<std[11391ffec397fd56]::thread::Builder>::spawn_unchecked_<rustc_interface[a0d981abc9377f30]::util::run_in_thread_pool_with_globals<rustc_interface[a0d981abc9377f30]::interface::run_compiler<core[cfd2f0869b685cd6]::result::Result<(), rustc_errors[ec87cacb996b1da1]::ErrorGuaranteed>, rustc_driver[77a613ab63d15aac]::run_compiler::{closure#1}>::{closure#0}, core[cfd2f0869b685cd6]::result::Result<(), rustc_errors[ec87cacb996b1da1]::ErrorGuaranteed>>::{closure#0}::{closure#0}, core[cfd2f0869b685cd6]::result::Result<(), rustc_errors[ec87cacb996b1da1]::ErrorGuaranteed>>::{closure#1} as core[cfd2f0869b685cd6]::ops::function::FnOnce<()>>::call_once::{shim:vtable#0}
  20:        0x10b96a1f7 - std::sys::unix::thread::Thread::new::thread_start::ha121f0b94f5d418a
  21:     0x7ff814d514e1 - __pthread_start

Expected output

I expected an error like "malformed rustc_on_unimplemented attribute input". (I do see this error message if I move the attribute to the trait item.)

Regression

Stopped reporting the expected error in nightly-2021-08-28:

• Commit range ad02dc4..ac50a53

Became an ICE in nightly-2022-03-19:

• The call to delay_span_bug was added in bb8d430 (Make ErrorReported impossible to construct outside rustc_errors #93222)

Version

rustc 1.67.0-nightly (7eef946fc 2022-11-06)
binary: rustc
commit-hash: 7eef946fc0e0eff40e588eab77b09b287accbec3
commit-date: 2022-11-06
host: x86_64-apple-darwin
release: 1.67.0-nightly
LLVM version: 15.0.4

[fmease]

I did some digging. AstValidator::visit_attribute does not seem to visit any attributes ascribed to impl items and as a result of that, the meta-items of built-in attributes like rustc_on_unimplemented, rustc_main, rustc_paren_sugar are not validated.

[fmease]

… which stems from the fact that in AstValidator::visit_item we sometimes (e.g. in match arm ItemKind::Impl / for_trait: Some(_)) return early, thereby skipping visit::walk_item(self, item) located at the end of the method which would've called walk_list!(visitor, visit_attribute, &item.attrs); and would've prevented this issue.

Some other match arms (e.g. ItemKind::Trait) that return early therefore rightfully call walk_list!(self, visit_attribute, &item.attrs); beforehand. This should be done for every single match arm that returns early. I hope that this change will be backward compatible 😟

@rustbot claim