Generating PDB files on Windows

1. Add /Z7 as compiler flag in CROSSTOOL, this causes full debugging
   infomation built into object files, no PDB file is generated.

2. Add /DEBUG as linker flag so that a PDB file will be generated for
   executable or dll.
     * /DEBUG:FULL for dbg mode. the full PDB can be used to debug the
       executable when no other build products are available, such as
       when the executable is deployed.
     * /DEBUG:FASTLINK for fastbuild mode. object files are still needed
       when debugging the executable, but linking speed can be two to four
       times faster that full PDB generation.
     * No option is added for opt mode.
     More detailed info: https://msdn.microsoft.com/en-us/library/xe4t6fc1.aspx

3. Add an empty feature in MSVC CROSSTOOL to tell Bazel we need PDB file

4. Add PDB file artifact in an output named pdb_file of cc_binary,
   then you can build the pdb file by bazel build //foo/bar:bin --output_groups=pdb_file

5. Add test for PDB file generation

Change-Id: Ia5096470187ebca72f2c804f32d5b723f40c0b85
PiperOrigin-RevId: 153449059

5 files changed, 90 insertions, 13 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcBinary.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcBinary.java
index 9b9ae3118f..ca98d7bf61 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcBinary.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcBinary.java
@@ -297,6 +297,14 @@ public abstract class CcBinary implements RuleConfiguredTargetFactory {
       linkActionBuilder.setLTOIndexing(false);
     }
 
+    // On Windows, if GENERATE_PDB_FILE feature is enabled
+    // then a pdb file will be built along with the executable.
+    Artifact pdbFile = null;
+    if (featureConfiguration.isEnabled(CppRuleClasses.GENERATE_PDB_FILE)) {
+      pdbFile = ruleContext.getRelatedArtifact(binary.getRootRelativePath(), ".pdb");
+      linkActionBuilder.addActionOutput(pdbFile);
+    }
+
     CppLinkAction linkAction = linkActionBuilder.build();
     ruleContext.registerAction(linkAction);
     LibraryToLink outputLibrary = linkAction.getOutputLibrary();
@@ -412,6 +420,11 @@ public abstract class CcBinary implements RuleConfiguredTargetFactory {
           new ExecutionInfoProvider(ImmutableMap.of(ExecutionRequirements.REQUIRES_DARWIN, "")));
     }
 
+    // If PDB file is generated by the link action, we add it to pdb_file output group
+    if (pdbFile != null) {
+      ruleBuilder.addOutputGroup("pdb_file", pdbFile);
+    }
+
     return ruleBuilder
         .addProvider(RunfilesProvider.class, RunfilesProvider.simple(runfiles))
         .addProvider(
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppRuleClasses.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppRuleClasses.java
index 698353a105..6cad3decee 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppRuleClasses.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppRuleClasses.java
@@ -283,6 +283,12 @@ public class CppRuleClasses {
    */
   public static final String THIN_LTO = "thin_lto";
 
+  /**
+   * A string constant for the PDB file generation feature, should only be used for toolchains
+   * targeting Windows that include a linker producing PDB files
+   */
+  public static final String GENERATE_PDB_FILE = "generate_pdb_file";
+
   /*
    * A string constant for the fdo_instrument feature.
    */
diff --git a/src/test/shell/bazel/bazel_windows_example_test.sh b/src/test/shell/bazel/bazel_windows_example_test.sh
index 5f164990af..e62dc95b1c 100755
--- a/src/test/shell/bazel/bazel_windows_example_test.sh
+++ b/src/test/shell/bazel/bazel_windows_example_test.sh
@@ -57,6 +57,10 @@ function assert_binary_run_from_subdir() {
 function test_cpp() {
   local cpp_pkg=examples/cpp
   assert_build_output ./bazel-bin/${cpp_pkg}/libhello-lib.a ${cpp_pkg}:hello-world
+  assert_build_output ./bazel-bin/${cpp_pkg}/hello-world.pdb ${cpp_pkg}:hello-world --output_groups=pdb_file
+  assert_build_output ./bazel-bin/${cpp_pkg}/hello-world.pdb -c dbg ${cpp_pkg}:hello-world --output_groups=pdb_file
+  assert_build -c opt ${cpp_pkg}:hello-world --output_groups=pdb_file
+  test -f ./bazel-bin/${cpp_pkg}/hello-world.pdb && fail "PDB file should not be generated in OPT mode"
   assert_bazel_run "//examples/cpp:hello-world foo" "Hello foo"
   assert_test_ok "//examples/cpp:hello-success_test"
   assert_test_fails "//examples/cpp:hello-fail_test"
diff --git a/tools/cpp/CROSSTOOL.tpl b/tools/cpp/CROSSTOOL.tpl
index 60fb955e95..8bf57e5808 100644
--- a/tools/cpp/CROSSTOOL.tpl
+++ b/tools/cpp/CROSSTOOL.tpl
@@ -463,6 +463,16 @@ toolchain {
   }
 
   feature {
+    name: 'generate_pdb_file'
+    requires: {
+      feature: 'dbg'
+    }
+    requires: {
+      feature: 'fastbuild'
+    }
+  }
+
+  feature {
     name: 'has_configured_linker_path'
   }
 
@@ -641,7 +651,6 @@ toolchain {
     }
   }
 
-
   feature {
     name: 'linker_param_file'
     flag_set {
@@ -664,31 +673,78 @@ toolchain {
     }
   }
 
+  feature {
+    name: 'dbg'
+    flag_set {
+      action: 'c-compile'
+      action: 'c++-compile'
+      flag_group {
+        flag: "/Od"
+        flag: '/MTd'
+        flag: "/Z7"
+      }
+    }
+    flag_set {
+      action: 'c++-link-executable'
+      action: 'c++-link-dynamic-library'
+      flag_group {
+        flag: "/DEBUG:FULL"
+        flag: "/INCREMENTAL:NO"
+      }
+    }
+    implies: 'generate_pdb_file'
+  }
+
+  feature {
+    name: 'fastbuild'
+    flag_set {
+      action: 'c-compile'
+      action: 'c++-compile'
+      flag_group {
+        flag: "/Od"
+        flag: '/MT'
+        flag: "/Z7"
+      }
+    }
+    flag_set {
+      action: 'c++-link-executable'
+      action: 'c++-link-dynamic-library'
+      flag_group {
+        flag: "/DEBUG:FASTLINK"
+        flag: "/INCREMENTAL:NO"
+      }
+    }
+    implies: 'generate_pdb_file'
+  }
+
+  feature {
+    name: 'opt'
+    flag_set {
+      action: 'c-compile'
+      action: 'c++-compile'
+      flag_group {
+        flag: "/O2"
+        flag: '/MT'
+      }
+    }
+  }
+
   compilation_mode_flags {
     mode: DBG
-    compiler_flag: "/DDEBUG=1"
-    # This will signal the wrapper that we are doing a debug build, which sets
-    # some internal state of the toolchain wrapper. It is intentionally a "-"
-    # flag to make this very obvious.
-    compiler_flag: "-g"
-    compiler_flag: "/Od"
     compiler_flag: "-Xcompilation-mode=dbg"
     linker_flag: "-Xcompilation-mode=dbg"
   }
 
   compilation_mode_flags {
     mode: FASTBUILD
-    compiler_flag: "/DNDEBUG"
-    compiler_flag: "/Od"
     compiler_flag: "-Xcompilation-mode=fastbuild"
     linker_flag: "-Xcompilation-mode=fastbuild"
   }
 
   compilation_mode_flags {
     mode: OPT
-    compiler_flag: "/DNDEBUG"
-    compiler_flag: "/O2"
     compiler_flag: "-Xcompilation-mode=opt"
     linker_flag: "-Xcompilation-mode=opt"
   }
+
 }
diff --git a/tools/cpp/wrapper/bin/pydir/msvc_tools.py.tpl b/tools/cpp/wrapper/bin/pydir/msvc_tools.py.tpl
index 0695593d5f..127aa87c06 100644
--- a/tools/cpp/wrapper/bin/pydir/msvc_tools.py.tpl
+++ b/tools/cpp/wrapper/bin/pydir/msvc_tools.py.tpl
@@ -263,8 +263,6 @@ class ArgParser(object):
       if arg.startswith('/Fo') or arg.startswith('/Fa') or arg.startswith(
           '/Fi'):
         self.output_file = arg[3:]
-        self.options.append(
-            '/Fd%s.pdb' % self.NormPath(os.path.splitext(self.output_file)[0]))
       if num_matched == 0:
         # Strip out any .a's that have 0 size, they are header or intermediate
         # dependency libraries and don't contain any code. 0-length files are