https://git.ruby-lang.org/ruby.git/commit/?id=e6f4a39a4d

From e6f4a39a4de28067ff0b5dca55a8c09a8f9e2006 Mon Sep 17 00:00:00 2001
From: Alan Wu <XrXr@u...>
Date: Thu, 28 Oct 2021 17:48:21 -0400
Subject: MJIT MSVC: Use /Z7 to avoid PDB write race

With MSVC, MJIT uses the /Fd option on an installed PDB file when
compiling. Combined with the /Zi option, this causes the PDB file to be
modified every time MJIT compiles. Concurrent modifications to the same
PDB file is known to cause problems. MSVC even has an option, /FS to
deal with it. When running MJIT tests in parallel, sometimes this leads
to corrupting the PDB file, breaking subsequent compilations. On CI,
we get messages like these:

    rb_mjit_header-3.1.0.pdb is not the pdb file that was used when this precompiled header was created, recreate the precompiled header.

To avoid this race, use the /Z7 option when building precompiled header,
which asks the compiler to put debug info into the .obj file,
eliminating the need for pointing the compiler to the PDB file for the
precompiled header.

The /Fd option is changed to use a unique path based on the name of the
dll output. Because of the /debug linker flag, we generate a PDB file
at runtime even though we use /Z7.

There are a couple things missing from this change:
 - Because MJIT uses the interpreter's CFLAGS build option and that
   contains /Zi, putting /Z7 at the end leads to a build warning
 - With /Z7 no PDB file is built anymore, so the code for installing
   the PDB file can be removed

There might also be other problems with this change I haven't noticed
while developing this change using Github Actions. I don't have a
Windows dev environment with Visual Studio so I can't finish this
change easily. Please feel free to complete this change if it makes
sense.

Note:
 - On master, you can see the PDB file changing with llvm-pdbutil or a
   simple checksum. There is an age field in the file that is bumped
 - I'm not sure if users can specify compile flags on MSVC. If they
   couldn't, maybe it's easier to change MJIT's compile options to
   use /Z7 when building the precompile header.
 - MJIT could pass different options at runtime to generate fewer
   files. Right now it inherits the /DEBUG linker flag which causes
   a PDB file to be generated at runtime even though /Z7 is used.

Relevant MSVC docs:
 - [/Zi,/Z7](https://docs.microsoft.com/en-us/cpp/build/reference/z7-zi-zi-debug-information-format?view=msvc-160)
 - [/DEBUG](https://docs.microsoft.com/en-us/cpp/build/reference/debug-generate-debug-info?view=msvc-160)
 - [/FS](https://docs.microsoft.com/en-us/cpp/build/reference/fs-force-synchronous-pdb-writes?view=msvc-160)
---
 mjit_worker.c      | 13 ++++++++++---
 win32/Makefile.sub |  2 +-
 2 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/mjit_worker.c b/mjit_worker.c
index c6528a9bda5..a3a2acab7fa 100644
--- a/mjit_worker.c
+++ b/mjit_worker.c
@@ -788,7 +788,7 @@ static const int c_file_access_mode = https://github.com/ruby/ruby/blob/trunk/mjit_worker.c#L788
 static bool
 compile_c_to_so(const char *c_file, const char *so_file)
 {
-    const char *files[] = { NULL, NULL, NULL, NULL, NULL, NULL, "-link", libruby_pathflag, NULL };
+    const char *files[] = { NULL, NULL, NULL, NULL, NULL, NULL, NULL, "-link", libruby_pathflag, NULL };
     char *p;
 
     // files[0] = "-Fe*.dll"
@@ -824,12 +824,19 @@ compile_c_to_so(const char *c_file, const char *so_file) https://github.com/ruby/ruby/blob/trunk/mjit_worker.c#L824
     *p = '\0';
 
     // files[5] = "-Fd*.pdb"
-    files[5] = p = alloca(sizeof(char) * (rb_strlen_lit("-Fd") + strlen(pch_file) + 1));
+    // Generate .pdb file in temporary directory instead of cwd.
+    files[5] = p = alloca(sizeof(char) * (rb_strlen_lit("-Fd") + strlen(so_file) - rb_strlen_lit(DLEXT) + rb_strlen_lit(".pdb") + 1));
     p = append_lit(p, "-Fd");
-    p = append_str2(p, pch_file, strlen(pch_file) - rb_strlen_lit(".pch"));
+    p = append_str2(p, so_file, strlen(so_file) - rb_strlen_lit(DLEXT));
     p = append_lit(p, ".pdb");
     *p = '\0';
 
+    // files[6] = "-Z7"
+    // Put this last to override any debug options that came previously.
+    files[6] = p = alloca(sizeof(char) * rb_strlen_lit("-Z7") + 1);
+    p = append_lit(p, "-Z7");
+    *p = '\0';
+
     char **args = form_args(5, CC_LDSHARED_ARGS, CC_CODEFLAG_ARGS,
             files, CC_LIBS, CC_DLDFLAGS_ARGS);
     if (args == NULL)
diff --git a/win32/Makefile.sub b/win32/Makefile.sub
index ce8cafe8075..c0537f02713 100644
--- a/win32/Makefile.sub
+++ b/win32/Makefile.sub
@@ -1335,7 +1335,7 @@ clean-local:: https://github.com/ruby/ruby/blob/trunk/win32/Makefile.sub#L1335
 $(TIMESTAMPDIR)/$(MJIT_PRECOMPILED_HEADER_NAME:.pch=).time: probes.h vm.$(OBJEXT)
 	$(ECHO) building $(@F:.time=.pch)
 	$(Q) $(CC) -DMJIT_HEADER $(CFLAGS) $(XCFLAGS:-DRUBY_EXPORT =) -URUBY_EXPORT $(CPPFLAGS) $(srcdir)/vm.c -c -Yc \
-	  $(COUTFLAG)$(@F:.time=.)$(OBJEXT) -Fd$(@F:.time=.pdb) -Fp$(@F:.time=.pch).new
+	  $(COUTFLAG)$(@F:.time=.)$(OBJEXT) -Fd$(@F:.time=.pdb) -Fp$(@F:.time=.pch).new -Z7
 	$(Q) $(IFCHANGE) "--timestamp=$@" $(@F:.time=.pch) $(@F:.time=.pch).new
 
 $(MJIT_PRECOMPILED_HEADER_NAME): $(TIMESTAMPDIR)/$(MJIT_PRECOMPILED_HEADER_NAME:.pch=).time
-- 
cgit v1.2.1


--
ML: ruby-changes@q...
Info: http://www.atdot.net/~ko1/quickml/