Repository navigation
fix: non-deterministic temporary object paths in linker maps - #165971
paradoxicalguy wants to merge 1 commit into
Conversation
rustbot
added
A-run-make
This comment has been minimized.
This comment has been minimized.
ad1882c to
57c0e63
Compare
| .fatal(format!("failed to initialize linker object directory: {e}")) | ||
| }); | ||
|
|
||
| fs::rename(staging.path(), &objects_dir).unwrap_or_else(|e| { |
There was a problem hiding this comment.
one caveat here: fs::rename can replace an existing empty directory on unix. the lock prevents races between rustc invocations, but not with unrelated processes. would a no-replace rename be worth considering?
This comment has been minimized.
This comment has been minimized.
57c0e63 to
ddd18e5
Compare
|
adding ignore gcc backend |
This comment has been minimized.
This comment has been minimized.
| // Both linker inputs have stable names, protected by the same per-output lock. | ||
| // The lockfile must not be removed: another invocation could otherwise lock a | ||
| // different inode while an earlier invocation still holds the original lock. | ||
| pub(super) struct StableLinkObjects { |
There was a problem hiding this comment.
I think this should just follow the same filename derivation as regular object files. Then you don't need a dedicated file lock.
There was a problem hiding this comment.
that would simplify this :D, yes
but temp_path_for_cgu() includes an invocation-specific suffix, which would still appear in linker maps. is there another part of the regular object filename derivation you had in mind?
There was a problem hiding this comment.
That problem also exists for regular object files referenced in the linker map and can be solved all at once later. And I believe the random suffix only applies to incr comp.
There was a problem hiding this comment.
updated this to use temp_path_for_cgu() for both objects, and removed the dedicated locking/directory code. related run-make tests pass locally. thanks for the suggestion
ddd18e5 to
0c65b0b
Compare
This comment has been minimized.
This comment has been minimized.
0c65b0b to
6238f9c
Compare
fixes #142989
rustc creates temporary
rmeta.oandsymbols.ofiles in randomly named directories, causing otherwise identical builds to produce different linker maps.this gives both objects deterministic paths derived from the output filename. a per-output lock protects them during linking, and they're cleaned up afterward unless
-C save-tempsis enabled.the lockfile is kept across builds to avoid races. feedback on a simpler approach is welcome.
testing:
added a run-make regression test comparing complete linker maps across repeated builds, verified identical GNU ld and LLD maps on AArch64 linux, tested concurrent linking (20 rounds), cleanup and
-C save-tempsr? compiler