Upgrade to nightly #39

joshuawarner32 · Sep 10, 2016

NOTE: Do not merge yet! Shenanigans abound!

... but on the surface it seems to work.

joshuawarner32 · Sep 10, 2016

joshuawarner32 reviewed Sep 10, 2016

View changes

joshuawarner32 · Sep 10, 2016

src/trans.rs

@@ -534,6 +532,16 @@ impl<'v, 'tcx: 'v> BinaryenFnCtxt<'v, 'tcx> {
                TerminatorKind::Return => {
                    /* handled during bb creation */
                }
+                TerminatorKind::Assert { ref target, .. } => {
+                    // TODO: An assert is not a GOTO!!!


I wasn't sure what to do with this.

It's a conditional call to one of two lang items.

joshuawarner32 · Sep 10, 2016

joshuawarner32 reviewed Sep 10, 2016

View changes

joshuawarner32 · Sep 10, 2016

src/trans.rs

-                    }
-                }
-            }
+            // if self.sig.output == ty::FnOutput::FnDiverging {


It wasn't clear what to do in this case. FnOutput was removed in a recent refactor (see: rust-lang/rust@ed02344).

self.sig.output.is_never(). But also, this whole block is a very lengthy way to do Some(self.did) == tcx.lang_items.panic_fn() (which doesn't require the check for the return type).

joshuawarner32 · Sep 10, 2016

joshuawarner32 reviewed Sep 10, 2016

View changes

joshuawarner32 · Sep 10, 2016

src/trans.rs

@@ -719,6 +727,48 @@ impl<'v, 'tcx: 'v> BinaryenFnCtxt<'v, 'tcx> {
                }
            }

+            Rvalue::CheckedBinaryOp(ref op, ref left, ref right) => {
+                // TODO: This shouldn't just be a copy BinaryOp above!
+                // We should do some actual _checking_!


This is obviously not correct. I may take a second look this weekend.

This needs either support for ops that also return the overflow bit, or emulation thereof.

On second thought, this would be much easier to do with a call to an emulation function, and letting binaryen (or downstream jit compilers) inline that as appropriate. Is there precedent for doing that sort of thing?

Having that emulation written in Rust instead of generating it? Depending on what the internal API is of mir2wasm, it could be just as easy to emit the right code here (and you wouldn't have compiler-implementation dependencies).

I was actually not thinking of writing it in rust, though that's also a good idea. The problem I'm trying to avoid is that checked operations tend to do a number on the local control flow - and I haven't fully wrapped my head around the binaryen APIs needed to implement that. Just emitting a call makes this really easy. But I could easily be dramatically overestimating the complexity of just doing it the obvious (inline) way.

joshuawarner32 · Sep 10, 2016

FYI, I was just playing around with this (awesome stuff!). I thought this might be helpful. If not, don't hesitate to close this. No worries.

eddyb · Sep 10, 2016

eddyb reviewed Sep 10, 2016

View changes

eddyb · Sep 10, 2016

src/trans.rs

@@ -282,6 +277,9 @@ impl<'v, 'tcx: 'v> BinaryenFnCtxt<'v, 'tcx> {
                    StatementKind::Assign(ref lvalue, ref rvalue) => {
                        self.trans_assignment(lvalue, rvalue, &mut binaryen_stmts);
                    }
+                    StatementKind::StorageLive(ref var) => {}
+                    StatementKind::StorageDead(ref var) => {}


You can use _ instead of ref var to eliminate warnings about unused variables.

eddyb · Sep 10, 2016

eddyb reviewed Sep 10, 2016

View changes

eddyb · Sep 10, 2016

src/trans.rs

            Lvalue::Temp(i) => {
+                let i: u32 = unsafe { mem::transmute(i) };


Never, ever use transmute except as a last resort. These newtypes have an API for getting their index integer.
Besides, this computation has been replaced with local_index or something like that, also used by rustc_trans.

Fair. I had a hell of a time finding the definition of the Temp struct (in this case). I think I was just taking out my frustration on the code. :)

I just pushed a fix to use i.index() instead. Is that what you mean by local_index - or is there an even better way to simplify this?

eddyb · Sep 10, 2016

eddyb reviewed Sep 10, 2016

View changes

eddyb · Sep 10, 2016

src/trans.rs

@@ -1034,7 +1091,7 @@ impl<'v, 'tcx: 'v> BinaryenFnCtxt<'v, 'tcx> {

    #[inline]
    fn type_size(&self, ty: Ty<'tcx>) -> usize {
-        let substs = self.tcx.mk_substs(Substs::empty());
+        let substs = Substs::empty(*self.tcx);


Odd, is self.tcx a reference to a reference?

Nope, but Substs::empty takes a struct, not a reference. FWIW, that seems like a common pattern with TyCtxt: lots of functions ownership of it, but the TyCtxt has #[derive(Clone, Copy)] on it. Just glancing at it, I hope the compiler is doing some magic to eliminate those copies, because TyCtxt is a pretty big struct.

TyCtxt is a reference (or two), hence, &TyCtxt is a reference to a reference.

Ah, you're right. Somehow my eyes just glazed over the &'a part of those fields. For posterity: https://github.com/rust-lang/rust/blob/f2b672d5567c3b20542e845baaf8a9c47d9df907/src/librustc/ty/context.rs#L285

eddyb · Sep 10, 2016

eddyb reviewed Sep 10, 2016

View changes

eddyb · Sep 10, 2016

src/trans.rs

-                    }
-                }
-            }
+            let mut is_fn_panic = Some(self.did) == self.tcx.lang_items.panic_fn();


