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

Few LLVM lint "Unusual" #48229

Closed
leonardo-m opened this issue Feb 15, 2018 · 11 comments
Closed

Few LLVM lint "Unusual" #48229

leonardo-m opened this issue Feb 15, 2018 · 11 comments
Labels
A-codegen Area: Code generation A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@leonardo-m
Copy link

In Issue #48227 I've seen code compiled with "-C passes=lint", I've used it on some of my code and I've found few problems, reduced below:


fn foo(_: &u32, _: &u32) {}
fn main() {
    let a = 0;
    foo(&a, &a);
}


...>rustc -C passes=lint test1.rs
Unusual: noalias argument aliases another argument
  call void @_ZN4bug13foo17h2a852a9d6b40f8a0E(i32* noalias readonly dereferenceable(4) %1, i32* noalias readonly dereferenceable(4) %1)

fn main() {
    (1 .. 9).take_while(|i| i * i <= 10).any(|_| true);
}


...>rustc -C passes=lint test2.rs
Unusual: noalias argument aliases another argument
  %3 = call i32 @"_ZN86_$LT$$RF$$u27$b$u20$i32$u20$as$u20$core..ops..arith..Mul$LT$$RF$$u27$a$u20$i32$GT$$GT$3mul17hdb3638dfdb52b819E"(i32* noalias readonly dereferenceab
le(4) %1, i32* noalias readonly dereferenceable(4) %1)

fn main() {
    (1 .. 9).filter(|_| true).sum::<u32>();
}


...>rustc -C passes=lint test3.rs
Unusual: Return statement in function with noreturn attribute
  ret void


@leonardo-m
Copy link
Author

rustc 1.25.0-nightly (3ec5a99aa 2018-02-14)
binary: rustc
commit-hash: 3ec5a99aaa0084d97a9e845b34fdf03d1462c475
commit-date: 2018-02-14
host: x86_64-pc-windows-gnu
release: 1.25.0-nightly
LLVM version: 6.0

@kennytm kennytm added A-codegen Area: Code generation T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness C-bug Category: This is a bug. labels Feb 15, 2018
@nagisa
Copy link
Member

nagisa commented Feb 16, 2018

Duplicate of #7463

@nagisa nagisa marked this as a duplicate of #7463 Feb 16, 2018
@nagisa nagisa closed this as completed Feb 16, 2018
@nagisa
Copy link
Member

nagisa commented Feb 16, 2018

I guess I’ll reopen, since this contains minimal and actionable code samples.

@nagisa nagisa reopened this Feb 16, 2018
@RalfJung
Copy link
Member

This seems like the LLVM lint disagrees with the statement somewhere in the compiler sources saying that noalias on read-only data is fine even if thins alias -- because we can still reorder them?

@hanna-kruppe
Copy link
Contributor

Doesn't noalias also factor into optimizing pointer comparisons?

@RalfJung
Copy link
Member

RalfJung commented Mar 28, 2018

Well, whoever wrote that code strongly things that is not the case...

Also, depending on what exactly clang does, that would be unsound:

int foo(restrict int *x, restrict int *y) {
  (x == y) ? 1 : 0
}

This can not be optimized to return 0.

@hanna-kruppe
Copy link
Contributor

Hm, you're right, and indeed I couldn't find any such optimization in a quick skim of instcombine.

Centril added a commit to Centril/rust that referenced this issue Apr 3, 2019
Never return uninhabited values at all

Functions with uninhabited return values are already marked `noreturn`,
but we were still generating return instructions for this. When running
with `C passes=lint`, LLVM prints:

    Unusual: Return statement in function with noreturn attribute

The LLVM manual makes a stronger statement about `noreturn` though:

> This produces undefined behavior at runtime if the function ever does
dynamically return.

We now mark such return values with a new `IgnoreMode::Uninhabited`, and
emit an `abort` anywhere that would have returned.

Fixes rust-lang#48227
cc rust-lang#7463 rust-lang#48229

r? @eddyb
Centril added a commit to Centril/rust that referenced this issue Apr 3, 2019
Never return uninhabited values at all

Functions with uninhabited return values are already marked `noreturn`,
but we were still generating return instructions for this. When running
with `C passes=lint`, LLVM prints:

    Unusual: Return statement in function with noreturn attribute

The LLVM manual makes a stronger statement about `noreturn` though:

> This produces undefined behavior at runtime if the function ever does
dynamically return.

We now mark such return values with a new `IgnoreMode::Uninhabited`, and
emit an `abort` anywhere that would have returned.

Fixes rust-lang#48227
cc rust-lang#7463 rust-lang#48229

r? @eddyb
Centril added a commit to Centril/rust that referenced this issue Apr 4, 2019
Never return uninhabited values at all

Functions with uninhabited return values are already marked `noreturn`,
but we were still generating return instructions for this. When running
with `-C passes=lint`, LLVM prints:

    Unusual: Return statement in function with noreturn attribute

The LLVM manual makes a stronger statement about `noreturn` though:

> This produces undefined behavior at runtime if the function ever does
dynamically return.

We now emit an `abort` anywhere that would have tried to return an
uninhabited value.

Fixes rust-lang#48227
cc rust-lang#7463 rust-lang#48229

r? @eddyb
Centril added a commit to Centril/rust that referenced this issue Apr 4, 2019
Never return uninhabited values at all

Functions with uninhabited return values are already marked `noreturn`,
but we were still generating return instructions for this. When running
with `-C passes=lint`, LLVM prints:

    Unusual: Return statement in function with noreturn attribute

The LLVM manual makes a stronger statement about `noreturn` though:

> This produces undefined behavior at runtime if the function ever does
dynamically return.

We now emit an `abort` anywhere that would have tried to return an
uninhabited value.

Fixes rust-lang#48227
cc rust-lang#7463 rust-lang#48229

r? @eddyb
Centril added a commit to Centril/rust that referenced this issue Apr 4, 2019
Never return uninhabited values at all

Functions with uninhabited return values are already marked `noreturn`,
but we were still generating return instructions for this. When running
with `-C passes=lint`, LLVM prints:

    Unusual: Return statement in function with noreturn attribute

The LLVM manual makes a stronger statement about `noreturn` though:

> This produces undefined behavior at runtime if the function ever does
dynamically return.

We now emit an `abort` anywhere that would have tried to return an
uninhabited value.

Fixes rust-lang#48227
cc rust-lang#7463 rust-lang#48229

r? @eddyb
Centril added a commit to Centril/rust that referenced this issue Apr 4, 2019
Never return uninhabited values at all

Functions with uninhabited return values are already marked `noreturn`,
but we were still generating return instructions for this. When running
with `-C passes=lint`, LLVM prints:

    Unusual: Return statement in function with noreturn attribute

The LLVM manual makes a stronger statement about `noreturn` though:

> This produces undefined behavior at runtime if the function ever does
dynamically return.

We now emit an `abort` anywhere that would have tried to return an
uninhabited value.

Fixes rust-lang#48227
cc rust-lang#7463 rust-lang#48229

r? @eddyb
Centril added a commit to Centril/rust that referenced this issue Apr 4, 2019
Never return uninhabited values at all

Functions with uninhabited return values are already marked `noreturn`,
but we were still generating return instructions for this. When running
with `-C passes=lint`, LLVM prints:

    Unusual: Return statement in function with noreturn attribute

The LLVM manual makes a stronger statement about `noreturn` though:

> This produces undefined behavior at runtime if the function ever does
dynamically return.

We now emit an `abort` anywhere that would have tried to return an
uninhabited value.

Fixes rust-lang#48227
cc rust-lang#7463 rust-lang#48229

r? @eddyb
@cuviper
Copy link
Member

cuviper commented Apr 5, 2019

Unusual: Return statement in function with noreturn attribute

This case is fixed by #59639 as of nightly-2019-04-05.

@jonas-schievink
Copy link
Contributor

Removing I-unsound label. The remaining 2 cases ("noalias argument aliases another argument") seem to be caused by the LLVM lint being a bit overzealous. Maybe we should close this altogether?

@jonas-schievink jonas-schievink added A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. and removed I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness labels Dec 10, 2019
@cuviper
Copy link
Member

cuviper commented Dec 10, 2019

Those noalias lints should be relaxed by D60239, which is in LLVM 9. I will try to do a new rustc build with lints enabled to see if anything remains.

@jonas-schievink
Copy link
Contributor

Can confirm they're gone on rustc 1.41.0-nightly (797fd9262 2019-11-26) with -Cpasses=lint. In some other code, some of these have shown up:

Unusual: unreachable immediately preceded by instruction without side effects
  unreachable, !dbg !112

This shouldn't really be an issue and is just a bit sloppy codegen by rustc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-codegen Area: Code generation A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. C-bug Category: This is a bug. 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

7 participants