rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
97.53k stars 12.61k forks source link

division by zero on Box<dyn Iterator<Item = _>> #67638

Closed matthiaskrgr closed 4 years ago

matthiaskrgr commented 4 years ago

rustc src/test/ui/issues/issue-26805.rs -Cpasses=lint

Undefined behavior: Division by zero
  %26 = sdiv exact i64 %25, 0

code is

// run-pass
struct NonOrd;

fn main() {
    let _: Box<dyn Iterator<Item = _>> = Box::new(vec![NonOrd].into_iter());
}

rustc @ bbf13723bc22f1a850438bf0b103d09e474a1ef5

jonas-schievink commented 4 years ago

This is a false positive since offset_from will panic if the type is a ZST. The IntoIter code uses a different code path for ZSTs as well:

https://github.com/rust-lang/rust/blob/3ac40b69c75929dac5115b6a49eb4f1ecc352416/src/liballoc/vec.rs#L2536-L2549

The lint doesn't seem to trigger on regular divisions by zero since that seems to go through a load/store pair.

Mark-Simulacrum commented 4 years ago

Relevant LLVM IR:

; core::ptr::<impl *const T>::offset_from
; Function Attrs: inlinehint uwtable
define internal i64 @"_ZN4core3ptr33_$LT$impl$u20$$BP$const$u20$T$GT$11offset_from17h5d75b21cc3f5b3e2E"(%NonOrd* %self, %NonOrd* %origin) unnamed_addr #0 {
start:
  %0 = alloca i64, align 8
  %1 = alloca i64, align 8
  %ok = alloca i8, align 1
  store i64 0, i64* %0, align 8
  %2 = load i64, i64* %0, align 8
  br label %bb1

bb1:                                              ; preds = %start
  %_5 = icmp ult i64 0, %2
  br i1 %_5, label %bb4, label %bb3

bb2:                                              ; preds = %bb6
  store i8 1, i8* %ok, align 1
  br label %bb5

bb3:                                              ; preds = %bb6, %bb1
  store i8 0, i8* %ok, align 1
  br label %bb5

bb4:                                              ; preds = %bb1
  br label %bb6

bb5:                                              ; preds = %bb3, %bb2
  %3 = load i8, i8* %ok, align 1, !range !12
  %_13 = trunc i8 %3 to i1
  %_12 = xor i1 %_13, true
  %4 = icmp ule i1 %_12, true
  call void @llvm.assume(i1 %4)
  %_11 = zext i1 %_12 to i64
  %_15 = icmp ult i64 %_11, 1
  %5 = call i1 @llvm.expect.i1(i1 %_15, i1 true)
  br i1 %5, label %bb7, label %panic

bb6:                                              ; preds = %bb4
  %_7 = icmp ule i64 %2, 9223372036854775807
  br i1 %_7, label %bb2, label %bb3

bb7:                                              ; preds = %bb5
  %6 = ptrtoint %NonOrd* %self to i64
  %7 = ptrtoint %NonOrd* %origin to i64
  %8 = sub i64 %6, %7
  %9 = sdiv exact i64 %8, 0
  store i64 %9, i64* %1, align 8
  %10 = load i64, i64* %1, align 8
  br label %bb8

@jonas-schievink conclusion seems correct here so I'm going to go ahead and close. If anything, this seems like an LLVM bug as the sdiv is in dead code.

OTOH, we could plausibly consider this a const prop / MIR opt bug (unclear whether that's entirely true, though, given the dependence on monomorphization here) -- I'm not sure we can do any better than LLVM.

@matthiaskrgr feel free to reopen if you have further thoughts