This shouldn't be mut. In fact, it's only used once, in the condition below. If you switch the "then" and "else" bodies, you can use this comparison as the condition.

Similar story with is_entry_fn which could be Some(nid) == self.node_id below.

brson · Sep 13, 2016

Thanks @joshuawarner32 !

@eholk do you want to take a peek at this before I blindly merge?

eholk · Sep 13, 2016

I'd like to take a look. I should be able to get to it this afternoon. I
can go ahead and complete the merge when I'm done if that's alright with
you.

On Tue, Sep 13, 2016 at 2:59 PM Brian Anderson notifications@github.com
wrote:

Thanks @joshuawarner32 https://github.com/joshuawarner32 !

@eholk https://github.com/eholk do you want to take a peek at this
before I blindly merge?

—
You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub
#39 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAGdJoedqZgn29lDOlvab-FkPOs4fzURks5qpxzegaJpZM4J5mzG
.

joshuawarner32 · Sep 13, 2016

@brson Also be aware that several of the caveats mentioned above still apply; I haven't gotten a chance to look at the handling of Asserts and CheckedBinaryOperations. Also, the only testing I've done is to run cargo run -q -- rust-examples/nocore-hello-world.rs.

Also, I just saw rust-lang/rust#36339 (llvm-based wasm) - does that mean mir2wasm is not the "right" direction to go?

kripken · Sep 13, 2016

@joshuawarner32

does that mean mir2wasm is not the "right" direction to go?

Both approaches (mir2wasm and llvm-based) are worthwhile in my opinion. Each has different things that makes it useful:

mir2wasm should be able to beat all other options - including native llvm-based builds - on compile time, easily.
the llvm-based approach can more easily integrate with emscripten libraries like GL, SDL, etc.
mir2wasm is more under rust's control - it doesn't need to use existing emscripten "ABI" type things like how it manages the stack, etc. That means it's harder to integrate with existing libraries, but allows doing things in more rust-specific ways. In particular for simple "pure computation" uses of rust on the web, mir2wasm could emit more focused and compact code.

eholk · Sep 13, 2016

I'd like to see cargo test pass before we merge this. I just gave it a try and after updating compiletest_rs, I got the following error (among others):

test [run-compile-pass] tests/compile-pass/struct-derive.rs ... FAILED with exit code Some(101)
stdout: 

stderr: 
 error: internal compiler error: ../src/librustc/ty/subst.rs:457: Type parameter `Self/#0` (Self/0) out of range when substituting (root type=Some(&Self)) substs=[]

