Trait bounds not checked on specializable associated types #33017
Labels
A-associated-items
Area: Associated items (types, constants & functions)
A-specialization
Area: Trait impl specialization
C-bug
Category: This is a bug.
E-needs-test
Call for participation: An issue has been fixed and does not reproduce, but no test has been added.
F-specialization
`#![feature(specialization)]`
I-unsound
Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness
P-medium
Medium priority
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.
Comments
apasel422
added
I-ICE
arielb1
added
I-unsound
apasel422
changed the title
|
Triage: still an issue. |
|
I just hit this, though the error is a bit different now: "resolving bounds after type-checking" rather than "fulfilling during trans". |
This was referenced Nov 28, 2016
|
ping @arielb1 how is this unsound? |
bstrie
added
I-nominated
T-compiler
|
Given that @arielb1 has seen fit to tag this as a soundness bug I'm nominating it so that it can receive a priority assessment, though I'll second ubsan in asking for a demonstration of the unsoundness here. |
|
In general, having trait bounds that are not checked can lead to unsoundness, since some part of the code will assume that they hold when in fact they do not (consider, e.g., what might happen if the bound were |
|
I'm going to mark this as P-medium and tag it from the specialization issue (it is specific to specialization, right?) |
|
triage: P-medium |
bstrie
added
the
B-unstable
|
The code in the OP compiles for me with the latest nightly Rust (1.29.0-nightly (97085f9 2018-08-01)). However, the following still fails (and I assume it's the same issue; if not I can file a new issue): #![feature(specialization)]
trait PreProcess<T> {
type Type: PostProcess<T> + std::default::Default;
fn pre_process() -> Self::Type;
}
trait PostProcess<T> {
fn post_process(self);
}
impl<T> PreProcess<T> for T {
default type Type = std::marker::PhantomData<T>;
default fn pre_process() -> Self::Type {
return <Self::Type as std::default::Default>::default();
}
}
pub struct Foo;
pub fn do_foo() {
PostProcess::post_process(<Foo as PreProcess<Foo>>::pre_process());
}Backtrace: |
This was referenced Jul 6, 2019
|
So I think this is "fixed-by-chalk", but this is fixed-by-chalk in a way that breaks specialization (cc @rust-lang/wg-traits). Let's have a more easy-to-understand version of the problem: #![feature(specialization)]
trait Foo: Sized {
type Assoc: Copy + From<Self> + Into<Self>;
}
impl<T> Foo for T {
default type Assoc = Self;
}
fn xyz<T: Foo>(t: T) -> T::Bar {
t.into()
}
fn main() {
xyz(5i32);
}The problem is the following:
Was Chalk changed to do something else by now? Does this have a different interaction with specialization? |
|
A PoC to show the unsoundness of this. It's a use-after free by 'copying' a String: #![feature(specialization)]
trait UncheckedCopy: Sized {
type Output: From<Self> + Copy + Into<Self>;
}
impl <T> UncheckedCopy for T {
default type Output = Self;
}
fn unchecked_copy<T: UncheckedCopy>(other: &T::Output) -> T {
(*other).into()
}
fn bug(origin: String) {
// Turn the String into it's Output type...
// Which we can just do by `.into()`, the assoc type states `From<Self>`.
let origin_output = origin.into();
// Make a copy of String::Output, which is a String...
let mut copy: String = unchecked_copy::<String>(&origin_output);
// Turn the Output type into a String again,
// Which we can just do by `.into()`, the assoc type states `Into<Self>`.
let mut origin: String = origin_output.into();
// assert both Strings use the same buffer.
assert_eq!(copy.as_ptr(), origin.as_ptr());
// Any use of the copy we made becomes invalid,
drop(origin);
// OH NO! UB UB UB UB!
copy.push_str(" world!");
println!("{}", copy);
}
fn main() {
bug(String::from("hello"));
}Link to the playground: https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018 |
Centril
added
requires-nightly
jonas-schievink
removed
the
B-unstable
jonas-schievink
added
the
A-associated-items
Elinvynia
added
the
I-prioritize
LeSeulArtichaut
removed
the
I-prioritize
|
This is fixed now: https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018 and needs a test |
Dylan-DPC-zz
added
the
E-needs-test
marmeladema
added a commit
to marmeladema/rust
that referenced
this issue
Apr 23, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
foo.rs:The text was updated successfully, but these errors were encountered: