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

rustc fails to detect duplicate switch cases due to integer overflow #13727

Closed
farcaller opened this issue Apr 24, 2014 · 7 comments
Closed

rustc fails to detect duplicate switch cases due to integer overflow #13727

farcaller opened this issue Apr 24, 2014 · 7 comments
Labels
A-codegen Area: Code generation I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️

Comments

@farcaller
Copy link
Contributor

A small app to demonstrate the problem:

fn test(val: u8) {
  match val {
    256 => print!("0b1110\n"),
    512 => print!("0b1111\n"),
    _   => print!("fail\n"),
  }
}

fn main() {
  test(1);
}

Compilation fails in llvm:

% rustc test.rs
test.rs:3:5: 3:11 warning: literal out of range for its type, #[warn(type_overflow)] on by default
test.rs:3     256 => print!("0b1110\n"),
              ^~~~~~
test.rs:4:5: 4:11 warning: literal out of range for its type, #[warn(type_overflow)] on by default
test.rs:4     512 => print!("0b1111\n"),
              ^~~~~~
Duplicate integer as switch case
  switch i8 %1, label %match_else [
    i8 0, label %match_case
    i8 0, label %match_case3
  ]
i8 0
LLVM ERROR: Broken function found, compilation aborted!

Rustc should detect the double label due to overflow.

@steveklabnik
Copy link
Member

Triage: still reproduces.

@remram44
Copy link
Contributor

remram44 commented Mar 4, 2015

Still happens. Interestingly, rustc then crashed (Windows, 32b, nightly 1576142 2015-03-01).

@frewsxcv
Copy link
Member

Visiting for triage: this is still an issue

@mkpankov
Copy link
Contributor

Seems this is not due to overflow. I've triggered the bug with this code:

https://is.gd/8DWf5V

@frewsxcv
Copy link
Member

Seems this is not due to overflow. I've triggered the bug with this code:

Reduced:

#[allow(dead_code)]
#[repr(u32)]
enum SomeEnum {
    A = 0x0,
    B = 0x0,
}

#[allow(dead_code)]
fn a(b: SomeEnum) {
    match b {
        SomeEnum::A => (),
        SomeEnum::B => (),
    }
}

fn main() {}
Duplicate integer as switch case
  switch i32 %1, label %match_else [
    i32 0, label %match_case
    i32 0, label %match_case2
  ], !dbg !22
i32 0
LLVM ERROR: Broken function found, compilation aborted!
playpen: application terminated with error code 1

https://is.gd/poTgnO

@eefriedman
Copy link
Contributor

I can't reproduce this on nightly; the 256/512 testcase gives an "unreachable pattern" error, and the enum testcase gives "discriminant value 0u32 already exists".

@frewsxcv
Copy link
Member

frewsxcv commented Jun 1, 2016

I'll create regression tests for this.

frewsxcv added a commit to frewsxcv/rust that referenced this issue Jun 7, 2016
bors added a commit that referenced this issue Jun 8, 2016
arcnmx pushed a commit to arcnmx/rust that referenced this issue Dec 17, 2022
…hievink

fix: add fallback case in generated `PartialEq` impl

Partially fixes rust-lang#13727.

When generating `PartialEq` implementations for enums, the original code can already generate the following fallback case:

```rs
_ => std::mem::discriminant(self) == std::mem::discriminant(other),
```

However, it has been suppressed in the following example for no good reason:

```rs
enum Either<T, U> {
    Left(T),
    Right(U),
}

impl<T, U> PartialEq for Either<T, U> {
    fn eq(&self, other: &Self) -> bool {
        match (self, other) {
            (Self::Left(l0), Self::Left(r0)) => l0 == r0,
            (Self::Right(l0), Self::Right(r0)) => l0 == r0,
            // _ => std::mem::discriminant(self) == std::mem::discriminant(other),
            // ^ this completes the match arms!
        }
    }
}
```

This PR has removed that suppression logic.

~~Of course, the PR could have suppressed the fallback case generation for single-variant enums instead, but I believe that this case is quite rare and should be caught by `#[warn(unreachable_patterns)]` anyway.~~

After this fix, when the enum has >1 variants, the following fallback arm will be generated :

* `_ => false,` if we've already gone through every case where the variants of `self` and `other` match;
* The original one (as stated above) in other cases.

---

Note: The code example is still wrong after the fix due to incorrect trait bounds.
arcnmx pushed a commit to arcnmx/rust that referenced this issue Jan 9, 2023
…Veykril

fix: add generic `TypeBoundList` in generated derivable impl

Potentially fixes rust-lang#13727.

Continuing with the work in rust-lang#13732, this fix tries to add correct type bounds in the generated `impl` block:

```diff
  enum Either<T, U> {
      Left(T),
      Right(U),
  }

- impl<T, U> PartialEq for Either<T, U> {
+ impl<T: PartialEq, U: PartialEq> PartialEq for Either<T, U> {
      fn eq(&self, other: &Self) -> bool {
          match (self, other) {
              (Self::Left(l0), Self::Left(r0)) => l0 == r0,
              (Self::Right(l0), Self::Right(r0)) => l0 == r0,
              _ => false,
          }
      }
  }
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-codegen Area: Code generation I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️
Projects
None yet
Development

No branches or pull requests

7 participants