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

Add test which attempts to modify static memory at compile-time #53818

Closed
oli-obk opened this issue Aug 30, 2018 · 6 comments · Fixed by #54147
Closed

Add test which attempts to modify static memory at compile-time #53818

oli-obk opened this issue Aug 30, 2018 · 6 comments · Fixed by #54147
Labels
A-const-eval Area: Constant evaluation (MIR interpretation) E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-needs-test Call for participation: An issue has been fixed and does not reproduce, but no test has been added.

Comments

@oli-obk
Copy link
Contributor

oli-obk commented Aug 30, 2018

No matter what kind of const eval features we add, we should never allow this. Even if we managed to allow this, I think the current impl would ICE. Even if it does not ICE, it could never actually modify the memory. We should ensure that the following code always errors.

#![feature(const_raw_ptr_deref)]
#![feature(const_let)]

use std::cell::UnsafeCell;

struct Foo(UnsafeCell<u32>);

unsafe impl Send for Foo {}
unsafe impl Sync for Foo {}

static FOO: Foo = Foo(UnsafeCell::new(42));

static BAR: () = unsafe {
    *FOO.0.get() = 5;
};

fn main() {
    println!("{}", unsafe { *FOO.0.get() });
}
@oli-obk oli-obk added E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-needs-test Call for participation: An issue has been fixed and does not reproduce, but no test has been added. A-const-eval Area: Constant evaluation (MIR interpretation) labels Aug 30, 2018
@agnxy
Copy link
Contributor

agnxy commented Aug 30, 2018

Hi @oli-obk , I would like to work on this if that's ok. I'm a new Rustacean and I'm learning rustc development and testing. It's time to get my feet wet 😃

@oli-obk
Copy link
Contributor Author

oli-obk commented Aug 30, 2018

Great!

I think the following steps should suffice, but don't hesitate to ask if anything is unclear or doesn't work as expected

  1. add the test in a new file in src/test/ui/const/const-eval
  2. add a //~ ERROR error message here annotation to the line that should report an error (replace "error message here" with the relevant error message)
  3. run ./x.py test --stage 1 src/test/ui --test-args const-eval --bless
  4. repeat 3 until everything is green
  5. commit your test and the generated .stderr file and open a PR
  6. add fixes #53818 to the PR message so when it gets merged this issue gets closed automatically

@agnxy
Copy link
Contributor

agnxy commented Sep 8, 2018

Hi @oli-obk ,
When I was running the test in step 3, or compiled the new test with nightly rustc (fc81e3624), I got errors like this.

error[E0015]: calls in statics are limited to constant functions, tuple structs and tuple variants
--> mod-static-with-const-fn.rs:24:6
|
24 | *FOO.0.get() = 5;
| ^^^^^^^^^^^

error[E0658]: statements in statics are unstable (see issue #48821)
--> mod-static-with-const-fn.rs:24:5
|
24 | *FOO.0.get() = 5;
| ^^^^^^^^^^^^^^^^
|
= help: add #![feature(const_let)] to the crate attributes to enable

error: aborting due to 2 previous errors

I think E0015 is the expected error. But I'm not sure about E0658. It seems that the const_let feature is not stabilized yet. (But compiling const_let.rs doesn't return this error.) Could you please help with this problem? Thanks a lot.

@oli-obk
Copy link
Contributor Author

oli-obk commented Sep 8, 2018

These errors are fine. the idea of this issue is to just make sure the code always fails to compile. More errors are fine, too. the const let feature has a bug where even enabling it will still report it. that's not problematic for this issue. Can you add a comment to the test mentioning that this test should never ever succeed?

If you want you can additionally try to minimize the test to something still emitting the feature gate error even though the feature gate is active (and maybe to something that should be perfectly fine at compile time). then open a new issue about that.

@agnxy
Copy link
Contributor

agnxy commented Sep 10, 2018

Thanks @oli-obk ! I will try to reproduce the unexpected error related to the feature gate.

Meanwhile it seems that this task will be blocked by that issue since the ui test will fail if the error signature doesn't match with the expected one. Is that right?

@oli-obk
Copy link
Contributor Author

oli-obk commented Sep 10, 2018

you can just make them match ;) Add all the annotations you need to make the test pass. Whenever someone changes your test, because they fixed other bugs or implemented new features, they'll hopefully look at the comment about the fact that the test should never succeed to compile.

kennytm added a commit to kennytm/rust that referenced this issue Sep 13, 2018
Add a test that tries to modify static memory at compile-time

Attempt to fix rust-lang#53818
cc @oli-obk
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-const-eval Area: Constant evaluation (MIR interpretation) E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-needs-test Call for participation: An issue has been fixed and does not reproduce, but no test has been added.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants