-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
Add known-bug
tests for all(?) I-unsound issues
#105107
Comments
@jackh726 I would like to give this a shot, any hints on where I could begin |
@sladyn98 sure! Okay, so we have a concept of Note the The goal for this issue is to go through the open I-unsound issues (https://github.com/rust-lang/rust/issues?page=1&q=is%3Aopen+is%3Aissue+label%3AI-unsound), identify a minimal reproduction that shouldn't pass, but does, and add a Nearly all of the issues should have a minimal repro in the comments somewhere, so I would start with those. Tests should go in Let me know if that helps! |
#105084 |
My previous comment still applies; there haven't been any PRs opened for this. If anything is confusing, please let me know. |
Part of the resolution to rust-lang#105107
Part of the resolution to rust-lang#105107
Part of the resolution to rust-lang#105107
…er-errors Add known-bug tests for a few I-unsound issues Just a few commits to push rust-lang#105107 forward. r? `@jackh726`
…er-errors Add known-bug tests for a few I-unsound issues Just a few commits to push rust-lang#105107 forward. r? ``@jackh726``
NOTE: #105084 is already added |
…unsound-issues, r=jackh726 Add `known-bug` tests for 11 unsound issues r? `@jackh726` Should tests for other issues be in separate PRs? Thanks. Edit: Partially addresses rust-lang#105107. This PR adds `known-bug` tests for 11 unsound issues: - rust-lang#25860 - rust-lang#49206 - rust-lang#57893 - rust-lang#84366 - rust-lang#84533 - rust-lang#84591 - rust-lang#85099 - rust-lang#98117 - rust-lang#100041 - rust-lang#100051 - rust-lang#104005
…unsound-issues, r=jackh726 Add `known-bug` tests for 11 unsound issues r? ``@jackh726`` Should tests for other issues be in separate PRs? Thanks. Edit: Partially addresses rust-lang#105107. This PR adds `known-bug` tests for 11 unsound issues: - rust-lang#25860 - rust-lang#49206 - rust-lang#57893 - rust-lang#84366 - rust-lang#84533 - rust-lang#84591 - rust-lang#85099 - rust-lang#98117 - rust-lang#100041 - rust-lang#100051 - rust-lang#104005
…unsound-issues, r=jackh726 Add `known-bug` tests for 11 unsound issues r? `@jackh726` Should tests for other issues be in separate PRs? Thanks. Edit: Partially addresses rust-lang#105107. This PR adds `known-bug` tests for 11 unsound issues: - rust-lang#25860 - rust-lang#49206 - rust-lang#57893 - rust-lang#84366 - rust-lang#84533 - rust-lang#84591 - rust-lang#85099 - rust-lang#98117 - rust-lang#100041 - rust-lang#100051 - rust-lang#104005
…unsound-issues, r=jackh726 Add `known-bug` tests for 11 unsound issues r? ``@jackh726`` Should tests for other issues be in separate PRs? Thanks. Edit: Partially addresses rust-lang#105107. This PR adds `known-bug` tests for 11 unsound issues: - rust-lang#25860 - rust-lang#49206 - rust-lang#57893 - rust-lang#84366 - rust-lang#84533 - rust-lang#84591 - rust-lang#85099 - rust-lang#98117 - rust-lang#100041 - rust-lang#100051 - rust-lang#104005
…unsound-issues, r=jackh726 Add `known-bug` tests for 11 unsound issues r? `@jackh726` Should tests for other issues be in separate PRs? Thanks. Edit: Partially addresses rust-lang#105107. This PR adds `known-bug` tests for 11 unsound issues: - rust-lang#25860 - rust-lang#49206 - rust-lang#57893 - rust-lang#84366 - rust-lang#84533 - rust-lang#84591 - rust-lang#85099 - rust-lang#98117 - rust-lang#100041 - rust-lang#100051 - rust-lang#104005
…unsound-issues, r=jackh726 Add `known-bug` tests for 11 unsound issues r? ``@jackh726`` Should tests for other issues be in separate PRs? Thanks. Edit: Partially addresses rust-lang#105107. This PR adds `known-bug` tests for 11 unsound issues: - rust-lang#25860 - rust-lang#49206 - rust-lang#57893 - rust-lang#84366 - rust-lang#84533 - rust-lang#84591 - rust-lang#85099 - rust-lang#98117 - rust-lang#100041 - rust-lang#100051 - rust-lang#104005
Big thanks to @gburgessiv for the big checklist! Very useful. @jackh726 Thanks for adding the test labels and updating the issue. Looking through the remaining issues, I'm a bit lost for most of these. I'm comfortable adding tests where the mcve compiles but shouldn't, but I'm wondering what to do for mcves that don't quite follow this pattern or have other variations: multiple crates, llvm/asm/etc, panic/warning/error, etc. Below, I've organized the remaining issues as I see them, hopefully useful for other contributors as well. (Currently only 44 issues in my list vs 55 actual, so missing 11 issues as of 2023.04.28) Let me know if there's anything else sort of bookkeeping that would be useful here. 1. Not able to find an mcve for these 7 issues:
2. Tracking issue needs test?Wondering if the specialization tracking issue (issue 31844) needs a test, since it's sort of multiple issues in 1 issue. 3. These 4 issues already include tests on master:
4. These 2 issues have an mcve, but don't work as is on playground:
5. These 3 issues may need regression tests instead?
6. Not sure how to test: These 11 issues involve llvm, asm, C, etc
7. Not sure how to test: These 10 issues error, segfault, or panic:
8. Not sure how to test: These 2 mcves split code across multiple crates
9. This mcve compiles, but not sure if it's supposed to error?
10.
|
issue | name | mcve |
---|---|---|
Keeping references to #[thread_local] statics is allowed across yields. | #49682 (comment) | |
Specialization and lifetime dispatch | #40582 (comment) | |
negative_impls and optin_builtin_traits nightly-features allow trait impls to overlap | #74629 (comment) | |
specialization: default items completely drop candidates instead of ambiguity | #105782 (comment) |
11. These 3 issues are part of @gburgessiv's PR 108445:
issue | name |
---|---|
105787 | occurs check with projections results in error, not ambiguity |
107975 | Miscompilation: Equal pointers comparing as unequal |
108425 | TAIT shouldn't constrain impl generics |
…unsound-issues, r=jackh726 Add `known-bug` tests for 4 unsound issues This PR adds `known-bug` tests for 4 unsound issues as part of rust-lang#105107 - rust-lang#40582 - rust-lang#49682 - rust-lang#74629 - rust-lang#105782
…unsound-issues, r=jackh726 Add `known-bug` tests for 4 unsound issues This PR adds `known-bug` tests for 4 unsound issues as part of rust-lang#105107 - rust-lang#40582 - rust-lang#49682 - rust-lang#74629 - rust-lang#105782
That link is dead. I'm also not sure I understand what to do with the MCVEs. Are they to be used for writing the tests? |
Fixed
The test is essentially the MCVE, with some comments describing the issue. |
For a somewhat different approach, there is a pull request for a clippy lint for three of these issues #25860 #84591 and #100051 : The lint looks for nested references and suggests to add a lifetimes bound that is implied by the nested reference. |
Part of the resolution to rust-lang#105107
Part of the resolution to rust-lang#105107
Part of the resolution to rust-lang#105107
…illot Add tests for rust-lang#112905 This is a part of rust-lang#105107. Adds the tests from the OP in rust-lang#112905.
Add a test for rust-lang#107975 The int is zero. But also not zero. This is so much fun. This is a part of rust-lang#105107. Initially I was going to just rebase rust-lang#108445, but quite a few things changed since then: * The [mcve](rust-lang#105787 (comment)) used for rust-lang#105787 got fixed.[^upd2] * You can't just `a ?= b` for rust-lang#107975 anymore. Now you have to `a-b ?= 0`. This is what this PR does. As an additional flex, it show that three ways of converting a pointer to its address have this issue: 1. `as usize` 2. `.expose_provenance()` 3. `.addr()` * rust-lang#108425 simply got fixed. Yay. As an aside, the naming for `addr_of!` is quite unfortunate in context of provenance APIs. Because `addr_of!` gives you a pointer, but what provenance APIs refer to as "address" is the `usize` value. Oh well. UPD1: GitHub is incapable of parsing rust-lang#107975 in the PR name, so let's add it here. [^upd2]: UPD2: [The other mcve](rust-lang#105787 (comment)) does not work anymore either, saying "this behavior recently changed as a result of a bug fix; see rust-lang#56105 for details."
Add a test for rust-lang#107975 The int is zero. But also not zero. This is so much fun. This is a part of rust-lang#105107. Initially I was going to just rebase rust-lang#108445, but quite a few things changed since then: * The [mcve](rust-lang#105787 (comment)) used for rust-lang#105787 got fixed.[^upd2] * You can't just `a ?= b` for rust-lang#107975 anymore. Now you have to `a-b ?= 0`. This is what this PR does. As an additional flex, it show that three ways of converting a pointer to its address have this issue: 1. `as usize` 2. `.expose_provenance()` 3. `.addr()` * rust-lang#108425 simply got fixed. Yay. As an aside, the naming for `addr_of!` is quite unfortunate in context of provenance APIs. Because `addr_of!` gives you a pointer, but what provenance APIs refer to as "address" is the `usize` value. Oh well. UPD1: GitHub is incapable of parsing rust-lang#107975 in the PR name, so let's add it here. [^upd2]: UPD2: [The other mcve](rust-lang#105787 (comment)) does not work anymore either, saying "this behavior recently changed as a result of a bug fix; see rust-lang#56105 for details."
Add a test for rust-lang#107975 The int is zero. But also not zero. This is so much fun. This is a part of rust-lang#105107. Initially I was going to just rebase rust-lang#108445, but quite a few things changed since then: * The [mcve](rust-lang#105787 (comment)) used for rust-lang#105787 got fixed.[^upd2] * You can't just `a ?= b` for rust-lang#107975 anymore. Now you have to `a-b ?= 0`. This is what this PR does. As an additional flex, it show that three ways of converting a pointer to its address have this issue: 1. `as usize` 2. `.expose_provenance()` 3. `.addr()` * rust-lang#108425 simply got fixed. Yay. As an aside, the naming for `addr_of!` is quite unfortunate in context of provenance APIs. Because `addr_of!` gives you a pointer, but what provenance APIs refer to as "address" is the `usize` value. Oh well. UPD1: GitHub is incapable of parsing rust-lang#107975 in the PR name, so let's add it here. [^upd2]: UPD2: [The other mcve](rust-lang#105787 (comment)) does not work anymore either, saying "this behavior recently changed as a result of a bug fix; see rust-lang#56105 for details."
Add a test for rust-lang#107975 The int is zero. But also not zero. This is so much fun. This is a part of rust-lang#105107. Initially I was going to just rebase rust-lang#108445, but quite a few things changed since then: * The [mcve](rust-lang#105787 (comment)) used for rust-lang#105787 got fixed.[^upd2] * You can't just `a ?= b` for rust-lang#107975 anymore. Now you have to `a-b ?= 0`. This is what this PR does. As an additional flex, it show that three ways of converting a pointer to its address have this issue: 1. `as usize` 2. `.expose_provenance()` 3. `.addr()` * rust-lang#108425 simply got fixed. Yay. As an aside, the naming for `addr_of!` is quite unfortunate in context of provenance APIs. Because `addr_of!` gives you a pointer, but what provenance APIs refer to as "address" is the `usize` value. Oh well. UPD1: GitHub is incapable of parsing rust-lang#107975 in the PR name, so let's add it here. [^upd2]: UPD2: [The other mcve](rust-lang#105787 (comment)) does not work anymore either, saying "this behavior recently changed as a result of a bug fix; see rust-lang#56105 for details."
Add a test for rust-lang#107975 The int is zero. But also not zero. This is so much fun. This is a part of rust-lang#105107. Initially I was going to just rebase rust-lang#108445, but quite a few things changed since then: * The [mcve](rust-lang#105787 (comment)) used for rust-lang#105787 got fixed.[^upd2] * You can't just `a ?= b` for rust-lang#107975 anymore. Now you have to `a-b ?= 0`. This is what this PR does. As an additional flex, it show that three ways of converting a pointer to its address have this issue: 1. `as usize` 2. `.expose_provenance()` 3. `.addr()` * rust-lang#108425 simply got fixed. Yay. As an aside, the naming for `addr_of!` is quite unfortunate in context of provenance APIs. Because `addr_of!` gives you a pointer, but what provenance APIs refer to as "address" is the `usize` value. Oh well. UPD1: GitHub is incapable of parsing rust-lang#107975 in the PR name, so let's add it here. [^upd2]: UPD2: [The other mcve](rust-lang#105787 (comment)) does not work anymore either, saying "this behavior recently changed as a result of a bug fix; see rust-lang#56105 for details."
Add a test for rust-lang#107975 The int is zero. But also not zero. This is so much fun. This is a part of rust-lang#105107. Initially I was going to just rebase rust-lang#108445, but quite a few things changed since then: * The [mcve](rust-lang#105787 (comment)) used for rust-lang#105787 got fixed.[^upd2] * You can't just `a ?= b` for rust-lang#107975 anymore. Now you have to `a-b ?= 0`. This is what this PR does. As an additional flex, it show that three ways of converting a pointer to its address have this issue: 1. `as usize` 2. `.expose_provenance()` 3. `.addr()` * rust-lang#108425 simply got fixed. Yay. As an aside, the naming for `addr_of!` is quite unfortunate in context of provenance APIs. Because `addr_of!` gives you a pointer, but what provenance APIs refer to as "address" is the `usize` value. Oh well. UPD1: GitHub is incapable of parsing rust-lang#107975 in the PR name, so let's add it here. [^upd2]: UPD2: [The other mcve](rust-lang#105787 (comment)) does not work anymore either, saying "this behavior recently changed as a result of a bug fix; see rust-lang#56105 for details."
Add a test for rust-lang#107975 The int is zero. But also not zero. This is so much fun. This is a part of rust-lang#105107. Initially I was going to just rebase rust-lang#108445, but quite a few things changed since then: * The [mcve](rust-lang#105787 (comment)) used for rust-lang#105787 got fixed.[^upd2] * You can't just `a ?= b` for rust-lang#107975 anymore. Now you have to `a-b ?= 0`. This is what this PR does. As an additional flex, it show that three ways of converting a pointer to its address have this issue: 1. `as usize` 2. `.expose_provenance()` 3. `.addr()` * rust-lang#108425 simply got fixed. Yay. As an aside, the naming for `addr_of!` is quite unfortunate in context of provenance APIs. Because `addr_of!` gives you a pointer, but what provenance APIs refer to as "address" is the `usize` value. Oh well. UPD1: GitHub is incapable of parsing rust-lang#107975 in the PR name, so let's add it here. [^upd2]: UPD2: [The other mcve](rust-lang#105787 (comment)) does not work anymore either, saying "this behavior recently changed as a result of a bug fix; see rust-lang#56105 for details."
I'm going to go ahead and close this. While there are still many issues (75 as of writing this!) that don't have a known-bug test, I don't expect keeping this issue open to meaningful make progress on reducing that number. The remaining relevant issues are mixed in the reasoning for why they don't have a known-bug test, but it's most often because either 1) there is no MCVE because the unsoundness is either "theoretical" or just hard to test, or 2) the unsoundness is only for some select targets. Thanks all who've helped make progress on this though! |
I'll come back at some point and create an actual list. However, we should aim to add
known-bug
tests (or work to finding MCVEs) for all the I-unsound issues.The list of issues remaining: https://github.com/rust-lang/rust/issues?q=is%3Aopen+is%3Aissue+label%3AI-unsound+-label%3AS-bug-has-test+
The list of completed issues: https://github.com/rust-lang/rust/issues?q=is%3Aopen+is%3Aissue+label%3AI-unsound+label%3AS-bug-has-test+
The text was updated successfully, but these errors were encountered: