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

[Nightly] LLVM Assertion: SrcTy must be larger than DestTy for Trunc #41744

Closed
steffengy opened this issue May 4, 2017 · 15 comments
Closed

[Nightly] LLVM Assertion: SrcTy must be larger than DestTy for Trunc #41744

steffengy opened this issue May 4, 2017 · 15 comments
Assignees
Labels
A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. I-crash Issue: The compiler crashes (SIGSEGV, SIGABRT, etc). Use I-ICE instead when the compiler panics. P-high High priority regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@steffengy
Copy link
Contributor

steffengy commented May 4, 2017

Last good version: rustc 1.19.0-nightly (6a5fc9eec 2017-05-02) (previous nightly)
Version: rustc 1.19.0-nightly (2d4ed8e0c 2017-05-03)

Unfortunately I can't provide a small testcase for reproduction,
but the following works reliably:

Reproduction

EDIT: For readability the reduced example from below:

trait Tc {}
impl Tc for bool {}

fn main() {
    let _: &[&Tc] = &[&true];
}

OLD:

# You'll need openssl for the build process on linux, e.g. for ubuntu:
sudo apt-get install libssl-dev
git clone https://github.com/steffengy/tiberius -b ssl
cd tiberius && cargo test

Output:

rustc: /checkout/src/llvm/lib/IR/Constants.cpp:1568: static llvm::Constant*
 llvm::ConstantExpr::getTrunc(llvm::Constant*, llvm::Type*, bool): 
Assertion `C->getType()->getScalarSizeInBits() > Ty->getScalarSizeInBits()&& 
SrcTy must be larger than DestTy for Trunc!"' failed.
@kennytm
Copy link
Member

kennytm commented May 4, 2017

The error comes from the test_bit_0 and test_bit_1 in src/types/mod.rs.

@steffengy
Copy link
Contributor Author

steffengy commented May 4, 2017

That was fast.

Just to add a bit of context:

test_bit_0 and test_bit_1 are tests that ensure that different types work
when passed as query parameters.

The difference between test_bit_0 and test_i8 is only in
what the following two traits return for a specific value
(test_bit_0 tests for bool [more specific for false], test_i8 for i8, ...):

pub trait ToColumnData {
    fn to_column_data(&self) -> ColumnData;
}
pub trait ToSql : ToColumnData {
    fn to_sql(&self) -> &'static str;
}

ColumnData is an enum of several possible values (e.g. ColumnData::I8(i8))

Interestingly enough it's not enough for reproduction to just define
ToSql for bool and call to_sql and/or to_column_data for
a (casted) value boolval as &ToSql.

(And FromColumnData is used for the .get assert)

Therefore I'm not quite sure about what's actually causing this.

@kennytm
Copy link
Member

kennytm commented May 4, 2017

Reduced test case:

trait Tc {}
impl Tc for bool {}

fn main() {
    let _: &[&Tc] = &[&true];
}

(Tc is the ToSql or ToColumnData trait in the real code).

@kennytm
Copy link
Member

kennytm commented May 4, 2017

LLDB Stack trace
(lldb) r
Process 34235 launched: '~/.rustup/toolchains/nightly-x86_64-apple-darwin/bin/rustc' (x86_64)
Assertion failed: (C->getType()->getScalarSizeInBits() > Ty->getScalarSizeInBits()&& "SrcTy must be larger than DestTy for Trunc!"), function getTrunc, file /Users/travis/build/rust-lang/rust/src/llvm/lib/IR/Constants.cpp, line 1568.
Process 34235 stopped
* thread #2, name = 'rustc', stop reason = signal SIGABRT
    frame #0: 0x00007fffc99b9d42 libsystem_kernel.dylib`__pthread_kill + 10
libsystem_kernel.dylib`__pthread_kill:
->  0x7fffc99b9d42 <+10>: jae    0x7fffc99b9d4c            ; <+20>
    0x7fffc99b9d44 <+12>: movq   %rax, %rdi
    0x7fffc99b9d47 <+15>: jmp    0x7fffc99b2caf            ; cerror_nocancel
    0x7fffc99b9d4c <+20>: retq   
(lldb) bt
* thread #2, name = 'rustc', stop reason = signal SIGABRT
  * frame #0: 0x00007fffc99b9d42 libsystem_kernel.dylib`__pthread_kill + 10
    frame #1: 0x00007fffc9aa75bf libsystem_pthread.dylib`pthread_kill + 90
    frame #2: 0x00007fffc991f420 libsystem_c.dylib`abort + 129
    frame #3: 0x00007fffc98e6893 libsystem_c.dylib`__assert_rtn + 320
    frame #4: 0x0000000103f45f84 librustc_llvm-a020c9c42ea0ddd7.dylib`llvm::ConstantExpr::getTrunc(llvm::Constant*, llvm::Type*, bool) + 484
    frame #5: 0x0000000100fd13ec librustc_trans-2a23ec83470b8da3.dylib`rustc_trans::mir::constant::MirConstContext::const_lvalue::hcabbdbff5e8ff317 + 2668
    frame #6: 0x0000000100fd2422 librustc_trans-2a23ec83470b8da3.dylib`rustc_trans::mir::constant::MirConstContext::const_rvalue::h19d240f0e896d085 + 418
    frame #7: 0x0000000100fcfad0 librustc_trans-2a23ec83470b8da3.dylib`rustc_trans::mir::constant::MirConstContext::trans::ha4a4f90ba0446139 + 1840
    frame #8: 0x0000000100fd6749 librustc_trans-2a23ec83470b8da3.dylib`rustc_trans::mir::constant::_$LT$impl$u20$rustc_trans..mir..MirContext$LT$$u27$a$C$$u20$$u27$tcx$GT$$GT$::trans_constant::hfab8591e311015c1 + 697
    frame #9: 0x0000000100fd9f91 librustc_trans-2a23ec83470b8da3.dylib`rustc_trans::mir::operand::_$LT$impl$u20$rustc_trans..mir..MirContext$LT$$u27$a$C$$u20$$u27$tcx$GT$$GT$::trans_operand::he38cb0ddc21903f4 + 49
    frame #10: 0x0000000100fdb6f1 librustc_trans-2a23ec83470b8da3.dylib`rustc_trans::mir::rvalue::_$LT$impl$u20$rustc_trans..mir..MirContext$LT$$u27$a$C$$u20$$u27$tcx$GT$$GT$::trans_rvalue_operand::hee4d435691371db0 + 129
    frame #11: 0x0000000100fc6944 librustc_trans-2a23ec83470b8da3.dylib`rustc_trans::mir::block::_$LT$impl$u20$rustc_trans..mir..MirContext$LT$$u27$a$C$$u20$$u27$tcx$GT$$GT$::trans_block::he55852e7fc7fd602 + 4404
    frame #12: 0x0000000100fc351c librustc_trans-2a23ec83470b8da3.dylib`rustc_trans::mir::trans_mir::h2b1fef795fef4867 + 16268
    frame #13: 0x0000000100fe875e librustc_trans-2a23ec83470b8da3.dylib`rustc_trans::trans_item::TransItem::define::h66a94f3ed0cbd750 + 2414
    frame #14: 0x0000000100f8190b librustc_trans-2a23ec83470b8da3.dylib`rustc_trans::base::trans_crate::hc8538e6b8fc181e8 + 12955
    frame #15: 0x000000010019706f librustc_driver-45e2d96c004bc54e.dylib`rustc_driver::driver::phase_4_translate_to_llvm::h8794d833f6109935 + 1567
<snip>

Triggered from this line:

                            if projected_ty.is_bool() {
                                unsafe {
                                    val = llvm::LLVMConstTrunc(val, Type::i1(self.ccx).to_ref());
                                }
                            }

The LLVM assertion:

