Prepend temp files with per-invocation random string to avoid temp filename conflicts #139453

compiler-errors · 2025-04-06T19:02:51Z

#139407 uncovered a very subtle unsoundness with incremental codegen, failing compilation sessions (due to assembler errors), and the "prefer hard linking over copying files" strategy we use in the compiler for file management.

Specifically, imagine we're building a single file 3 times, all with -Csave-temps -Cincremental=.... Let's call the object file we're building for the codegen unit for main "XXX.o" just for clarity since it's probably some gigantic hash name:

#[inline(never)]
#[cfg(any(rpass1, rpass3))]
fn a() -> i32 {
    0
}

#[cfg(any(cfail2))]
fn a() -> i32 {
    1
}

fn main() {
    evil::evil();
    assert_eq!(a(), 0);
}

mod evil {
    #[cfg(any(rpass1, rpass3))]
    pub fn evil() {
        unsafe {
            std::arch::asm!("/*  */");
        }
    }

    #[cfg(any(cfail2))]
    pub fn evil() {
        unsafe {
            std::arch::asm!("missing");
        }
    }
}

Session 1 (rpass1):

Type-check, borrow-check, etc.
Serialize the dep graph to the incremental working directory .../s-...-working/.
Codegen object file to a temp file XXX.rcgu.o which is spit out in the cwd.
Hard-link¹ XXX.rcgu.o to the incremental working directory .../s-...-working/XXX.o.
Save-temps option means we don't delete XXX.rgcu.o.
Link the binary and stuff.
Finalize² the working incremental session by renaming .../s-...-working to s-...-asjkdhsjakd (some other finalized incr comp session dir name).

Session 2 (cfail2):

Load artifacts from the previous finalized incremental session, namely the dep graph.
Type-check, borrow-check, etc. since the file has changed, so most dep graph nodes are red.
Serialize the dep graph to the incremental working directory .../s-...-working/.
Codegen object file to a temp file XXX.rcgu.o. HERE IS THE PROBLEM: The hard-link is still set up to point to the inode from XXX.o from the first session, so this also modifies the XXX.o in the previous finalized session directory.
Codegen emits an error b/c missing is not an instruction, so we abort before finalizing the incremental session. Specifically, this means that the previous session is the last finalized session.

Session 3 (rpass3):

Load artifacts from the previous finalized incremental session, namely the dep graph. NOTE that this is from session 1.
All the dep graph nodes are green since we are basically replaying session 1.
codegen object file XXX.o, which is detected as reused from session 1 since dep nodes were green. That means we reuse XXX.o which had been dirtied from session 2.
Link the binary and stuff.

This results in a binary which reuses some of the build artifacts from session 2, but thinks it's from session 1.

At this point, I hope it's clear to see that the incremental results from session 1 were dirtied from session 2, but we reuse them as if session 1 was the previous (finalized) incremental session we ran. This is at best really buggy, and at worst unsound.

This isn't limited to -C save-temps, since there are other combinations of flags that may keep around temporary files (hard linked) in the working directory (like -C debuginfo=1 -C split-debuginfo=unpacked on darwin, for example).

This PR implements a fix which is to prepend temp filenames with a random string that is generated per invocation of rustc. This string is not deterministic, but temporary files are transient anyways, so I don't believe this is a problem.

That means that temp files are now something like... {crate-name}.{cgu}.{invocation_temp}.rcgu.o, where {invocation_temp} is the new temporary string we generate per invocation of rustc.

Fixes #139407

rustbot · 2025-04-06T19:02:56Z

r? @jieyouxu

rustbot has assigned @jieyouxu.
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

rustbot · 2025-04-06T19:02:58Z

Some changes occurred in compiler/rustc_codegen_ssa

cc @WaffleLapkin

Some changes occurred in compiler/rustc_codegen_gcc

cc @antoyo, @GuillaumeGomez

This PR modifies run-make tests.

cc @jieyouxu

tests/run-make/dirty-incr-due-to-hard-link/rmake.rs

bjorn3 · 2025-04-06T19:49:30Z

cg_clif will also need changes. In any case would it make sense to codegen to a file with a randomly generated name using eg the tempfile crate rather using a fixed filename? The existing fixed filename can still be used within the incr comp cache and as name for the archive members. This would also prevent issues when two rustc instances with the same crate name and -Cmetadata are running inside the same directory if they use a different output filename (and if they use the same output filename, one of them would win rather than non-deterministically mixing object files from the two). See also #111083

compiler-errors · 2025-04-06T20:30:58Z

cg_clif will also need changes

Yeah, the cc was not really me asking whether cg_clif needed changes but asking where we should be making those changes, since I don't know how temp files get generated in clif 😆

We already have a fixed name for the file in the incr comp cache, so yeah, I think(?) we can just modify the way we generate temporary file names¹ to append the per-session temporary name? Like XXX.session-hash.rcgu.o? 🤔

Though I'm not sure if anyone relies on the filename of these temporaries today; that seems like a more dramatic change given that people are probably doing... funny things with the outputs of -Csave-temps 😓

https://github.com/rust-lang/rust/blob/fd4dc18e684ffb8c000b053e4d34f412ee1d586a/compiler/rustc_session/src/config.rs#L1097 ↩

bjorn3 · 2025-04-06T20:47:18Z

Yeah, the cc was not really me asking whether cg_clif needed changes but asking where we should be making those changes, since I don't know how temp files get generated in clif

All this code is in src/driver/aot.rs.

Though I'm not sure if anyone relies on the filename of these temporaries today

People may depend on the rough format of the filename, but can't depend on the exact filename as it is rustc version dependent.

the8472 · 2025-04-06T21:45:19Z

This seems a bit scattered. I think our usage of hard-links in the compiler is really sketchy and feels like it's somewhat brittle; not using hard links would also fix the problem.

Ideally all our cachable outputs would be created read-only to prevent this kind of accidental modification since they're semantically write-once anyway. Similar to #137025

rustbot · 2025-04-06T22:52:18Z

Some changes occurred in compiler/rustc_codegen_cranelift

cc @bjorn3

compiler-errors · 2025-04-06T22:52:48Z

Anyways, I haven't updated this to a new strategy for encoding temporary files. I'll probably evaluate that in a bit and if it's too hard/annoying then I'd prefer we just land this approach to patch the unsoundness.

rustbot · 2025-04-06T23:53:26Z

These commits modify the Cargo.lock file. Unintentional changes to Cargo.lock can be introduced when switching branches and rebasing PRs.

If this was unintentional then you should revert the changes before this PR is merged.
Otherwise, you can ignore this comment.

compiler-errors · 2025-04-06T23:53:48Z

OK, I updated the description to implement a new strategy of just augmenting these temp filenames with a per-invocation temporary string.

I think it's a lot more straightforward than unlinking. I opted to just use rand to generate the string, since that's what we use for generating the session directory names too.

compiler-errors · 2025-04-07T00:36:19Z

CGU reuse 💀

jieyouxu

Thanks for the fix! The approach and changes look good to me, only left a few remarks.

jieyouxu · 2025-04-07T04:05:56Z

compiler/rustc_session/src/config.rs

+        // FIXME: This is sketchy that we're not appending `.rcgu` when the ext is empty.
+        // Append `.rcgu.{ext}`.
        if !ext.is_empty() {
-            if !extension.is_empty() {
-                extension.push('.');
-                extension.push_str(RUST_CGU_EXT);
-                extension.push('.');
-            }
-
+            extension.push('.');
+            extension.push_str(RUST_CGU_EXT);
+            extension.push('.');
            extension.push_str(ext);
        }


Remark: okay so this bit also seemed a bit sus to me, git archaeology points way back to

65e8a13 as part of Drive trans from the output of the translation item collector #33890

and I can't seem to find any specific reason to skip appending .rcgu when the ext is empty, except for the diagnostic long-type-hash file. So instead, I just looked at the diff:

- self.output_filenames(()).temp_path_ext(&format!("long-type-{hash}.txt"), None) + self.output_filenames(()).temp_path_for_diagnostic(&format!("long-type-{hash}.txt"))

All other call sites AFAICT explicitly constructs a CGU name and passes Some(..), so I believe it was correct but just hard to follow -- in that it used the CGU name being present or not to encode diagnostics file vs non-diagnostics file...

The cleanup here is indeed very nice because we no longer rely on that Option<CguName> to splice what temp_path_ext does.

The cleanup here is indeed very nice because we no longer rely on that Option to splice what temp_path_ext does.

Yeah, that's why I did all that splitting first, since there seemed to be totally distinct responsibilities for temp_path -- for cgu files and for "other" files.

tests/run-make/dirty-incr-due-to-hard-link/rmake.rs

jieyouxu · 2025-04-07T04:29:13Z

Also, damn, that's a nasty problem

compiler-errors · 2025-04-07T21:45:20Z

@bors r=jieyouxu rollup=never

bors · 2025-04-07T21:45:23Z

📌 Commit 9c372d8 has been approved by jieyouxu

It is now in the queue for this repository.

Prepend temp files with per-invocation random string to avoid temp filename conflicts rust-lang#139407 uncovered a very subtle unsoundness with incremental codegen, failing compilation sessions (due to assembler errors), and the "prefer hard linking over copying files" strategy we use in the compiler for file management. Specifically, imagine we're building a single file 3 times, all with `-Csave-temps -Cincremental=...`. Let's call the object file we're building for the codegen unit for `main` "`XXX.o`" just for clarity since it's probably some gigantic hash name: ``` #[inline(never)] #[cfg(any(rpass1, rpass3))] fn a() -> i32 { 0 } #[cfg(any(cfail2))] fn a() -> i32 { 1 } fn main() { evil::evil(); assert_eq!(a(), 0); } mod evil { #[cfg(any(rpass1, rpass3))] pub fn evil() { unsafe { std::arch::asm!("/* */"); } } #[cfg(any(cfail2))] pub fn evil() { unsafe { std::arch::asm!("missing"); } } } ``` Session 1 (`rpass1`): * Type-check, borrow-check, etc. * Serialize the dep graph to the incremental working directory `.../s-...-working/`. * Codegen object file to a temp file `XXX.rcgu.o` which is spit out in the cwd. * Hard-link[^1] `XXX.rcgu.o` to the incremental working directory `.../s-...-working/XXX.o`. * Save-temps option means we don't delete `XXX.rgcu.o`. * Link the binary and stuff. * Finalize[^2] the working incremental session by renaming `.../s-...-working` to ` s-...-asjkdhsjakd` (some other finalized incr comp session dir name). Session 2 (`cfail2`): * Load artifacts from the previous *finalized* incremental session, namely the dep graph. * Type-check, borrow-check, etc. since the file has changed, so most dep graph nodes are red. * Serialize the dep graph to the incremental working directory `.../s-...-working/`. * Codegen object file to a temp file `XXX.rcgu.o`. **HERE IS THE PROBLEM**: The hard-link is still set up to point to the inode from `XXX.o` from the first session, so this also modifies the `XXX.o` in the previous finalized session directory. * Codegen emits an error b/c `missing` is not an instruction, so we abort before finalizing the incremental session. Specifically, this means that the *previous* session is the last finalized session. Session 3 (`rpass3`): * Load artifacts from the previous *finalized* incremental session, namely the dep graph. NOTE that this is from session 1. * All the dep graph nodes are green since we are basically replaying session 1. * codegen object file `XXX.o`, which is detected as *reused* from session 1 since dep nodes were green. That means we **reuse** `XXX.o` which had been dirtied from session 2. * Link the binary and stuff. This results in a binary which reuses some of the build artifacts from session 2, but thinks it's from session 1. At this point, I hope it's clear to see that the incremental results from session 1 were dirtied from session 2, but we reuse them as if session 1 was the previous (finalized) incremental session we ran. This is at best really buggy, and at worst **unsound**. This isn't limited to `-C save-temps`, since there are other combinations of flags that may keep around temporary files (hard linked) in the working directory (like `-C debuginfo=1 -C split-debuginfo=unpacked` on darwin, for example). --- This PR implements a fix which is to prepend temp filenames with a random string that is generated per invocation of rustc. This string is not *deterministic*, but temporary files are transient anyways, so I don't believe this is a problem. That means that temp files are now something like... `{crate-name}.{cgu}.{invocation_temp}.rcgu.o`, where `{invocation_temp}` is the new temporary string we generate per invocation of rustc. Fixes rust-lang#139407 [^1]: https://github.com/rust-lang/rust/blob/175dcc7773d65c1b1542c351392080f48c05799f/compiler/rustc_fs_util/src/lib.rs#L60 [^2]: https://github.com/rust-lang/rust/blob/175dcc7773d65c1b1542c351392080f48c05799f/compiler/rustc_incremental/src/persist/fs.rs#L1-L40

bors · 2025-04-11T02:37:19Z

⌛ Testing commit 9c372d8 with merge f9dd500...

rust-log-analyzer · 2025-04-11T04:23:00Z

The job x86_64-mingw-1 failed! Check out the build log: (web) (plain)

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

failures:

---- [assembly] tests\assembly\targets\targets-elf.rs#armv7_unknown_linux_musleabihf stdout ----

error in revision `armv7_unknown_linux_musleabihf`: auxiliary build of "D:\\a\\rust\\rust\\tests\\auxiliary\\minicore.rs" failed to compile: 
status: exit code: 1
command: PATH="D:\a\rust\rust\build\x86_64-pc-windows-gnu\stage2\bin;D:\a\rust\rust\build\x86_64-pc-windows-gnu\stage0-bootstrap-tools\x86_64-pc-windows-gnu\release\deps;D:\a\rust\rust\build\x86_64-pc-windows-gnu\stage0\bin;D:\a\rust\rust\ninja;D:\a\rust\rust\mingw64\bin;C:\msys64\usr\bin;D:\a\rust\rust\sccache;C:\Program Files\MongoDB\Server\5.0\bin;C:\aliyun-cli;C:\vcpkg;C:\Program Files (x86)\NSIS;C:\tools\zstd;C:\Program Files\Mercurial;C:\hostedtoolcache\windows\stack\3.5.1\x64;C:\cabal\bin;C:\ghcup\bin;C:\mingw64\bin;C:\Program Files\dotnet;C:\Program Files\MySQL\MySQL Server 8.0\bin;C:\Program Files\R\R-4.4.2\bin\x64;C:\SeleniumWebDrivers\GeckoDriver;C:\SeleniumWebDrivers\EdgeDriver;C:\SeleniumWebDrivers\ChromeDriver;C:\Program Files (x86)\sbt\bin;C:\Program Files (x86)\GitHub CLI;C:\Program Files\Git\bin;C:\Program Files (x86)\pipx_bin;C:\npm\prefix;C:\hostedtoolcache\windows\go\1.21.13\x64\bin;C:\hostedtoolcache\windows\Python\3.9.13\x64\Scripts;C:\hostedtoolcache\windows\Python\3.9.13\x64;C:\hostedtoolcache\windows\Ruby\3.0.7\x64\bin;C:\Program Files\OpenSSL\bin;C:\tools\kotlinc\bin;C:\hostedtoolcache\windows\Java_Temurin-Hotspot_jdk\8.0.442-6\x64\bin;C:\Program Files\ImageMagick-7.1.1-Q16-HDRI;C:\Program Files\Microsoft SDKs\Azure\CLI2\wbin;C:\ProgramData\kind;C:\ProgramData\Chocolatey\bin;C:\Windows\system32;C:\Windows;C:\Windows\System32\Wbem;C:\Windows\System32\WindowsPowerShell\v1.0;C:\Windows\System32\OpenSSH;C:\Program Files\dotnet;C:\Program Files\PowerShell\7;C:\Program Files\Microsoft\Web Platform Installer;C:\Program Files\TortoiseSVN\bin;C:\Program Files\Microsoft SQL Server\Client SDK\ODBC\170\Tools\Binn;C:\Program Files\Microsoft SQL Server\150\Tools\Binn;C:\Program Files (x86)\Windows Kits\10\Windows Performance Toolkit;C:\Program Files (x86)\WiX Toolset v3.14\bin;C:\Program Files\Microsoft SQL Server\130\DTS\Binn;C:\Program Files\Microsoft SQL Server\140\DTS\Binn;C:\Program Files\Microsoft SQL Server\150\DTS\Binn;C:\Program Files\Microsoft SQL Server\160\DTS\Binn;C:\Strawberry\c\bin;C:\Strawberry\perl\site\bin;C:\Strawberry\perl\bin;C:\ProgramData\chocolatey\lib\pulumi\tools\Pulumi\bin;C:\Program Files\CMake\bin;C:\ProgramData\chocolatey\lib\maven\apache-maven-3.9.9\bin;C:\Program Files\Microsoft Service Fabric\bin\Fabric\Fabric.Code;C:\Program Files\Microsoft SDKs\Service Fabric\Tools\ServiceFabricLocalClusterManager;C:\Program Files\nodejs;C:\Program Files\Git\cmd;C:\Program Files\Git\mingw64\bin;C:\Program Files\Git\usr\bin;C:\Program Files\GitHub CLI;C:\tools\php;C:\Program Files (x86)\sbt\bin;C:\Program Files\Amazon\AWSCLIV2;C:\Program Files\Amazon\SessionManagerPlugin\bin;C:\Program Files\Amazon\AWSSAMCLI\bin;C:\Program Files\Microsoft SQL Server\130\Tools\Binn;C:\Program Files\LLVM\bin;C:\Users\runneradmin\.dotnet\tools;C:\Users\runneradmin\.cargo\bin;C:\Users\runneradmin\AppData\Local\Microsoft\WindowsApps" "D:\\a\\rust\\rust\\build\\x86_64-pc-windows-gnu\\stage2\\bin\\rustc.exe" "D:\\a\\rust\\rust\\tests\\auxiliary\\minicore.rs" "-Zthreads=1" "-Zsimulate-remapped-rust-src-base=/rustc/FAKE_PREFIX" "-Ztranslate-remapped-path-to-local-path=no" "-Z" "ignore-directory-in-diagnostics-source-blocks=C:\\Users\\runneradmin\\.cargo" "-Z" "ignore-directory-in-diagnostics-source-blocks=D:\\a\\rust\\rust\\vendor" "--sysroot" "D:\\a\\rust\\rust\\build\\x86_64-pc-windows-gnu\\stage2" "--cfg" "armv7_unknown_linux_musleabihf" "--check-cfg" "cfg(test,FALSE,aarch64_be_unknown_linux_gnu,aarch64_be_unknown_linux_gnu_ilp32,aarch64_be_unknown_netbsd,aarch64_kmc_solid_asp3,aarch64_linux_android,aarch64_nintendo_switch_freestanding,aarch64_unknown_freebsd,aarch64_unknown_fuchsia,aarch64_unknown_hermit,aarch64_unknown_illumos,aarch64_unknown_linux_gnu,aarch64_unknown_linux_gnu_ilp32,aarch64_unknown_linux_musl,aarch64_unknown_linux_ohos,aarch64_unknown_netbsd,aarch64_unknown_none,aarch64_unknown_none_softfloat,aarch64_unknown_nto_qnx700,aarch64_unknown_nto_qnx710,aarch64_unknown_nto_qnx710_iosock,aarch64_unknown_nto_qnx800,aarch64_unknown_openbsd,aarch64_unknown_redox,aarch64_unknown_teeos,aarch64_unknown_nuttx,aarch64_unknown_trusty,aarch64_wrs_vxworks,arm_linux_androideabi,arm_unknown_linux_gnueabi,arm_unknown_linux_gnueabihf,arm_unknown_linux_musleabi,arm_unknown_linux_musleabihf,armeb_unknown_linux_gnueabi,armebv7r_none_eabi,armebv7r_none_eabihf,armv4t_none_eabi,armv4t_unknown_linux_gnueabi,armv5te_none_eabi,armv5te_unknown_linux_gnueabi,armv5te_unknown_linux_musleabi,armv5te_unknown_linux_uclibceabi,armv6_unknown_freebsd,armv6_unknown_netbsd_eabihf,armv6k_nintendo_3ds,armv7_linux_androideabi,armv7_rtems_eabihf,armv7_sony_vita_newlibeabihf,armv7_unknown_freebsd,armv7_unknown_linux_gnueabi,armv7_unknown_linux_gnueabihf,armv7_unknown_linux_musleabi,armv7_unknown_linux_musleabihf,armv7_unknown_linux_ohos,armv7_unknown_linux_uclibceabi,armv7_unknown_linux_uclibceabihf,armv7_unknown_netbsd_eabihf,armv7_unknown_trusty,armv7_wrs_vxworks_eabihf,armv7a_kmc_solid_asp3_eabi,armv7a_kmc_solid_asp3_eabihf,armv7a_none_eabi,armv7a_none_eabihf,armv7a_nuttx_eabi,armv7a_nuttx_eabihf,armv7r_none_eabi,armv7r_none_eabihf,armv8r_none_eabihf,hexagon_unknown_linux_musl,hexagon_unknown_none_elf,i686_pc_nto_qnx700,i586_unknown_linux_gnu,i586_unknown_linux_musl,i586_unknown_netbsd,i586_unknown_redox,i686_linux_android,i686_unknown_freebsd,i686_unknown_haiku,i686_unknown_hurd_gnu,i686_unknown_linux_gnu,i686_unknown_linux_musl,i686_unknown_netbsd,i686_unknown_openbsd,i686_wrs_vxworks,loongarch64_unknown_linux_gnu,loongarch64_unknown_linux_musl,loongarch64_unknown_linux_ohos,loongarch64_unknown_none,loongarch64_unknown_none_softfloat,m68k_unknown_linux_gnu,m68k_unknown_none_elf,mips64_openwrt_linux_musl,mips64_unknown_linux_gnuabi64,mips64_unknown_linux_muslabi64,mips64el_unknown_linux_gnuabi64,mips64el_unknown_linux_muslabi64,mips_unknown_linux_gnu,mips_unknown_linux_musl,mips_unknown_linux_uclibc,mips_mti_none_elf,mipsel_mti_none_elf,mipsel_sony_psp,mipsel_sony_psx,mipsel_unknown_linux_gnu,mipsel_unknown_linux_musl,mipsel_unknown_linux_uclibc,mipsel_unknown_netbsd,mipsel_unknown_none,mipsisa32r6_unknown_linux_gnu,mipsisa32r6el_unknown_linux_gnu,mipsisa64r6_unknown_linux_gnuabi64,mipsisa64r6el_unknown_linux_gnuabi64,msp430_none_elf,powerpc64_unknown_freebsd,powerpc64_unknown_linux_gnu,powerpc64_unknown_linux_musl,powerpc64_unknown_openbsd,powerpc64_wrs_vxworks,powerpc64le_unknown_freebsd,powerpc64le_unknown_linux_gnu,powerpc64le_unknown_linux_musl,powerpc_unknown_freebsd,powerpc_unknown_linux_gnu,powerpc_unknown_linux_gnuspe,powerpc_unknown_linux_musl,powerpc_unknown_linux_muslspe,powerpc_unknown_netbsd,powerpc_unknown_openbsd,powerpc_wrs_vxworks,powerpc_wrs_vxworks_spe,riscv32_wrs_vxworks,riscv32e_unknown_none_elf,riscv32em_unknown_none_elf,riscv32emc_unknown_none_elf,riscv32gc_unknown_linux_gnu,riscv32gc_unknown_linux_musl,riscv32i_unknown_none_elf,riscv32im_risc0_zkvm_elf,riscv32im_unknown_none_elf,riscv32ima_unknown_none_elf,riscv32imac_esp_espidf,riscv32imac_unknown_none_elf,riscv32imac_unknown_xous_elf,riscv32imafc_unknown_none_elf,riscv32imafc_esp_espidf,riscv32imc_esp_espidf,riscv32imc_unknown_none_elf,riscv64_linux_android,riscv64_wrs_vxworks,riscv64gc_unknown_freebsd,riscv64gc_unknown_fuchsia,riscv64gc_unknown_hermit,riscv64gc_unknown_linux_gnu,riscv64gc_unknown_linux_musl,riscv64gc_unknown_netbsd,riscv64gc_unknown_none_elf,riscv64gc_unknown_openbsd,riscv64imac_unknown_none_elf,s390x_unknown_linux_gnu,s390x_unknown_linux_musl,sparc64_unknown_linux_gnu,sparc64_unknown_netbsd,sparc64_unknown_openbsd,sparc_unknown_linux_gnu,sparc_unknown_none_elf,sparcv9_sun_solaris,thumbv4t_none_eabi,thumbv5te_none_eabi,thumbv6m_none_eabi,thumbv7em_none_eabi,thumbv7em_none_eabihf,thumbv7m_none_eabi,thumbv7neon_linux_androideabi,thumbv7neon_unknown_linux_gnueabihf,thumbv7neon_unknown_linux_musleabihf,thumbv8m_base_none_eabi,thumbv8m_main_none_eabi,thumbv8m_main_none_eabihf,wasm32_unknown_emscripten,wasm32_unknown_unknown,wasm32v1_none,wasm32_wasip1,wasm32_wasip1_threads,wasm32_wasip2,wasm32_wali_linux_musl,wasm64_unknown_unknown,x86_64_fortanix_unknown_sgx,x86_64_linux_android,x86_64_pc_nto_qnx710,x86_64_pc_nto_qnx710_iosock,x86_64_pc_nto_qnx800,x86_64_pc_solaris,x86_64_unikraft_linux_musl,x86_64_unknown_dragonfly,x86_64_unknown_freebsd,x86_64_unknown_fuchsia,x86_64_unknown_haiku,x86_64_unknown_hurd_gnu,x86_64_unknown_hermit,x86_64_unknown_illumos,x86_64_unknown_l4re_uclibc,x86_64_unknown_linux_gnu,x86_64_unknown_linux_gnux32,x86_64_unknown_linux_musl,x86_64_unknown_linux_ohos,x86_64_unknown_linux_none,x86_64_unknown_netbsd,x86_64_unknown_none,x86_64_unknown_openbsd,x86_64_unknown_redox,x86_64_unknown_trusty,x86_64_wrs_vxworks,thumbv6m_nuttx_eabi,thumbv7a_nuttx_eabi,thumbv7a_nuttx_eabihf,thumbv7m_nuttx_eabi,thumbv7em_nuttx_eabi,thumbv7em_nuttx_eabihf,thumbv8m_base_nuttx_eabi,thumbv8m_main_nuttx_eabi,thumbv8m_main_nuttx_eabihf,riscv32imc_unknown_nuttx_elf,riscv32imac_unknown_nuttx_elf,riscv32imafc_unknown_nuttx_elf,riscv64imac_unknown_nuttx_elf,riscv64gc_unknown_nuttx_elf)" "-O" "-Cdebug-assertions=no" "-C" "prefer-dynamic" "-o" "D:\\a\\rust\\rust\\build\\x86_64-pc-windows-gnu\\test\\assembly\\targets\\targets-elf.armv7_unknown_linux_musleabihf\\libminicore.rlib" "-A" "unused" "-A" "internal_features" "-Crpath" "-Cdebuginfo=0" "--target" "armv7-unknown-linux-musleabihf" "-Cpanic=abort" "--crate-type" "rlib" "-Cpanic=abort"
stdout: none
--- stderr -------------------------------
error: couldn't create a temp dir: Access is denied. (os error 5) at path "C:\\Users\\RUNNER~1\\AppData\\Local\\Temp\\rustcb7WM0W"

error: aborting due to 1 previous error
------------------------------------------


---
test result: FAILED. 494 passed; 1 failed; 43 ignored; 0 measured; 0 filtered out; finished in 25.62s

Some tests failed in compiletest suite=assembly mode=assembly host=x86_64-pc-windows-gnu target=x86_64-pc-windows-gnu
Build completed unsuccessfully in 1:36:31
make: *** [Makefile:124: ci-mingw-x] Error 1
  local time: Fri Apr 11 04:22:46 CUT 2025
  network time: Fri, 11 Apr 2025 04:22:46 GMT
##[error]Process completed with exit code 2.
Post job cleanup.
[command]"C:\Program Files\Git\bin\git.exe" version

bors · 2025-04-11T04:23:39Z

💔 Test failed - checks-actions

Zalathar · 2025-04-11T04:35:08Z

@bors retry

Prepend temp files with per-invocation random string to avoid temp filename conflicts rust-lang#139407 uncovered a very subtle unsoundness with incremental codegen, failing compilation sessions (due to assembler errors), and the "prefer hard linking over copying files" strategy we use in the compiler for file management. Specifically, imagine we're building a single file 3 times, all with `-Csave-temps -Cincremental=...`. Let's call the object file we're building for the codegen unit for `main` "`XXX.o`" just for clarity since it's probably some gigantic hash name: ``` #[inline(never)] #[cfg(any(rpass1, rpass3))] fn a() -> i32 { 0 } #[cfg(any(cfail2))] fn a() -> i32 { 1 } fn main() { evil::evil(); assert_eq!(a(), 0); } mod evil { #[cfg(any(rpass1, rpass3))] pub fn evil() { unsafe { std::arch::asm!("/* */"); } } #[cfg(any(cfail2))] pub fn evil() { unsafe { std::arch::asm!("missing"); } } } ``` Session 1 (`rpass1`): * Type-check, borrow-check, etc. * Serialize the dep graph to the incremental working directory `.../s-...-working/`. * Codegen object file to a temp file `XXX.rcgu.o` which is spit out in the cwd. * Hard-link[^1] `XXX.rcgu.o` to the incremental working directory `.../s-...-working/XXX.o`. * Save-temps option means we don't delete `XXX.rgcu.o`. * Link the binary and stuff. * Finalize[^2] the working incremental session by renaming `.../s-...-working` to ` s-...-asjkdhsjakd` (some other finalized incr comp session dir name). Session 2 (`cfail2`): * Load artifacts from the previous *finalized* incremental session, namely the dep graph. * Type-check, borrow-check, etc. since the file has changed, so most dep graph nodes are red. * Serialize the dep graph to the incremental working directory `.../s-...-working/`. * Codegen object file to a temp file `XXX.rcgu.o`. **HERE IS THE PROBLEM**: The hard-link is still set up to point to the inode from `XXX.o` from the first session, so this also modifies the `XXX.o` in the previous finalized session directory. * Codegen emits an error b/c `missing` is not an instruction, so we abort before finalizing the incremental session. Specifically, this means that the *previous* session is the last finalized session. Session 3 (`rpass3`): * Load artifacts from the previous *finalized* incremental session, namely the dep graph. NOTE that this is from session 1. * All the dep graph nodes are green since we are basically replaying session 1. * codegen object file `XXX.o`, which is detected as *reused* from session 1 since dep nodes were green. That means we **reuse** `XXX.o` which had been dirtied from session 2. * Link the binary and stuff. This results in a binary which reuses some of the build artifacts from session 2, but thinks it's from session 1. At this point, I hope it's clear to see that the incremental results from session 1 were dirtied from session 2, but we reuse them as if session 1 was the previous (finalized) incremental session we ran. This is at best really buggy, and at worst **unsound**. This isn't limited to `-C save-temps`, since there are other combinations of flags that may keep around temporary files (hard linked) in the working directory (like `-C debuginfo=1 -C split-debuginfo=unpacked` on darwin, for example). --- This PR implements a fix which is to prepend temp filenames with a random string that is generated per invocation of rustc. This string is not *deterministic*, but temporary files are transient anyways, so I don't believe this is a problem. That means that temp files are now something like... `{crate-name}.{cgu}.{invocation_temp}.rcgu.o`, where `{invocation_temp}` is the new temporary string we generate per invocation of rustc. Fixes rust-lang#139407 [^1]: https://github.com/rust-lang/rust/blob/175dcc7773d65c1b1542c351392080f48c05799f/compiler/rustc_fs_util/src/lib.rs#L60 [^2]: https://github.com/rust-lang/rust/blob/175dcc7773d65c1b1542c351392080f48c05799f/compiler/rustc_incremental/src/persist/fs.rs#L1-L40

bors · 2025-04-11T13:59:38Z

⌛ Testing commit 9c372d8 with merge e1b06f7...

rustbot assigned jieyouxu Apr 6, 2025

rustbot added A-run-make Area: port run-make Makefiles to rmake.rs S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Apr 6, 2025

compiler-errors commented Apr 6, 2025

View reviewed changes

tests/run-make/dirty-incr-due-to-hard-link/rmake.rs Show resolved Hide resolved

compiler-errors force-pushed the incr branch from 9112f0f to d445713 Compare April 6, 2025 22:52

This comment has been minimized.

Sign in to view

compiler-errors changed the title ~~Unlink temporary file hard links from previous session before writing into them in codegen~~ Prepend temp files with per-invocation random string to avoid temp filename conflicts Apr 6, 2025

compiler-errors force-pushed the incr branch from d445713 to 54653a0 Compare April 6, 2025 23:53

This comment has been minimized.

Sign in to view

compiler-errors force-pushed the incr branch from 54653a0 to 8c001d1 Compare April 7, 2025 02:16

jieyouxu approved these changes Apr 7, 2025

View reviewed changes

jieyouxu added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 7, 2025

compiler-errors added 2 commits April 7, 2025 20:48

Simplify temp path creation a bit

effef88

Prepend temp files with a string per invocation of rustc

9c372d8

compiler-errors force-pushed the incr branch from 8c001d1 to 9c372d8 Compare April 7, 2025 20:48

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Apr 7, 2025

bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Apr 11, 2025

Zalathar added the CI-spurious-fail-mingw CI spurious failure: target env mingw label Apr 11, 2025

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 11, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Prepend temp files with per-invocation random string to avoid temp filename conflicts #139453

Prepend temp files with per-invocation random string to avoid temp filename conflicts #139453

compiler-errors commented Apr 6, 2025 •

edited

Loading

rustbot commented Apr 6, 2025

rustbot commented Apr 6, 2025

bjorn3 commented Apr 6, 2025 •

edited

Loading

compiler-errors commented Apr 6, 2025

bjorn3 commented Apr 6, 2025

the8472 commented Apr 6, 2025

rustbot commented Apr 6, 2025

compiler-errors commented Apr 6, 2025

This comment has been minimized.

rustbot commented Apr 6, 2025

compiler-errors commented Apr 6, 2025

This comment has been minimized.

compiler-errors commented Apr 7, 2025

jieyouxu left a comment •

edited

Loading

jieyouxu Apr 7, 2025 •

edited

Loading

compiler-errors Apr 7, 2025

jieyouxu commented Apr 7, 2025 •

edited

Loading

compiler-errors commented Apr 7, 2025

bors commented Apr 7, 2025

bors commented Apr 11, 2025

rust-log-analyzer commented Apr 11, 2025

bors commented Apr 11, 2025

Zalathar commented Apr 11, 2025

bors commented Apr 11, 2025

Prepend temp files with per-invocation random string to avoid temp filename conflicts #139453

Are you sure you want to change the base?

Prepend temp files with per-invocation random string to avoid temp filename conflicts #139453

Conversation

compiler-errors commented Apr 6, 2025 • edited Loading

Footnotes

rustbot commented Apr 6, 2025

rustbot commented Apr 6, 2025

bjorn3 commented Apr 6, 2025 • edited Loading

compiler-errors commented Apr 6, 2025

Footnotes

bjorn3 commented Apr 6, 2025

the8472 commented Apr 6, 2025

rustbot commented Apr 6, 2025

compiler-errors commented Apr 6, 2025

This comment has been minimized.

rustbot commented Apr 6, 2025

compiler-errors commented Apr 6, 2025

This comment has been minimized.

compiler-errors commented Apr 7, 2025

jieyouxu left a comment • edited Loading

Choose a reason for hiding this comment

jieyouxu Apr 7, 2025 • edited Loading

Choose a reason for hiding this comment

compiler-errors Apr 7, 2025

Choose a reason for hiding this comment

jieyouxu commented Apr 7, 2025 • edited Loading

compiler-errors commented Apr 7, 2025

bors commented Apr 7, 2025

bors commented Apr 11, 2025

rust-log-analyzer commented Apr 11, 2025

bors commented Apr 11, 2025

Zalathar commented Apr 11, 2025

bors commented Apr 11, 2025

compiler-errors commented Apr 6, 2025 •

edited

Loading

bjorn3 commented Apr 6, 2025 •

edited

Loading

jieyouxu left a comment •

edited

Loading

jieyouxu Apr 7, 2025 •

edited

Loading

jieyouxu commented Apr 7, 2025 •

edited

Loading