Set mmapped files as readonly to prevent other processes from modifying it by accident

[oli-obk]

Unfortunately this only is a guarantee on windows.
Double-unfortunately I don't know what's gonna happen when rustc segfaults on windows and a file is locked. Possibly the file is now deadlocked.

Linux just locks file as a hint, so only tools that know about file locking will actually respect it, others will just access it.

I guess if we had some sort of story for volatile memory we could use that as we're only reading bytes out of it and not actually mapping data structures to that memory.

[oli-obk]

@bors try @rust-timer queue

[rustbot]

r? @fee1-dead

rustbot has assigned @fee1-dead.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

[rust-timer]

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

[bors]

⌛ Trying commit 57ff16e with merge 393b4c0...

[ChrisDenton]

On Windows:

If a process terminates with a portion of a file locked or closes a file that has outstanding locks, the locks are unlocked by the operating system. However, the time it takes for the operating system to unlock these locks depends upon available system resources. Therefore, it is recommended that your process explicitly unlock all files it has locked when it terminates. If this is not done, access to these files may be denied if the operating system has not yet unlocked them.

Though I would also add:

Locking a region of a file does not prevent reading or writing from a mapped file view.

[bors]

💔 Test failed - checks-actions

[rustbot]

Some changes occurred in compiler/rustc_codegen_gcc

cc @antoyo, @GuillaumeGomez

[oli-obk]

@bors try @rust-timer queue

[rust-timer]

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

[bors]

⌛ Trying commit 0db5406 with merge 91c5854...

[bors]

💔 Test failed - checks-actions

[the8472]

The files we mmap are write-once. So couldn't we set them to readonly after creating them? That should be at least as good as locking since making them writable again would require another program going out of its way to bypass this, which isn't the case with advisory locks.

[bors]

☔ The latest upstream changes (presumably #137046) made this pull request unmergeable. Please resolve the merge conflicts.

[rustbot]

Some changes occurred in compiler/rustc_codegen_ssa

cc @WaffleLapkin

[rust-log-analyzer]

The job x86_64-gnu-llvm-18 failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

#19 exporting to docker image format
#19 sending tarball 20.1s done
#19 DONE 23.9s
##[endgroup]
Setting extra environment values for docker:  --env ENABLE_GCC_CODEGEN=1 --env GCC_EXEC_PREFIX=/usr/lib/gcc/
[CI_JOB_NAME=x86_64-gnu-llvm-18]
[CI_JOB_NAME=x86_64-gnu-llvm-18]
debug: `DISABLE_CI_RUSTC_IF_INCOMPATIBLE` configured.
---
sccache: Listening on address 127.0.0.1:4226
##[group]Configure the build
configure: processing command line
configure: 
configure: build.configure-args := ['--build=x86_64-unknown-linux-gnu', '--llvm-root=/usr/lib/llvm-18', '--enable-llvm-link-shared', '--set', 'rust.randomize-layout=true', '--set', 'rust.thin-lto-import-instr-limit=10', '--enable-verbose-configure', '--enable-sccache', '--disable-manage-submodules', '--enable-locked-deps', '--enable-cargo-native-static', '--set', 'rust.codegen-units-std=1', '--set', 'dist.compression-profile=balanced', '--dist-compression-formats=xz', '--set', 'rust.lld=false', '--disable-dist-src', '--release-channel=nightly', '--enable-debug-assertions', '--enable-overflow-checks', '--enable-llvm-assertions', '--set', 'rust.verify-llvm-ir', '--set', 'rust.codegen-backends=llvm,cranelift,gcc', '--set', 'llvm.static-libstdcpp', '--enable-new-symbol-mangling']
configure: build.build          := x86_64-unknown-linux-gnu
configure: target.x86_64-unknown-linux-gnu.llvm-config := /usr/lib/llvm-18/bin/llvm-config
configure: llvm.link-shared     := True
configure: rust.randomize-layout := True
configure: rust.thin-lto-import-instr-limit := 10
---
   Compiling rustc_builtin_macros v0.0.0 (/checkout/compiler/rustc_builtin_macros)
error: unnecessary `unsafe` block
    --> compiler/rustc_metadata/src/rmeta/encoder.rs:2251:20
     |
2251 |         let mmap = unsafe { Some(Mmap::map(path)?) };
     |                    ^^^^^^ unnecessary `unsafe` block
     |
     = note: `-D unused-unsafe` implied by `-D warnings`
     = help: to override `-D warnings` add `#[allow(unused_unsafe)]`

error: unnecessary `unsafe` block
   --> compiler/rustc_metadata/src/locator.rs:825:24
    |
825 |             let mmap = unsafe { Mmap::map(filename) };
    |                        ^^^^^^ unnecessary `unsafe` block

error: could not compile `rustc_metadata` (lib) due to 2 previous errors
warning: build failed, waiting for other jobs to finish...
Build completed unsuccessfully in 0:02:15

[the8472]

Why would removing the file be a problem? Mappings should say valid for unlinked files... unless network filesystems are involved I guess...

[the8472]

can't we immediately set it to readonly after initial creation? The writing process can have an FD with write permission and then make the file readonly that other processes can't open it for writing.

[oli-obk]

Hmm. That disconnects the read only setting from the mmap. I guess I could check I'd it's already read only

[klensy]

Btw, memmap2 currently used is super outdated (0.2.3), https://github.com/RazrFalcon/memmap2-rs/blob/v0.9.5/CHANGELOG.md, maybe bump it? Need to review changelog, as there was a lot of changes.

[the8472]

I think the linux CI containers run as root, so we probably should drop CAP_DAC_OVERRIDE to make sure that the permissions don't get bypassed.
OTOH macos and windows builds aren't affected by this so they would catch writes that shouldn't happen.