Constant *ConstantExpr::getTrunc(Constant *C, Type *Ty, bool OnlyIfReduced) {
  ...
  assert(C->getType()->getScalarSizeInBits() > Ty->getScalarSizeInBits()&&
"SrcTy must be larger than DestTy for Trunc!");

@kennytm
Copy link
Member

kennytm commented May 4, 2017

Can't repro on the master version.

Probably already fixed. I suggest you wait for tomorrow's nightly.

@TimNN TimNN added A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. I-crash Issue: The compiler crashes (SIGSEGV, SIGABRT, etc). Use I-ICE instead when the compiler panics. labels May 4, 2017
@TimNN
Copy link
Contributor

TimNN commented May 4, 2017

Still fails on master for me, did you build with LLVM Assertions, @kennytm?

@TimNN TimNN added regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels May 4, 2017
@kennytm
Copy link
Member

kennytm commented May 4, 2017

@TimNN Ah llvm.assertions = false by default. So right it is likely still not fixed. Sorry.

@brson brson added the P-high High priority label May 4, 2017
@TimNN
Copy link
Contributor

TimNN commented May 4, 2017

Introduced in the rollup #41717 which leads me to highly suspect #41625 (cc @nikomatsakis).

@TimNN
Copy link
Contributor

TimNN commented May 4, 2017

The problem appears to be that in https://github.com/rust-lang/rust/blob/master/src/librustc_trans/mir/constant.rs#L419 ConstantExpr::getTrunc is called with a bool constant and bool type as an argument, which causes the assertion because 1 > 1 is obviously false.

I don't know enough about that part of the compiler / LLVM to decide what the fix here should be.

@nikomatsakis
Copy link
Contributor

Off-hand, I see no reason that #41625 would be "at fault" here, or would even change the MIR being generated, but I guess I can't rule that out. It might also have just exposed an underlying problem, I'm not sure. Certainly it looks like the most suspicious member of #41717

@TimNN
Copy link
Contributor

TimNN commented May 4, 2017

Here is the diff of --emit=mir -Zno-trans: https://gist.github.com/TimNN/0bf68095617926e0046fa7e5205e5c5e

@TimNN
Copy link
Contributor

TimNN commented May 4, 2017

Ah, so with -Zmir-opt-level=0, the assertion also occurs before #41625 (and also (long) before the LLVM 4.0 upgrade).

@TimNN
Copy link
Contributor

TimNN commented May 4, 2017

With -Zmir-opt-level=0 this was introduced between nightly-2017-01-08 and nightly-2017-01-12 (Changes).

@TimNN
Copy link
Contributor

TimNN commented May 4, 2017

I've looked over the PRs merged in that period and suspect this was a pre-existing bug which was exposed by #38837. Other potentially interesting PRs in that range: #38813, #38989

@arielb1 arielb1 assigned eddyb and unassigned TimNN May 11, 2017
@nikomatsakis
Copy link
Contributor

Seems like the right fix is just to skip the call to truncate if the thing is already boolean. @eddyb will take it.

Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this issue May 12, 2017
rustc_trans: do not attempt to truncate an i1 const to i1.

Fixes rust-lang#41744 by skipping the truncation when it'd be a noop anyway.
Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this issue May 12, 2017
rustc_trans: do not attempt to truncate an i1 const to i1.

Fixes rust-lang#41744 by skipping the truncation when it'd be a noop anyway.
Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this issue May 12, 2017
rustc_trans: do not attempt to truncate an i1 const to i1.

Fixes rust-lang#41744 by skipping the truncation when it'd be a noop anyway.
Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this issue May 12, 2017
rustc_trans: do not attempt to truncate an i1 const to i1.

Fixes rust-lang#41744 by skipping the truncation when it'd be a noop anyway.
Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this issue May 12, 2017
rustc_trans: do not attempt to truncate an i1 const to i1.

Fixes rust-lang#41744 by skipping the truncation when it'd be a noop anyway.
Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this issue May 13, 2017
rustc_trans: do not attempt to truncate an i1 const to i1.

Fixes rust-lang#41744 by skipping the truncation when it'd be a noop anyway.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. I-crash Issue: The compiler crashes (SIGSEGV, SIGABRT, etc). Use I-ICE instead when the compiler panics. P-high High priority regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

6 participants