Repository: bazelbuild/bazel
Issue: Building Windows DLL without exports fails
Labels: type: feature request
P2
platform: windows
category: misc > misc

### Description of the problem / feature request:

In order to produce import library for a DLL bazel uses `/IMPLIB` flag of the linker and expect that it will force creating import library in the place specified:
https://msdn.microsoft.com/en-us/library/67wc07b9.aspx

Unfortunately, this flag is not managing the fact of creating or not creating import library but only sets the name for it. If DLL does not export anything, linker do not create it by design. Unluckily, we actively use DLLs without imports for a reason.

The output of bazel is as follows:
```
C:\Users\Chebotarev_V\Documents\bazel-issues\windows-dll-without-exports>bazel build ...
INFO: Analysed target //:hellolib.dll (8 packages loaded).
INFO: Found 1 target...
ERROR: C:/users/chebotarev_v/documents/bazel-issues/windows-dll-without-exports/BUILD:3:1: output 'hellolib.ifso' was not created
ERROR: C:/users/chebotarev_v/documents/bazel-issues/windows-dll-without-exports/BUILD:3:1: not all outputs were created or valid
Target //:hellolib.dll failed to build
Use --verbose_failures to see the command lines of failed build steps.
INFO: Elapsed time: 49.652s, Critical Path: 1.14s
FAILED: Build did NOT complete successfully
```

Please find an example in my repository:
https://github.com/excitoon/bazel-issues/tree/master/windows-dll-without-exports

### Feature requests: what underlying problem are you trying to solve with this feature?

We need building DLLs without exports.

### Bugs: what's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.

https://github.com/excitoon/bazel-issues/tree/master/windows-dll-without-exports

### What operating system are you running Bazel on?

Windows 10 x64

### What's the output of `bazel info release`?

```
Build label: 0.10.0
Build target: bazel-out/x64_windows-opt/bin/src/main/java/com/google/devtools/build/lib/bazel/BazelServer_deploy.jar
Build time: Thu Nov 9 22:42:19 +50056 (1517474731339)
Build timestamp: 1517474731339
Build timestamp as int: 1517474731339
```

###  Have you found anything relevant by searching the web?

Nothing expect mentioned.


Issue comments:
The same error in tensorflow:
```
ERROR: C:/users/chebotarev_v/documents/src/tensorflow/tensorflow/contrib/lite/BUILD:62:1: output 'tensorflow/contrib/lite/libcontext.ifso' was not created
ERROR: C:/users/chebotarev_v/documents/src/tensorflow/tensorflow/contrib/lite/BUILD:62:1: Couldn't build file tensorflow/contrib/lite/libcontext.so: not all outputs were created or valid
```

@excitoon I noticed this problem when I was implementing dynamic linking on Windows. The thing is you can not tell Bazel "this action may or may not generate this file". There's no way we can know if any symbols will be exported before the linking action. So the interface library (.ifso) has to be a mandatory output of the action. 

However, I think it's meaningless if you build a DLL that expose no symbol, right?

And to help with this issue, I implemented the `windows_export_all_symbols` feature.
You can use it like this:
```
cc_binary(
    name = "hellolib.dll",
    srcs = [
        "sources/hello-library.cpp"
    ],
    features = ["windows_export_all_symbols"],
    linkshared = 1
)
```
Then Bazel will parse the object file of `sources/hello-library.cpp` to generate a DEF file, then use it during linking time to export all symbols found.

> However, I think it's meaningless if you build a DLL that expose no symbol, right?

Not always. DLL is always exposing `DllMain()` and this is more than enough to make interaction with invoking process. I worked around that by specifying trivial `DEF` file. That forces compiler to generate `ifso` in any case.
@meteorcloudy Did you make this feature to address #4561 ? If so it seems to be a huge overkill, according to https://docs.microsoft.com/en-us/cpp/build/reference/implib-name-import-library?view=vs-2017 :
> A program contains exports if one **or more** of the following are specified:
>     The __declspec(dllexport) keyword in the source code
>     EXPORTS statement in a .def file
>     An /EXPORT specification in a LINK command

The solution of this issue should be as follows - if we have no `win_def_file` and `windows_export_all_symbols`, we should generate a def file containing only `LIBRARY <LIBNAME>` and pass it to the linker. In this case linker will always generate IMPLIB and add all symbols which are exported in the code (if any).

I wonder if `windows_export_all_symbols` is used by somebody?
@excitoon No, I didn't make that feature just to address #4561, it is a part of https://docs.google.com/document/d/1ZK4f84wype5NsZ7ltOUqvyFr7eKOIf4Wd-DrU6HCBkA/edit#heading=h.atngtzdodour

There is a similar feature in CMake as well, https://cmake.org/cmake/help/v3.4/prop_tgt/WINDOWS_EXPORT_ALL_SYMBOLS.html
It is used in TensorFlow, https://github.com/tensorflow/tensorflow/blob/dc34ddc6ddcc0581050322a98abbfc19203484e4/tensorflow/tensorflow.bzl#L1635
@meteorcloudy I understand, but the original problem is still not fixed (`windows_export_all_symbols` can greatly increase the size of a library). May I proceed with the solution I proposed?
Oh, I see. You actually don't want anything to be exported but still want the DLL file. If this is the case, your solution sounds good to me. Please proceed, I'm happy to review the change.
Exactly my case. But I have to add that this is not only our case. For example, Microsoft uses that: https://stackoverflow.com/questions/6588249/dll-without-exported-functions . Even without exporting anything, `DllMain` is getting called, and also DLL can be used as a resource storage.
Hello @meteorcloudy, I've just found another repro case of this issue, it seems more legitimate: 
repro is [here](https://github.com/maximyurchuk/bazel-issues/tree/master/use_runfiles_from_cc_library). 

This example is more real-world, because `use_runfiles` (from link above) rule can pack targets with all runfiles dependencies as an example (and actually this case not working for me). In this sample there is no strange .dll files without exports: I just want build cc_library and put all it outputs somethere. 

NB: `lib.if.lib` not in `ctx.attr.input[DefaultInfo].data_runfiles.files`, but I got error about `lib.if.lib`

Pull request review comments:
Seems name of function is not very good, because def file created by this function is suitable only for `.dll`. Maybe createTrivial**Dll**DefFileAction is better? 
I think the fact we are creating a DEF file implies it's for a Windows DLL, there's no DEF file for shared library on other platforms, right?
I mean that def can be used for linking `.exe`, not only for `.dll`. But def file created by `createTrivialDefFileAction` not suitable for `.exe` creation, because of `LIBRARY ...` string, which doesn't work for `.exe`.  So I propose emphasize that def file suitable for `.dll`, not `.exe`. 
I see, thanks for the notice. Done.
The rule only uses one of the three DEF file Artifacts, yet we create all three with their Actions.
I'm worried about the memory cost of these Actions.
Can we create `trivialDefFile` only if needed?
Regarding my comment above: can we create the `trivialDefFile` Artifact and Action here?
Can we spare creating `trivialDefFile` here too, and create it only in line 346?
This decision logic is the same as in cc_library, can you extract it to a function?
"Trivial" carries little meaning for me. Can we find a more precise name? What do you think about "no-export" or "fallback"?

And how about calling the function `createDefFileForDllWithoutExports` or something similar?
Thanks for the advice! I refactored the logic, should be better now.
Done.
Done.
Done,
I replaced it with "empty" here, because it's essentially an empty file. Does it sound good?
Is `private` enough?
Could you remove `customDefFile`? Seems like we no longer use it. You can inline `common.getWinDefFile()` into `getWindowsDefFileForLinking`.
`generatedDefFile` is used again in line 641. I wonder, do you have to use `winDefFile` there instead? If so, please update and remove `generatedDefFile` here and declare it in the `if` block's scope below.
No, it should still be geenratedDefFile in line 641. Because that's for users to get the generated DEF file via `def_file` output group, it should work even when `windows_export_all_symbols` feature is not enabled.
Done.
Thanks, done.

Patch that resolved the issue:
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 f4ffbd86fd8970..660daabd4ab923 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
@@ -430,7 +430,7 @@ public static ConfiguredTarget init(CppSemantics semantics, RuleContext ruleCont
     CcLinkingContext depsCcLinkingContext = collectCcLinkingContext(ruleContext);
 
     Artifact generatedDefFile = null;
-    Artifact customDefFile = null;
+    Artifact winDefFile = null;
     if (isLinkShared(ruleContext)) {
       if (featureConfiguration.isEnabled(CppRuleClasses.TARGETS_WINDOWS)) {
         ImmutableList.Builder<Artifact> objectFiles = ImmutableList.builder();
@@ -457,7 +457,7 @@ public static ConfiguredTarget init(CppSemantics semantics, RuleContext ruleCont
               CppHelper.createDefFileActions(
                   ruleContext, defParser, objectFiles.build(), binary.getFilename());
         }
-        customDefFile = common.getWinDefFile();
+        winDefFile = CppHelper.getWindowsDefFileForLinking(ruleContext, common.getWinDefFile(), generatedDefFile, featureConfiguration);
       }
     }
 
@@ -507,8 +507,7 @@ public static ConfiguredTarget init(CppSemantics semantics, RuleContext ruleCont
             cppConfiguration,
             linkType,
             pdbFile,
-            generatedDefFile,
-            customDefFile);
+            winDefFile);
 
     CcLinkingOutputs ccLinkingOutputsBinary = ccLinkingOutputsAndCcLinkingInfo.first;
 
@@ -697,8 +696,7 @@ public static Pair<CcLinkingOutputs, CcLauncherInfo> createTransitiveLinkingActi
       CppConfiguration cppConfiguration,
       LinkTargetType linkType,
       Artifact pdbFile,
-      Artifact generatedDefFile,
-      Artifact customDefFile)
+      Artifact winDefFile)
       throws InterruptedException, RuleErrorException {
     CcCompilationOutputs.Builder ccCompilationOutputsBuilder =
         CcCompilationOutputs.builder()
@@ -823,11 +821,7 @@ public static Pair<CcLinkingOutputs, CcLauncherInfo> createTransitiveLinkingActi
         .setPdbFile(pdbFile)
         .setFake(fake);
 
-    if (customDefFile != null) {
-      ccLinkingHelper.setDefFile(customDefFile);
-    } else if (CppHelper.shouldUseGeneratedDefFile(ruleContext, featureConfiguration)) {
-      ccLinkingHelper.setDefFile(generatedDefFile);
-    }
+    ccLinkingHelper.setDefFile(winDefFile);
 
     return Pair.of(
         ccLinkingHelper.link(ccCompilationOutputsWithOnlyObjects),
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibrary.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibrary.java
index 814b066c590861..ee3cb3de730089 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibrary.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibrary.java
@@ -311,11 +311,10 @@ public static void init(
     if (ruleContext.getRule().getImplicitOutputsFunction() != ImplicitOutputsFunction.NONE
         || !ccCompilationOutputs.isEmpty()) {
       if (featureConfiguration.isEnabled(CppRuleClasses.TARGETS_WINDOWS)) {
-        // If user specifies a custom DEF file, then we use it.
-        Artifact defFile = common.getWinDefFile();
+        Artifact customDefFile = common.getWinDefFile();
+        Artifact generatedDefFile = null;
 
         Artifact defParser = common.getDefParser();
-        Artifact generatedDefFile = null;
         if (defParser != null) {
           try {
             generatedDefFile =
@@ -332,18 +331,7 @@ public static void init(
             throw ruleContext.throwWithRuleError(e.getMessage());
           }
         }
-
-        // If no DEF file is specified and the windows_export_all_symbols feature is enabled, parse
-        // object files to generate DEF file and use it to export symbols - if we have a parser.
-        // Otherwise, use no DEF file.
-        if (defFile == null
-            && CppHelper.shouldUseGeneratedDefFile(ruleContext, featureConfiguration)) {
-          defFile = generatedDefFile;
-        }
-
-        if (defFile != null) {
-          linkingHelper.setDefFile(defFile);
-        }
+        linkingHelper.setDefFile(CppHelper.getWindowsDefFileForLinking(ruleContext, customDefFile, generatedDefFile, featureConfiguration));
       }
       ccLinkingOutputs = linkingHelper.link(ccCompilationOutputs);
     }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java
index b576d4b7d5ff34..7f45281fb4b7e0 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java
@@ -47,6 +47,7 @@
 import com.google.devtools.build.lib.analysis.TransitiveInfoCollection;
 import com.google.devtools.build.lib.analysis.actions.ActionConstructionContext;
 import com.google.devtools.build.lib.analysis.actions.CustomCommandLine;
+import com.google.devtools.build.lib.analysis.actions.FileWriteAction;
 import com.google.devtools.build.lib.analysis.actions.SpawnAction;
 import com.google.devtools.build.lib.analysis.actions.SymlinkAction;
 import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
@@ -62,6 +63,7 @@
 import com.google.devtools.build.lib.packages.Type;
 import com.google.devtools.build.lib.rules.cpp.CcLinkingContext.Linkstamp;
 import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.ExpansionException;
+import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.Feature;
 import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.FeatureConfiguration;
 import com.google.devtools.build.lib.rules.cpp.CppConfiguration.Tool;
 import com.google.devtools.build.lib.rules.cpp.Link.LinkTargetType;
@@ -885,6 +887,42 @@ public static Artifact createDefFileActions(
     return defFile;
   }
 
+  /**
+   * Create action for generating an empty DEF file without any exports, should only be used when
+   * targeting Windows.
+   *
+   * @return The artifact of an empty DEF file.
+   */
+  private static Artifact createEmptyDefFileAction(RuleContext ruleContext) {
+    Artifact trivialDefFile =
+        ruleContext.getBinArtifact(
+            ruleContext.getLabel().getName()
+                + ".gen.empty"
+                + Iterables.getOnlyElement(CppFileTypes.WINDOWS_DEF_FILE.getExtensions()));
+    ruleContext.registerAction(FileWriteAction.create(ruleContext, trivialDefFile, "", false));
+    return trivialDefFile;
+  }
+
+  /**
+   * Decide which DEF file should be used for the linking action.
+   *
+   * @return The artifact of the DEF file that should be used for the linking action.
+   */
+  public static Artifact getWindowsDefFileForLinking(
+      RuleContext ruleContext, Artifact customDefFile, Artifact generatedDefFile, FeatureConfiguration featureConfiguration) {
+    // 1. If a custom DEF file is specified in win_def_file attribute, use it.
+    // 2. If a generated DEF file is available and should be used, use it.
+    // 3. Otherwise, we use a empty DEF file to ensure the import library will be generated.
+    if (customDefFile != null) {
+      return customDefFile;
+    } else if (generatedDefFile != null && CppHelper.shouldUseGeneratedDefFile(ruleContext, featureConfiguration)) {
+      return generatedDefFile;
+    } else {
+      return createEmptyDefFileAction(ruleContext);
+    }
+  }
+
+
   /**
    * Returns true if the build implied by the given config and toolchain uses --start-lib/--end-lib
    * ld options.


Test patch:
diff --git a/src/test/py/bazel/bazel_windows_cpp_test.py b/src/test/py/bazel/bazel_windows_cpp_test.py
index 4c8e45bace7569..0cca8e5e44e7cf 100644
--- a/src/test/py/bazel/bazel_windows_cpp_test.py
+++ b/src/test/py/bazel/bazel_windows_cpp_test.py
@@ -138,15 +138,15 @@ def testBuildDynamicLibraryWithUserExportedSymbol(self):
     self.AssertExitCode(exit_code, 0, stderr)
 
     # TODO(pcloudy): change suffixes to .lib and .dll after making DLL
-    # extensions correct on
-    # Windows.
+    # extensions correct on Windows.
     import_library = os.path.join(bazel_bin, 'A.if.lib')
     shared_library = os.path.join(bazel_bin, 'A.dll')
-    def_file = os.path.join(bazel_bin, 'A.gen.def')
+    empty_def_file = os.path.join(bazel_bin, 'A.gen.empty.def')
+
     self.assertTrue(os.path.exists(import_library))
     self.assertTrue(os.path.exists(shared_library))
-    # DEF file shouldn't be generated for //:A
-    self.assertFalse(os.path.exists(def_file))
+    # An empty DEF file should be generated for //:A
+    self.assertTrue(os.path.exists(empty_def_file))
 
   def testBuildDynamicLibraryWithExportSymbolFeature(self):
     self.createProjectFiles()
@@ -159,8 +159,7 @@ def testBuildDynamicLibraryWithExportSymbolFeature(self):
     self.AssertExitCode(exit_code, 0, stderr)
 
     # TODO(pcloudy): change suffixes to .lib and .dll after making DLL
-    # extensions correct on
-    # Windows.
+    # extensions correct on Windows.
     import_library = os.path.join(bazel_bin, 'B.if.lib')
     shared_library = os.path.join(bazel_bin, 'B.dll')
     def_file = os.path.join(bazel_bin, 'B.gen.def')
@@ -175,8 +174,15 @@ def testBuildDynamicLibraryWithExportSymbolFeature(self):
         'build', '//:B', '--output_groups=dynamic_library',
         '--features=no_windows_export_all_symbols'
     ])
-    self.AssertExitCode(exit_code, 1, stderr)
-    self.assertIn('output \'B.if.lib\' was not created', ''.join(stderr))
+    self.AssertExitCode(exit_code, 0, stderr)
+    import_library = os.path.join(bazel_bin, 'B.if.lib')
+    shared_library = os.path.join(bazel_bin, 'B.dll')
+    empty_def_file = os.path.join(bazel_bin, 'B.gen.empty.def')
+    self.assertTrue(os.path.exists(import_library))
+    self.assertTrue(os.path.exists(shared_library))
+    # An empty DEF file should be generated for //:B
+    self.assertTrue(os.path.exists(empty_def_file))
+    self.AssertFileContentNotContains(empty_def_file, 'hello_B')
 
   def testBuildCcBinaryWithDependenciesDynamicallyLinked(self):
     self.createProjectFiles()
@@ -195,7 +201,7 @@ def testBuildCcBinaryWithDependenciesDynamicallyLinked(self):
     # a_shared_library
     self.assertTrue(os.path.exists(os.path.join(bazel_bin, 'A.dll')))
     # a_def_file
-    self.assertFalse(os.path.exists(os.path.join(bazel_bin, 'A.gen.def')))
+    self.assertTrue(os.path.exists(os.path.join(bazel_bin, 'A.gen.empty.def')))
     # b_import_library
     self.assertTrue(os.path.exists(os.path.join(bazel_bin, 'B.if.lib')))
     # b_shared_library
@@ -505,6 +511,29 @@ def testGetDefFileOfSharedLibraryFromCcBinary(self):
     self.AssertFileContentContains(def_file, 'hello_B')
     self.AssertFileContentContains(def_file, 'hello_C')
 
+  def testBuildSharedLibraryWithoutAnySymbolExported(self):
+    self.createProjectFiles()
+    self.ScratchFile(
+      'BUILD',
+      [
+        'cc_binary(',
+        '  name = "A.dll",',
+        '  srcs = ["a.cc", "a.h"],',
+        '  copts = ["/DNO_DLLEXPORT"],',
+        '  linkshared = 1,'
+        ')',
+      ])
+    bazel_bin = self.getBazelInfo('bazel-bin')
+
+    exit_code, _, stderr = self.RunBazel(['build', '//:A.dll'])
+    self.AssertExitCode(exit_code, 0, stderr)
+
+    # Although windows_export_all_symbols is not specified for this target,
+    # we should still be able to build a DLL without any symbol exported.
+    empty_def_file = os.path.join(bazel_bin, 'A.dll.gen.empty.def')
+    self.assertTrue(os.path.exists(empty_def_file))
+    self.AssertFileContentNotContains(empty_def_file, 'hello_A')
+
   def testUsingDefFileGeneratedFromCcLibrary(self):
     self.CreateWorkspaceWithDefaultRepos('WORKSPACE')
     self.ScratchFile('lib_A.cc', ['void hello_A() {}'])