From 97e41435581ddefa68fc34d318658593190eb1c0 Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Wed, 12 Oct 2016 17:36:04 +0200 Subject: [PATCH 1/3] Fix ICE by injecting bitcasts if types mismatch when building invokes or calls. --- src/librustc_trans/builder.rs | 49 +++++++++++++++++++++++------------ 1 file changed, 32 insertions(+), 17 deletions(-) diff --git a/src/librustc_trans/builder.rs b/src/librustc_trans/builder.rs index 90f96af549691..7e72b977de458 100644 --- a/src/librustc_trans/builder.rs +++ b/src/librustc_trans/builder.rs @@ -22,6 +22,7 @@ use value::Value; use util::nodemap::FnvHashMap; use libc::{c_uint, c_char}; +use std::borrow::Cow; use std::ffi::CString; use std::ptr; use syntax_pos::Span; @@ -175,8 +176,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { .collect::>() .join(", ")); - check_call("invoke", llfn, args); - + let args = self.check_call("invoke", llfn, args); let bundle = bundle.as_ref().map(|b| b.raw()).unwrap_or(ptr::null_mut()); unsafe { @@ -857,8 +857,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { .collect::>() .join(", ")); - check_call("call", llfn, args); - + let args = self.check_call("call", llfn, args); let bundle = bundle.as_ref().map(|b| b.raw()).unwrap_or(ptr::null_mut()); unsafe { @@ -1100,10 +1099,11 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { llvm::LLVMRustBuildAtomicFence(self.llbuilder, order, scope); } } -} -fn check_call(typ: &str, llfn: ValueRef, args: &[ValueRef]) { - if cfg!(debug_assertions) { + fn check_call<'b>(&self, + typ: &str, + llfn: ValueRef, + args: &'b [ValueRef]) -> Cow<'b, [ValueRef]> { let mut fn_ty = val_ty(llfn); // Strip off pointers while fn_ty.kind() == llvm::TypeKind::Pointer { @@ -1115,16 +1115,31 @@ fn check_call(typ: &str, llfn: ValueRef, args: &[ValueRef]) { let param_tys = fn_ty.func_params(); - let iter = param_tys.into_iter() - .zip(args.iter().map(|&v| val_ty(v))); - for (i, (expected_ty, actual_ty)) in iter.enumerate() { - if expected_ty != actual_ty { - bug!("Type mismatch in function call of {:?}. \ - Expected {:?} for param {}, got {:?}", - Value(llfn), - expected_ty, i, actual_ty); + let all_args_match = param_tys.iter() + .zip(args.iter().map(|&v| val_ty(v))) + .all(|(expected_ty, actual_ty)| *expected_ty == actual_ty); + + if all_args_match { + return Cow::Borrowed(args); + } + + let casted_args: Vec<_> = param_tys.into_iter() + .zip(args.iter()) + .enumerate() + .map(|(i, (expected_ty, &actual_val))| { + let actual_ty = val_ty(actual_val); + if expected_ty != actual_ty { + debug!("Type mismatch in function call of {:?}. \ + Expected {:?} for param {}, got {:?}; injecting bitcast", + Value(llfn), + expected_ty, i, actual_ty); + self.bitcast(actual_val, expected_ty) + } else { + actual_val + } + }) + .collect(); - } - } + return Cow::Owned(casted_args); } } From f166930dfa77f0aa6c1e5601f94712c170cc2f4b Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Wed, 12 Oct 2016 19:26:06 +0200 Subject: [PATCH 2/3] Inject bitcast if types mismatch when building a store instruction. --- src/librustc_trans/builder.rs | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/src/librustc_trans/builder.rs b/src/librustc_trans/builder.rs index 7e72b977de458..8556e95903c18 100644 --- a/src/librustc_trans/builder.rs +++ b/src/librustc_trans/builder.rs @@ -543,6 +543,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { debug!("Store {:?} -> {:?}", Value(val), Value(ptr)); assert!(!self.llbuilder.is_null()); self.count_insn("store"); + let ptr = self.check_store(val, ptr); unsafe { llvm::LLVMBuildStore(self.llbuilder, val, ptr) } @@ -552,6 +553,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { debug!("Store {:?} -> {:?}", Value(val), Value(ptr)); assert!(!self.llbuilder.is_null()); self.count_insn("store.volatile"); + let ptr = self.check_store(val, ptr); unsafe { let insn = llvm::LLVMBuildStore(self.llbuilder, val, ptr); llvm::LLVMSetVolatile(insn, llvm::True); @@ -562,6 +564,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { pub fn atomic_store(&self, val: ValueRef, ptr: ValueRef, order: AtomicOrdering) { debug!("Store {:?} -> {:?}", Value(val), Value(ptr)); self.count_insn("store.atomic"); + let ptr = self.check_store(val, ptr); unsafe { let ty = Type::from_ref(llvm::LLVMTypeOf(ptr)); let align = llalign_of_pref(self.ccx, ty.element_type()); @@ -1100,6 +1103,27 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { } } + /// Returns the ptr value that should be used for storing `val`. + fn check_store<'b>(&self, + val: ValueRef, + ptr: ValueRef) -> ValueRef { + let dest_ptr_ty = val_ty(ptr); + let stored_ty = val_ty(val); + let stored_ptr_ty = stored_ty.ptr_to(); + + assert_eq!(dest_ptr_ty.kind(), llvm::TypeKind::Pointer); + + if dest_ptr_ty == stored_ptr_ty { + ptr + } else { + debug!("Type mismatch in store. \ + Expected {:?}, got {:?}; inserting bitcast", + dest_ptr_ty, stored_ptr_ty); + self.bitcast(ptr, stored_ptr_ty) + } + } + + /// Returns the args that should be used for a call to `llfn`. fn check_call<'b>(&self, typ: &str, llfn: ValueRef, From 05626546081394f4ba0c9d916c35e48cc29d2af6 Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Wed, 12 Oct 2016 19:29:50 +0200 Subject: [PATCH 3/3] Some tests to check that lifetime parametric fn's do not trip up LLVM. --- .../issue-36744-bitcast-args-if-needed.rs | 32 +++++++++++++++++++ .../run-pass/issue-36744-without-calls.rs | 22 +++++++++++++ 2 files changed, 54 insertions(+) create mode 100644 src/test/run-pass/issue-36744-bitcast-args-if-needed.rs create mode 100644 src/test/run-pass/issue-36744-without-calls.rs diff --git a/src/test/run-pass/issue-36744-bitcast-args-if-needed.rs b/src/test/run-pass/issue-36744-bitcast-args-if-needed.rs new file mode 100644 index 0000000000000..1859cc9ca00b5 --- /dev/null +++ b/src/test/run-pass/issue-36744-bitcast-args-if-needed.rs @@ -0,0 +1,32 @@ +// Copyright 2016 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// This tests for an ICE (and, if ignored, subsequent LLVM abort) when +// a lifetime-parametric fn is passed into a context whose expected +// type has a differing lifetime parameterization. + +struct A<'a> { + _a: &'a i32, +} + +fn call(s: T, functions: &Vec fn(&'n T)>) { + for function in functions { + function(&s); + } +} + +fn f(a: &A) { println!("a holds {}", a._a); } + +fn main() { + let a = A { _a: &10 }; + + let vec: Vec fn(&'u A<'v>)> = vec![f]; + call(a, &vec); +} diff --git a/src/test/run-pass/issue-36744-without-calls.rs b/src/test/run-pass/issue-36744-without-calls.rs new file mode 100644 index 0000000000000..1766edb06b481 --- /dev/null +++ b/src/test/run-pass/issue-36744-without-calls.rs @@ -0,0 +1,22 @@ +// Copyright 2016 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// Tests for an LLVM abort when storing a lifetime-parametric fn into +// context that is expecting one that is not lifetime-parametric +// (i.e. has no `for <'_>`). + +pub struct A<'a>(&'a ()); +pub struct S(T); + +pub fn bad<'s>(v: &mut S)>, y: S fn(A<'b>)>) { + *v = y; +} + +fn main() {}