thread 'main' panicked at 'Box<Any>', ../src/librustc_errors/lib.rs:590
stack backtrace:
   1:     0x7f711db2b173 - std::sys::backtrace::tracing::imp::write::h4b09e6e8c01db097
   2:     0x7f711db3bd1d - std::panicking::default_hook::{{closure}}::h1d3243f546573ff4
   3:     0x7f711db3a3aa - std::panicking::default_hook::h96c288d728df3ebf
   4:     0x7f711db3a9a8 - std::panicking::rust_panic_with_hook::hb1322e5f2588b4db
   5:     0x7f7122110328 - std::panicking::begin_panic::hb7c71bbc491f561c
   6:     0x7f71223b5935 - rustc::session::opt_span_bug_fmt::{{closure}}::h6ad7caa20ac20c4b
   7:     0x7f712230ec85 - rustc::session::opt_span_bug_fmt::h38eb39636c07401c
   8:     0x7f712230eb14 - rustc::session::span_bug_fmt::heb3fdb6acac8d0c7
   9:     0x7f712236c2de - <rustc::ty::subst::SubstFolder<'a, 'gcx, 'tcx> as rustc::ty::fold::TypeFolder<'gcx, 'tcx>>::fold_ty::h17c67c094233ff36
  10:     0x7f712236bf5a - <rustc::ty::subst::SubstFolder<'a, 'gcx, 'tcx> as rustc::ty::fold::TypeFolder<'gcx, 'tcx>>::fold_ty::h17c67c094233ff36
  11:     0x7f7122ca29af - rustc::ty::structural_impls::<impl rustc::ty::fold::TypeFoldable<'tcx> for &'tcx rustc::ty::TyS<'tcx>>::fold_with::h36c63211b891560a
                        at /buildslave/rust-buildbot/slave/nightly-dist-rustc-linux/build/obj/../src/librustc/ty/structural_impls.rs:508
  12:     0x7f7122cb0c05 - <T as rustc::ty::subst::Subst<'tcx>>::subst_spanned::h1e2909ff760b6f53
                        at /buildslave/rust-buildbot/slave/nightly-dist-rustc-linux/build/obj/../src/librustc/ty/subst.rs:352
  13:     0x7f7122ca4c7d - rustc::ty::subst::Subst::subst::h273373dc5ac91f64
                        at /buildslave/rust-buildbot/slave/nightly-dist-rustc-linux/build/obj/../src/librustc/ty/subst.rs:331
  14:     0x7f7122cfd7cf - mir2wasm::monomorphize::apply_ty_substs::hef3b5edb9ee9e940
                        at /usr/local/google/home/eholk/wasm/mir2wasm/src/monomorphize.rs:20
  15:     0x7f7122cfa812 - mir2wasm::trans::BinaryenFnCtxt::type_layout_with_substs::hcddbf3b0d36a58a0
                        at /usr/local/google/home/eholk/wasm/mir2wasm/src/trans.rs:1080
  16:     0x7f7122cfa7b3 - mir2wasm::trans::BinaryenFnCtxt::type_layout::h599f742f9b96d87d
                        at /usr/local/google/home/eholk/wasm/mir2wasm/src/trans.rs:1074
  17:     0x7f7122cf2a01 - mir2wasm::trans::BinaryenFnCtxt::trans_assignment::hc4ecc00d5d562645
                        at /usr/local/google/home/eholk/wasm/mir2wasm/src/trans.rs:634
  18:     0x7f7122cec046 - mir2wasm::trans::BinaryenFnCtxt::trans::hfde2fbe142e22c3a
                        at /usr/local/google/home/eholk/wasm/mir2wasm/src/trans.rs:278
  19:     0x7f7122ceaaa2 - <mir2wasm::trans::BinaryenModuleCtxt<'v, 'tcx> as rustc::hir::intravisit::Visitor<'v>>::visit_fn::h51bafc35e583b5f8
                        at /usr/local/google/home/eholk/wasm/mir2wasm/src/trans.rs:184
  20:     0x7f7122ca7701 - rustc::hir::intravisit::walk_trait_item::hb1ec63911b017ada
                        at /buildslave/rust-buildbot/slave/nightly-dist-rustc-linux/build/obj/../src/librustc/hir/intravisit.rs:662
  21:     0x7f7122ca8aac - rustc::hir::intravisit::Visitor::visit_trait_item::hab873576703f59aa
                        at /buildslave/rust-buildbot/slave/nightly-dist-rustc-linux/build/obj/../src/librustc/hir/intravisit.rs:145
  22:     0x7f7122cac5ff - rustc::hir::intravisit::walk_item::he9e3385587e0fc9d
                        at /usr/local/google/home/eholk/wasm/mir2wasm/<syntax macros>:2
  23:     0x7f7122ca873c - rustc::hir::intravisit::Visitor::visit_item::h1dc71540c0e4b486
                        at /buildslave/rust-buildbot/slave/nightly-dist-rustc-linux/build/obj/../src/librustc/hir/intravisit.rs:92
  24:     0x7f7122cac8e9 - rustc::hir::Crate::visit_all_items::hb6fdac0d00839bd9
                        at /buildslave/rust-buildbot/slave/nightly-dist-rustc-linux/build/obj/../src/librustc/hir/mod.rs:415
  25:     0x7f7122cea005 - mir2wasm::trans::trans_crate::hb5f4874b54b7cb66
                        at /usr/local/google/home/eholk/wasm/mir2wasm/src/trans.rs:79
  26:     0x7f7122c4d239 - <mir2wasm::WasmCompilerCalls as rustc_driver::CompilerCalls<'a>>::build_controller::{{closure}}::h47f88d76f5b28e7e
                        at /usr/local/google/home/eholk/wasm/mir2wasm/src/bin/mir2wasm.rs:49
  27:     0x7f712276f8d7 - rustc_driver::driver::compile_input::{{closure}}::hdf708031c67974d4
  28:     0x7f7122760d55 - rustc_driver::driver::phase_3_run_analysis_passes::{{closure}}::hcac3a2328cd9259a
  29:     0x7f712272c695 - rustc_driver::driver::phase_3_run_analysis_passes::h9865057b323cad0e
  30:     0x7f712271d793 - rustc_driver::driver::compile_input::hc0edbed7edb3eb18
  31:     0x7f7122749626 - rustc_driver::run_compiler::h22d678d32fb7c300
  32:     0x7f7122c4c791 - mir2wasm::main::h79208956a14bce2e
                        at /usr/local/google/home/eholk/wasm/mir2wasm/src/bin/mir2wasm.rs:137
  33:     0x7f711db49046 - __rust_maybe_catch_panic
  34:     0x7f711db39961 - std::rt::lang_start::haaae1186de9de8cb
  35:     0x7f7122c4d353 - main
  36:     0x7f711cf60f44 - __libc_start_main
  37:     0x7f7122c17d48 - <unknown>
  38:                0x0 - <unknown>

I'm guessing @brson or someone else on the Rust team knows how to fix this pretty readily.

Incidentally, I had a similar in progress branch with the same problem. Just from a cursory look, @joshuawarner32's version seems better overall, since it seems to actually handle a few cases that I just aggressively deleted.

It'd also be good to update the readme with the specific Rust nightly that this builds with so that we have a known good revision the next time nightly breaks us.

brson · Sep 13, 2016

@joshuawarner32 Thanks for the caveats. Since this is such an early prototype still I'm happy to endure some breakage, as long as it looks good to @eholk and/or @lqd. And as @kripken mentioned, there are reasons to pursue both approaches, so even once the LLVM-based approach is merged I intend to keep plugging away at this one.

Maybe @eddyb can see what's being generated incorrectly to cause that. Otherwise I can poke at it later.

eholk · Sep 13, 2016

I guess a totally legitimate way to make the tests pass in this case is the xfail the failing ones. @brson's point about enduring breakage on an early prototype is a good one; it's probably better to track nightly a little closer than we're doing than have production-quality tests.

eddyb · Sep 14, 2016

eddyb reviewed Sep 14, 2016

View changes

eddyb · Sep 14, 2016

src/traits.rs

    match fulfill_obligation(tcx, trait_ref) {
        traits::VtableImpl(vtable_impl) => {
            let impl_did = vtable_impl.impl_def_id;
            let mname = tcx.item_name(def_id);
            // Create a concatenated set of substitutions which includes those from the impl
            // and those from the method:
-            let impl_substs = vtable_impl.substs.with_method_from(substs);
-            let substs = tcx.mk_substs(impl_substs);
+            let substs = substs.rebase_onto(*tcx, def_id, vtable_impl.substs);


I think def_id here might need to be trait_id.

joshuawarner32 · Sep 14, 2016

@eholk: Not sure if this helps, but based on some println debugging, it looks like the internal compiler errors are happening while processing the methods of the ne method of the pub trait PartialEq<Rhs: ?Sized = Self> traits in the test files.

For example, here's some of the relevant values as they appear at trans.rs:1083, just before the ICE (appogies for the horrible formatting; you can see the line number at the very end of that third line):

ty: &Self
self.did: DefId { krate: 0, node: DefIndex(90) => PartialEq::ne }
self.mir: Mir { basic_blocks: [BasicBlockData { statements: [StorageLive(var0), var0 = arg0, StorageLive(var1), var1 = arg1, StorageLive(tmp0), StorageLive(tmp1), tmp1 = &(*var0), StorageLive(tmp2), tmp2 = &(*var1)], terminator: Some(Terminator { source_info: SourceInfo { span: tests/compile-pass/operators.rs:103:42: 103:56, scope: scope1 }, kind: tmp0 = <Self as PartialEq<Rhs>>::eq(tmp1, tmp2) -> bb1 }), is_cleanup: false }, BasicBlockData { statements: [return = Not(tmp0), StorageDead(tmp0), StorageDead(tmp2), StorageDead(tmp1), StorageDead(var1), StorageDead(var0)], terminator: Some(Terminator { source_info: SourceInfo { span: tests/compile-pass/operators.rs:103:5: 103:58, scope: scope1 }, kind: return }), is_cleanup: false }], visibility_scopes: [VisibilityScopeData { span: tests/compile-pass/operators.rs:103:5: 103:58, parent_scope: None }, VisibilityScopeData { span: tests/compile-pass/operators.rs:103:39: 103:58, parent_scope: Some(scope0) }], promoted: [], return_ty: bool, var_decls: [VarDecl { mutability: Not, name: self(25), ty: &Self, source_info: SourceInfo { span: tests/compile-pass/operators.rs:103:11: 103:16, scope: scope1 } }, VarDecl { mutability: Not, name: other(99), ty: &Rhs, source_info: SourceInfo { span: tests/compile-pass/operators.rs:103:18: 103:23, scope: scope1 } }], arg_decls: [ArgDecl { ty: &Self, spread: false, debug_name: self(25) }, ArgDecl { ty: &Rhs, spread: false, debug_name: other(99) }], temp_decls: [TempDecl { ty: bool }, TempDecl { ty: &Self }, TempDecl { ty: &Rhs }], upvar_decls: [], span: tests/compile-pass/operators.rs:103:5: 103:58, cache: Cache { predecessors: RefCell { value: None } } }

eddyb · Sep 14, 2016

@joshuawarner32 Did my suggestion not fix it?

lqd · Sep 14, 2016

Sorry I haven't had the time to check this out (and am unfortunately unlikely to be able to do so in the immediate future) however there were a couple of things I thought I needed to mention 😄

our generics/traits/fulfillment/monomorphization story is far from complete. It works on the simple test cases we have, but more complex cases would fail to compile and ICE with similar errors as the ones we're seeing here.
as commented on top of traits.rs, the whole thing is basically from miri (but apparently they've since updated it a lot). Specifically, from here https://github.com/solson/miri/blob/master/src/interpreter/terminator.rs — which we know works at least for their test cases and the most recent nightly they updated to (which seems recent). If this PR is updating to a similar nightly, maybe it'd be best to try their approach instead of trying to modify ours to match the more recent rustc API anyway ?

joshuawarner32 · Sep 15, 2016

@eddyb - I didn't notice a difference in the outcome of the tests after I implemented your def_id to trait_id suggestion. But it does make sense - at least to someone with only a glancing familiarity in this code.

@lqd - Do you have a concrete suggestion on how to pull that in? It's not immediately obvious to me how to adapt that to the current interface. The only parts that look remotely equatable is the ImplMethod type. I experimentally pulled those changes in, with no differences.

Tiny bit of incremental progress on the internal compiler error. The following snippet seems to be the minimal code needed to trigger the error. Removing the body on the method defined in the trait (as opposed to the impl) is enough to no longer see the error.

pub trait Test {
    fn test(&self) { } // removing the body here gets us passed the ICE
}

impl Test for isize {
    fn test(&self) { }
}

Does that ring a bell for anyone?

eddyb · Sep 15, 2016

@joshuawarner32 Sounds like it uses the default body even when the method is implemented - but it's not using the trait substs, but the impl ones, with the trait default method.

lqd · Sep 15, 2016

@joshuawarner32 yes:

traits::get_impl_method is miri's get_impl_method https://github.com/solson/miri/blob/master/src/interpreter/terminator.rs#L588
traits::fulfill_obligation is miri's fulfill_obligation slightly modified to remove the EvalContext self parameter by passing the TyCtxt directly https://github.com/solson/miri/blob/master/src/interpreter/terminator.rs#L470
traits::resolve_trait_method is miri's trait_method slightly modified in the same manner https://github.com/solson/miri/blob/master/src/interpreter/terminator.rs#L493

(There were also a couple of different error handling suggestions from eddy which miri didn't implement at the time)

It's very possible that this doesn't fix the issue indeed, but at least you won't have to update the current traits module to the newer rustc API.

It's not a problem if it doesn't fix it either, if @eholk is happy with the PR, I'd say let's merge it to get the benefits of the newer nightly, and then deal with this — as we'll have to fix other monomorphizations cases anyway!

eholk · Sep 15, 2016

Hmm, does binaryen still print out the result of running a program? It looks like some of our tests aren't showing the expected output. For example,

cargo run -- tests/compile-pass/nocore-hello-world.rs --run

doesn't give me any output, but the test is expected to print (i32.const 6).

joshuawarner32 · Sep 16, 2016

@eholk: Perhaps you're referring to tests/run-pass/nocore-hello-world.rs, which in this PR currently prints (i32.const 0)?

I tracked that down to a problem with how I assumed checked ops would work in mir. How they actually work is the CheckedAdd operation actually gives something like a (i32, i32) for (value, overflow_bit) respectively - and then there's an assert operation to check the value of the overflow bit. Since I directly emitted a value instead of a struct/pair, it got emitted as a SetLocal, whereas the code to use it got emitted as a GetLocal followed by a Load to grab the particular field. Yeah. I'll fix that.

<side-note>Longer term, it's probably best to model (non-escaping) structs on the stack as exploded locals. Otherwise, the optimizer is going to have one hell of a time cleaning that up, since the WebAssembly memory model doesn't allow eliminating that store. But I bet that could easily become a rat's nest.</side-note>

joshuawarner32 · Sep 16, 2016

Quick update:

I fixed a few of the outstanding problems. (In particular, @eholk, tests/run-pass/nocore-hello-world.rs now prints (i32.const 6) as it should.)

I'm still going to have a go at ICE failures, and possibly look at pulling in up-to-date miri code as @lqd suggested.

eholk · Sep 20, 2016

I just tried xfailing the tests that were failing, and unfortunately I ran into another issue. It looks like there's a deps test that runs afterwards and that is segfaulting now, at least for me. It seems to be trying to call a destructor on an std::vector that has already been freed.

Here's the stack:

* thread #1: tid = 23158, 0x00007ffff34af606 libstd-411f48d3.so`free + 150, name = 'mir2wasm-ce627d', stop reason = signal SIGSEGV: invalid address (fault address: 0x8)
  * frame #0: 0x00007ffff34af606 libstd-411f48d3.so`free + 150
    frame #1: 0x00007ffff60b7133 librustc_llvm-411f48d3.so`operator delete(void*) + 31
    frame #2: 0x0000555555583942 mir2wasm-ce627d33d91d8d02`__gnu_cxx::new_allocator<char>::deallocate(this=0x00007ffff0cbf120, __p="", (null)=140737233142491) + 32 at new_allocator.h:110
    frame #3: 0x000055555557ad6a mir2wasm-ce627d33d91d8d02`std::_Vector_base<char, std::allocator<char> >::_M_deallocate(this=0x00007ffff0cbf120, __p="", __n=140737233142491) + 50 at stl_vector.h:174
    frame #4: 0x00005555555746e1 mir2wasm-ce627d33d91d8d02`std::_Vector_base<char, std::allocator<char> >::~_Vector_base(this=0x00007ffff0cbf120) + 61 at stl_vector.h:160
    frame #5: 0x000055555556e407 mir2wasm-ce627d33d91d8d02`std::vector<char, std::allocator<char> >::~vector(this=0x00007ffff0cbf120) + 65 at stl_vector.h:416
    frame #6: 0x000055555562d9d4 mir2wasm-ce627d33d91d8d02`cashew::IStringSet::~IStringSet(this=0x00007ffff0cbf0f0) + 28 at istring.h:151
    frame #7: 0x0000555555631d40 mir2wasm-ce627d33d91d8d02`cashew::OperatorClass::~OperatorClass(this=0x00007ffff0cbf0f0) + 24 at parser.h:131
    frame #8: 0x0000555555631d5e mir2wasm-ce627d33d91d8d02`void __gnu_cxx::new_allocator<cashew::OperatorClass>::destroy<cashew::OperatorClass>(this=0x00005555558bb530, __p=0x00007ffff0cbf0f0) + 28 at new_allocator.h:124
    frame #9: 0x0000555555631036 mir2wasm-ce627d33d91d8d02`std::enable_if<std::allocator_traits<std::allocator<cashew::OperatorClass> >::__destroy_helper<cashew::OperatorClass>::value, void>::type std::allocator_traits<std::allocator<cashew::OperatorClass> >::_S_destroy<cashew::OperatorClass>(__a=0x00005555558bb530, __p=0x00007ffff0cbf0f0) + 35 at alloc_traits.h:281
    frame #10: 0x00005555556301e9 mir2wasm-ce627d33d91d8d02`void std::allocator_traits<std::allocator<cashew::OperatorClass> >::destroy<cashew::OperatorClass>(__a=0x00005555558bb530, __p=0x00007ffff0cbf0f0) + 35 at alloc_traits.h:405
    frame #11: 0x000055555562ed2c mir2wasm-ce627d33d91d8d02`void std::vector<cashew::OperatorClass, std::allocator<cashew::OperatorClass> >::_M_emplace_back_aux<char const (this=0x00005555558bb530, (null)=<no value available>, (null)=0x00007fffffffda80, (null)=0x00007fffffffda90) [8], bool, cashew::OperatorClass::Type>(char const (&) [8], bool&&, cashew::OperatorClass::Type&&) + 516 at vector.tcc:422
    frame #12: 0x000055555562e0a6 mir2wasm-ce627d33d91d8d02`void std::vector<cashew::OperatorClass, std::allocator<cashew::OperatorClass> >::emplace_back<char const (this=0x00005555558bb530, (null)=<no value available>, (null)=0x00007fffffffda80, (null)=0x00007fffffffda90) [8], bool, cashew::OperatorClass::Type>(char const (&) [8], bool&&, cashew::OperatorClass::Type&&) + 204 at vector.tcc:101
    frame #13: 0x000055555562da8e mir2wasm-ce627d33d91d8d02`cashew::Init::Init(this=0x00005555558bb568) + 106 at parser.cpp:125
    frame #14: 0x0000555555634325 mir2wasm-ce627d33d91d8d02`::__static_initialization_and_destruction_0(__initialize_p=1, __priority=65535) + 2557 at parser.cpp:148
    frame #15: 0x00005555556343c2 mir2wasm-ce627d33d91d8d02`::_GLOBAL__sub_I_parser.cpp() + 19 at parser.cpp:161
    frame #16: 0x00005555556356ed mir2wasm-ce627d33d91d8d02`__libc_csu_init + 77
    frame #17: 0x00007ffff28c5ed5 libc.so.6`__libc_start_main(main=(mir2wasm-ce627d33d91d8d02`main), argc=1, argv=0x00007fffffffdc08, init=(mir2wasm-ce627d33d91d8d02`__libc_csu_init), fini=<unavailable>, rtld_fini=<unavailable>, stack_end=0x00007fffffffdbf8) + 133 at libc-start.c:246
    frame #18: 0x000055555555fef9 mir2wasm-ce627d33d91d8d02`_start + 41

I'm tempted just to go ahead and open a bug to track this new issue and merge this PR, because I'd like to get it merged.

joshuawarner32 · Sep 20, 2016

@eholk - then let me take a stab this evening at isolating the portions of the tests that are actually causing the ICE, and mark just those parts as xfail. I'd rather have mostly-working tests than mostly-broken tests.

eholk · Sep 20, 2016

@joshuawarner32 - sounds good! I like mostly working tests too :) As long as you're okay to keep working on them, I'm happy to wait. I just don't want you to feel like I'm putting up arbitrary roadblocks for you or anything like that.

lqd · Sep 20, 2016

an std::vector & libstdc++ ? is this related to binaryen ?

joshuawarner32 · Sep 20, 2016

@eholk - I modified / xfailed the appropriate tests, and now everything seems to be passing.

Also, I'm not able to reproduce the std::vector / double-free issue you're seeing. I'm running macOS sierra, if that's helpful. Are you seeing that with just a cargo test?

eholk · Sep 23, 2016

Sorry @joshuawarner32, I didn't notice your update to this a few days ago. I tried it on my mac and also didn't see the crash. In the interest of moving things forward, I'm going to accept this and open an issue about the crash I'm seeing on Linux.

@lqd - I have some suspicions that it might be related to binaryen, but I don't have any real evidence to base this on yet.

lqd · Sep 23, 2016

Thanks @joshuawarner32! and thanks @eholk of course as well!

joshuawarner32 added some commits Sep 10, 2016

upgrade to nightly as of 2016-09-09, with some hacks

d56b743

commit Cargo.lock changes

f7736fa

do a little cleanup

59e39f9

joshuawarner32 added some commits Sep 10, 2016

simplify conditionals

2b1ad8a

fix warnings

65ad8c4

joshuawarner32 added some commits Sep 14, 2016

update compiletest to fix test (build) breakage

7464ba3

change def_id to trait_id

4668114

implement Bit{And,Or,Xor}

b2e4d0a

joshuawarner32 added some commits Sep 16, 2016

fix checked ops

462a16e

fix missing AssertParamIsClone

e1d5f44

document new nightly version

7ff9081

eholk referenced this pull request Sep 20, 2016
Closed
internal compiler error: ../src/librustc/ty/subst.rs:457: Type parameter `Self/#0` (Self/0) out of range when substituting (root type=Some(&Self)) substs=[] #40

joshuawarner32 added some commits Sep 20, 2016

remove default trait methods from tests where possible, to get them p…
…assing

b681772

xfail cmp.rs test, since it uses both default trait methods and gener…
…ic impls (side-step issue #40)

5c65677

eholk merged commit 6aa925e into brson:master Sep 23, 2016

brson/mir2wasm forked from rust-lang/miri

Join GitHub today

Upgrade to nightly #39

Conversation

joshuawarner32 commented Sep 10, 2016

joshuawarner32 added some commits Sep 10, 2016

joshuawarner32 reviewed Sep 10, 2016 View changes

This comment has been minimized.

This comment has been minimized.

joshuawarner32 reviewed Sep 10, 2016 View changes

This comment has been minimized.

This comment has been minimized.

joshuawarner32 reviewed Sep 10, 2016 View changes

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

joshuawarner32 commented Sep 10, 2016

eddyb reviewed Sep 10, 2016 View changes

This comment has been minimized.

eddyb reviewed Sep 10, 2016 View changes

This comment has been minimized.

This comment has been minimized.

eddyb reviewed Sep 10, 2016 View changes

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

eddyb reviewed Sep 10, 2016 View changes

This comment has been minimized.

joshuawarner32 added some commits Sep 10, 2016

This comment has been minimized.

brson commented Sep 13, 2016

This comment has been minimized.

eholk commented Sep 13, 2016

This comment has been minimized.

joshuawarner32 commented Sep 13, 2016

This comment has been minimized.

kripken commented Sep 13, 2016

This comment has been minimized.

eholk commented Sep 13, 2016

This comment has been minimized.

brson commented Sep 13, 2016

This comment has been minimized.

eholk commented Sep 13, 2016

eddyb reviewed Sep 14, 2016 View changes

This comment has been minimized.

joshuawarner32 added some commits Sep 14, 2016

This comment has been minimized.

joshuawarner32 commented Sep 14, 2016 • edited

This comment has been minimized.

eddyb commented Sep 14, 2016

This comment has been minimized.

lqd commented Sep 14, 2016

This comment has been minimized.

joshuawarner32 commented Sep 15, 2016

This comment has been minimized.

eddyb commented Sep 15, 2016

This comment has been minimized.

lqd commented Sep 15, 2016 • edited

This comment has been minimized.

eholk commented Sep 15, 2016

This comment has been minimized.

joshuawarner32 commented Sep 16, 2016

joshuawarner32 added some commits Sep 16, 2016

This comment has been minimized.

joshuawarner32 commented Sep 16, 2016

This comment has been minimized.

eholk commented Sep 20, 2016

This comment has been minimized.

joshuawarner32 commented Sep 20, 2016

eholk referenced this pull request Sep 20, 2016

This comment has been minimized.

eholk commented Sep 20, 2016

This comment has been minimized.

lqd commented Sep 20, 2016

joshuawarner32 added some commits Sep 20, 2016

This comment has been minimized.

joshuawarner32 reviewed Sep 10, 2016

View changes

joshuawarner32 reviewed Sep 10, 2016

View changes

joshuawarner32 reviewed Sep 10, 2016

View changes

eddyb reviewed Sep 10, 2016

View changes

eddyb reviewed Sep 10, 2016

View changes

eddyb reviewed Sep 10, 2016

View changes

eddyb reviewed Sep 10, 2016

View changes

eddyb reviewed Sep 14, 2016

View changes

joshuawarner32 commented Sep 14, 2016 •

edited

lqd commented Sep 15, 2016 •

edited

eholk merged commit `6aa925e` into brson:master Sep 23, 2016