Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Default associated types are not typechecked, and can cause UB #62211

Closed
DutchGhost opened this issue Jun 28, 2019 · 5 comments · Fixed by #61812
Closed

Default associated types are not typechecked, and can cause UB #62211

DutchGhost opened this issue Jun 28, 2019 · 5 comments · Fixed by #61812
Labels
A-associated-items Area: Associated items (types, constants & functions) C-bug Category: This is a bug. F-associated_type_defaults `#![feature(associated_type_defaults)]` I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness requires-nightly This issue requires a nightly compiler in some way. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-lang Relevant to the language team, which will review and decide on the PR/issue.

Comments

@DutchGhost
Copy link
Contributor

DutchGhost commented Jun 28, 2019

Due to associated defaults not being type checked whether they indeed implement all traits specified by the associated type, you can create a program in safe Rust that will segfault:

#![feature(associated_type_defaults)]

use std::{
    fmt::Display,
    ops::{AddAssign, Deref}
};


trait UncheckedCopy: Sized {
    // This Output is said to be Copy. Yet we default to Self
    // and it's accepted, not knowing if Self ineed is Copy
    type Output: Copy
        + Deref<Target = str>
        + AddAssign<&'static str>
        + From<Self>
        + Display = Self;

    // We said the Output type was Copy, so we can Copy it freely!
    fn unchecked_copy(other: &Self::Output) -> Self::Output {
        (*other)
    }

    fn make_origin(s: Self) -> Self::Output {
        s.into()
    }
}

impl<T> UncheckedCopy for T {}

fn bug<T: UncheckedCopy>(origin: T) {
    let mut origin = T::make_origin(origin);
    let mut copy = T::unchecked_copy(&origin);

    // assert we indeed have 2 strings pointing to the same buffer.
    assert_eq!(origin.as_ptr(), copy.as_ptr());

    // Drop the origin. Any use of `copy` is UB.
    drop(origin);

    copy += "This is invalid!";
    println!("{}", copy);
}

fn main() {
    bug(String::from("hello!"));
}

Playground link: https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018&gist=395dbcdd4da0d0b66de9fcda6e1deb13

@Centril Centril added I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-lang Relevant to the language team, which will review and decide on the PR/issue. C-bug Category: This is a bug. I-nominated labels Jun 28, 2019
@DutchGhost
Copy link
Contributor Author

DutchGhost commented Jun 28, 2019

Also, if we call bug() with any other type than String, we get an ICE:

(the type used was () here)

Backtrace:
error: internal compiler error: src/librustc/traits/codegen/mod.rs:55: Encountered error `Unimplemented` selecting `Binder(<() as std::ops::Deref>)` during codegen

thread 'rustc' panicked at 'Box<Any>', src/librustc_errors/lib.rs:643:9
stack backtrace:
   0: backtrace::backtrace::libunwind::trace
             at /cargo/registry/src/github.com-1ecc6299db9ec823/backtrace-0.3.29/src/backtrace/libunwind.rs:88
   1: backtrace::backtrace::trace_unsynchronized
             at /cargo/registry/src/github.com-1ecc6299db9ec823/backtrace-0.3.29/src/backtrace/mod.rs:66
   2: std::sys_common::backtrace::_print
             at src/libstd/sys_common/backtrace.rs:47
   3: std::sys_common::backtrace::print
             at src/libstd/sys_common/backtrace.rs:36
   4: std::panicking::default_hook::{{closure}}
             at src/libstd/panicking.rs:198
   5: std::panicking::default_hook
             at src/libstd/panicking.rs:212
   6: rustc::util::common::panic_hook
   7: std::panicking::rust_panic_with_hook
             at src/libstd/panicking.rs:479
   8: std::panicking::begin_panic
   9: rustc_errors::Handler::bug
  10: rustc::util::bug::opt_span_bug_fmt::{{closure}}
  11: rustc::ty::context::tls::with_opt::{{closure}}
  12: rustc::ty::context::tls::with_context_opt
  13: rustc::ty::context::tls::with_opt
  14: rustc::util::bug::opt_span_bug_fmt
  15: rustc::util::bug::bug_fmt
  16: rustc::ty::context::GlobalCtxt::enter_local
  17: rustc::traits::codegen::codegen_fulfill_obligation
  18: rustc::ty::query::__query_compute::codegen_fulfill_obligation
  19: rustc::ty::query::<impl rustc::ty::query::config::QueryAccessors for rustc::ty::query::queries::codegen_fulfill_obligation>::compute
  20: rustc::dep_graph::graph::DepGraph::with_task_impl
  21: rustc::ty::query::plumbing::<impl rustc::ty::context::TyCtxt>::get_query
  22: rustc::ty::instance::Instance::resolve
  23: <rustc_mir::monomorphize::collector::MirNeighborCollector as rustc::mir::visit::Visitor>::visit_terminator_kind
  24: rustc_mir::monomorphize::collector::collect_items_rec
  25: rustc_mir::monomorphize::collector::collect_items_rec
  26: rustc_mir::monomorphize::collector::collect_crate_mono_items::{{closure}}
  27: rustc::util::common::time
  28: rustc_mir::monomorphize::collector::collect_crate_mono_items
  29: rustc::util::common::time
  30: rustc_mir::monomorphize::partitioning::collect_and_partition_mono_items
  31: rustc::ty::query::__query_compute::collect_and_partition_mono_items
  32: rustc::ty::query::<impl rustc::ty::query::config::QueryAccessors for rustc::ty::query::queries::collect_and_partition_mono_items>::compute
  33: rustc::dep_graph::graph::DepGraph::with_task_impl
  34: rustc::ty::query::plumbing::<impl rustc::ty::context::TyCtxt>::get_query
  35: rustc_codegen_ssa::base::codegen_crate
  36: <rustc_codegen_llvm::LlvmCodegenBackend as rustc_codegen_utils::codegen_backend::CodegenBackend>::codegen_crate
  37: rustc::util::common::time
  38: rustc_interface::passes::start_codegen
  39: rustc::ty::context::tls::enter_global
  40: rustc_interface::passes::BoxedGlobalCtxt::access::{{closure}}
  41: rustc_interface::passes::create_global_ctxt::{{closure}}
  42: rustc_interface::passes::BoxedGlobalCtxt::enter
  43: rustc_interface::queries::Query<T>::compute
  44: rustc_interface::queries::<impl rustc_interface::interface::Compiler>::ongoing_codegen
  45: rustc_interface::interface::run_compiler_in_existing_thread_pool
  46: std::thread::local::LocalKey<T>::with
  47: scoped_tls::ScopedKey<T>::set
  48: syntax::with_globals
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
query stack during panic:
#0 [codegen_fulfill_obligation] checking if `std::ops::Deref` fulfills its obligations
#1 [collect_and_partition_mono_items] collect_and_partition_mono_items
end of query stack
error: aborting due to previous error

@Centril
Copy link
Contributor

Centril commented Jun 28, 2019

cc @jonas-schievink @nikomatsakis

This should be fixed by #61812.

@jonas-schievink jonas-schievink self-assigned this Jun 28, 2019
@jonas-schievink
Copy link
Contributor

Assigning myself to add tests to #61812

@jonas-schievink jonas-schievink added the A-associated-items Area: Associated items (types, constants & functions) label Jun 28, 2019
@jonas-schievink
Copy link
Contributor

Can confirm that #61812 correctly rejects the default type.

@jonas-schievink jonas-schievink removed their assignment Jun 28, 2019
@arielb1
Copy link
Contributor

arielb1 commented Jul 6, 2019

This has the same root cause as #33017, so this is still not quite solved at the root.

@Centril Centril added the requires-nightly This issue requires a nightly compiler in some way. label Jul 28, 2019
@jonas-schievink jonas-schievink added the F-associated_type_defaults `#![feature(associated_type_defaults)]` label Aug 29, 2019
@bors bors closed this as completed in 3a0d106 Feb 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-associated-items Area: Associated items (types, constants & functions) C-bug Category: This is a bug. F-associated_type_defaults `#![feature(associated_type_defaults)]` I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness requires-nightly This issue requires a nightly compiler in some way. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants