Skip to content

Conversation

@Zoxc
Copy link
Contributor

@Zoxc Zoxc commented Apr 24, 2018

This fixes a bug where NopLogger in static mut LOGGER: &'static Log = &NopLogger will be marked mutable by mark_static_initialized. With this bugfix, Allocation.runtime_mutability is entirely unused and is removed.

r? @oli-obk

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 24, 2018
@oli-obk
Copy link
Contributor

oli-obk commented Apr 24, 2018

Won't this break static mut FOO: &mut u32 = &mut 42;?

We also need the flag for #49933 though it might be possible to get that flag back via the machine.

fn mark_static_initialized<'a>(
_mem: &mut Memory<'a, 'mir, 'tcx, Self>,
_id: AllocId,
_mutability: Mutability,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This argument needs to stay for miri the tool

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does the tool use it for?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Knowing whether the memory can be changed. The tool actually mutates statics.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The tool shouldn't use mark_static_initialized at all though, since that only applies to compile time execution?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we need to do this because otherwise we deallocate the memory of the 42 in static FOO: &u32 = &42; after the computation is done.

@rust-highfive
Copy link
Contributor

The job x86_64-gnu-llvm-3.9 of your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.
[00:44:44] [00:44:44] running 2965 tests [00:44:59] .................................................................................................... [00:45:14] .............................................i...................................................... [00:45:31] .................................................................F.................................. [00:46:00] .................................................................................................... [00:46:21] .................................................................................................... [00:46:36] .................................................................................................... [00:46:51] .................................................................................................... --- [00:55:14] [00:55:14] thread 'main' panicked at 'Some tests failed', tools/compiletest/src/main.rs:488:22 [00:55:14] [00:55:14] [00:55:14] command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-tools-bin/compiletest" "--compile-lib-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib" "--run-lib-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib/rustlib/x86_64-unknown-linux-gnu/lib" "--rustc-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "--src-base" "/checkout/src/test/run-pass" "--build-base" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/run-pass" "--stage-id" "stage2-x86_64-unknown-linux-gnu" "--mode" "run-pass" "--target" "x86_64-unknown-linux-gnu" "--host" "x86_64-unknown-linux-gnu" "--llvm-filecheck" "/usr/lib/llvm-3.9/bin/FileCheck" "--host-rustcflags" "-Crpath -O -Zunstable-options " "--target-rustcflags" "-Crpath -O -Zunstable-options -Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "--docck-python" "/usr/bin/python2.7" "--lldb-python" "/usr/bin/python2.7" "--gdb" "/usr/bin/gdb" "--quiet" "--llvm-version" "3.9.1\n" "--system-llvm" "--cc" "" "--cxx" "" "--cflags" "" "--llvm-components" "" "--llvm-cxxflags" "" "--adb-path" "adb" "--adb-test-dir" "/data/tmp/work" "--android-cross-path" "" "--color" "always" [00:55:14] [00:55:14] [00:55:14] failed 6_64-unknown-linux-gnu/release/incremental 111128 ./obj/build/x86_64-unknown-linux-gnu/stage0-tools/x86_64-unknown-linux-gnu 111128 ./obj/build/x86_64-unknown-linux-gnu/stage0-tools/x86_64-unknown-linux-gnu 111124 ./obj/build/x86_64-unknown-linux-gnu/stage0-tools/x86_64-unknown-linux-gnu/release 107240 ./obj/build/x86_64-unknown-linux-gnu/stage0-tools/x86_64-unknown-linux-gnu/release/deps 102808 ./obj/build/x86_64-unknown-linux-gnu/stage0/lib/rustlib/x86_64-unknown-linux-gnu/codegen-backends 101924 ./obj/build/bootstrap/debug/incremental/bootstrap-2s8ik7x786gic 101920 ./obj/build/bootstrap/debug/incremental/bootstrap-2s8ik7x786gic/s-f0enuo18a0-1d1jek5-8chh8umqc096 89216 ./obj/build/x86_64-unknown-linux-gnu/stage1 89192 ./obj/build/x86_64-unknown-linux-gnu/stage1/lib 88056 ./obj/build/x86_64-unknown-linux-gnu/stage1-std/x86_64-unknown-linux-gnu/release/incremental/core-31lccp6wy7orz 88056 ./obj/build/x86_64-unknown-linux-gnu/stage1-std/x86_64-unknown-linux-gnu/release/incremental/core-31lccp6wy7orz 88052 ./obj/build/x86_64-unknown-linux-gnu/stage1-std/x86_64-unknown-linux-gnu/release/incremental/core-31lccp6wy7orz/s-f0ens9tbq7-p0tlls-3by8jin3pp2v3 84832 ./obj/build/x86_64-unknown-linux-gnu/stage1-rustc/x86_64-unknown-linux-gnu/release/deps 81104 ./obj/build/x86_64-unknown-linux-gnu/doc/std 78700 ./obj/build/x86_64-unknown-linux-gnu/stage0-sysroot 78696 ./obj/build/x86_64-unknown-linux-gnu/stage0-sysroot/lib 

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

@Zoxc
Copy link
Contributor Author

Zoxc commented Apr 24, 2018

Won't this break static mut FOO: &mut u32 = &mut 42;?

That is illegal.

Edit: but static mut TEST: &'static mut [isize] = &mut [1]; is not....

@oli-obk
Copy link
Contributor

oli-obk commented Apr 24, 2018

Immutable references to unsafecell also need to be marked I think

@Zoxc
Copy link
Contributor Author

Zoxc commented Apr 24, 2018

Immutable references to unsafecell also need to be marked I think

Those give: error[E0492]: cannot borrow a constant which may contain interior mutability, create a static instead

@oli-obk
Copy link
Contributor

oli-obk commented Apr 24, 2018

Isn't that just a limitation of the current system?

static mut FOO: &UnsafeCell<i32> = &UnsafeCell::new(42);

should be fine in my mental model of statics

@eddyb
Copy link
Member

eddyb commented Apr 24, 2018

@oli-obk Those should be allowed, yes, but only for "escaping" references (i.e. no StorageLive on the underlying local being borrowed), and properly checked for Sync.
And we need to make sure we don't accidentally allow too much in const.

@Zoxc Zoxc closed this Apr 26, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.

4 participants