Add workaround for dsymutil stack overflow during fat LTO

Summary: Motivation described in comment. Reviewed By: jtorkkola fbshipit-source-id: ac7f462c8e
styurin · Apr 19, 2019 · 38712b7 · 38712b7
1 parent 6ec9288
commit 38712b7
Show file tree

Hide file tree

Showing 5 changed files with 69 additions and 12 deletions.
diff --git a/src/com/facebook/buck/apple/AppleConfig.java b/src/com/facebook/buck/apple/AppleConfig.java
@@ -429,6 +429,11 @@ public boolean addCellPathToIquotePath() {
     return delegate.getBoolean(APPLE_SECTION, "add_cell_path_to_iquote_path").orElse(true);
   }
 
+  public boolean shouldWorkAroundDsymutilLTOStackOverflowBug() {
+    return delegate.getBooleanValue(
+        APPLE_SECTION, "work_around_dsymutil_lto_stack_overflow_bug", false);
+  }
+
   @Value.Immutable
   @BuckStyleTuple
   interface AbstractApplePackageConfig {

diff --git a/src/com/facebook/buck/apple/toolchain/impl/AppleCxxPlatforms.java b/src/com/facebook/buck/apple/toolchain/impl/AppleCxxPlatforms.java
@@ -33,6 +33,7 @@
 import com.facebook.buck.core.model.UserFlavor;
 import com.facebook.buck.core.sourcepath.PathSourcePath;
 import com.facebook.buck.core.toolchain.tool.Tool;
+import com.facebook.buck.core.toolchain.tool.impl.CommandTool;
 import com.facebook.buck.core.toolchain.tool.impl.VersionedTool;
 import com.facebook.buck.core.toolchain.toolprovider.impl.ConstantToolProvider;
 import com.facebook.buck.core.util.log.Logger;
@@ -277,6 +278,15 @@ public static AppleCxxPlatform buildWithXcodeToolFinder(
         getXcodeTool(
             filesystem, toolSearchPaths, xcodeToolFinder, appleConfig, "dsymutil", version);
 
+    // We are seeing a stack overflow in dsymutil during (fat) LTO
+    // builds. Upstream dsymutil was patched to avoid recursion in the
+    // offending path in https://reviews.llvm.org/D48899, and
+    // https://reviews.llvm.org/D45172 mentioned that there is much
+    // more stack space available when single threaded.
+    if (appleConfig.shouldWorkAroundDsymutilLTOStackOverflowBug()) {
+      dsymutil = new CommandTool.Builder(dsymutil).addArg("-num-threads=1").build();
+    }
+
     Tool lipo =
         getXcodeTool(filesystem, toolSearchPaths, xcodeToolFinder, appleConfig, "lipo", version);
 

diff --git a/src/com/facebook/buck/apple/toolchain/impl/BUCK b/src/com/facebook/buck/apple/toolchain/impl/BUCK
@@ -20,6 +20,7 @@ java_library_with_plugins(
         "//src/com/facebook/buck/io:io",
         "//src/com/facebook/buck/io/file:file",
         "//src/com/facebook/buck/io/filesystem:filesystem",
+        "//src/com/facebook/buck/rules/args:args",
         "//src/com/facebook/buck/swift:swift",
         "//src/com/facebook/buck/swift/toolchain:toolchain",
         "//src/com/facebook/buck/swift/toolchain/impl:impl",

diff --git a/test/com/facebook/buck/apple/AppleConfigTest.java b/test/com/facebook/buck/apple/AppleConfigTest.java
@@ -21,8 +21,10 @@
 import static org.hamcrest.Matchers.notNullValue;
 import static org.hamcrest.Matchers.startsWith;
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertThat;
+import static org.junit.Assert.assertTrue;
 import static org.junit.Assume.assumeTrue;
 
 import com.facebook.buck.apple.toolchain.ApplePlatform;
@@ -196,4 +198,24 @@ public void testDefaultCodesignTimeout() {
     AppleConfig config = FakeBuckConfig.builder().build().getView(AppleConfig.class);
     assertThat(config.getCodesignTimeout(), equalTo(Duration.ofSeconds(300)));
   }
+
+  @Test
+  public void testShouldWorkAroundDsymutilLTOStackOverflowBug() {
+    AppleConfig configExplicitTrue =
+        FakeBuckConfig.builder()
+            .setSections("[apple]", "work_around_dsymutil_lto_stack_overflow_bug = true")
+            .build()
+            .getView(AppleConfig.class);
+    assertTrue(configExplicitTrue.shouldWorkAroundDsymutilLTOStackOverflowBug());
+
+    AppleConfig configExplicitFalse =
+        FakeBuckConfig.builder()
+            .setSections("[apple]", "work_around_dsymutil_lto_stack_overflow_bug = false")
+            .build()
+            .getView(AppleConfig.class);
+    assertFalse(configExplicitFalse.shouldWorkAroundDsymutilLTOStackOverflowBug());
+
+    AppleConfig configUnset = FakeBuckConfig.builder().build().getView(AppleConfig.class);
+    assertFalse(configUnset.shouldWorkAroundDsymutilLTOStackOverflowBug());
+  }
 }
diff --git a/test/com/facebook/buck/apple/toolchain/impl/AppleCxxPlatformsTest.java b/test/com/facebook/buck/apple/toolchain/impl/AppleCxxPlatformsTest.java
@@ -153,8 +153,7 @@ private void touchFile(Path file) {
     }
   }
 
-  @Test
-  public void iphoneOSSdkPathsBuiltFromDirectory() {
+  private AppleCxxPlatform buildAppleCxxPlatformWithConfig(BuckConfig buckConfig) {
     AppleSdkPaths appleSdkPaths =
         AppleSdkPaths.builder()
             .setDeveloperPath(developerDir)
@@ -191,17 +190,21 @@ public void iphoneOSSdkPathsBuiltFromDirectory() {
             .build();
     paths.forEach(this::touchFile);
 
+    return AppleCxxPlatforms.buildWithXcodeToolFinder(
+        projectFilesystem,
+        targetSdk,
+        "7.0",
+        "armv7",
+        appleSdkPaths,
+        buckConfig,
+        new XcodeToolFinder(buckConfig.getView(AppleConfig.class)),
+        new AppleCxxPlatforms.XcodeBuildVersionCache());
+  }
+
+  @Test
+  public void iphoneOSSdkPathsBuiltFromDirectory() {
     BuckConfig buckConfig = FakeBuckConfig.builder().build();
-    AppleCxxPlatform appleCxxPlatform =
-        AppleCxxPlatforms.buildWithXcodeToolFinder(
-            projectFilesystem,
-            targetSdk,
-            "7.0",
-            "armv7",
-            appleSdkPaths,
-            buckConfig,
-            new XcodeToolFinder(buckConfig.getView(AppleConfig.class)),
-            new AppleCxxPlatforms.XcodeBuildVersionCache());
+    AppleCxxPlatform appleCxxPlatform = buildAppleCxxPlatformWithConfig(buckConfig);
 
     CxxPlatform cxxPlatform = appleCxxPlatform.getCxxPlatform();
 
@@ -485,6 +488,22 @@ public void appleTVOSSdkPathsBuiltFromDirectory() {
             .get(0));
   }
 
+  @Test
+  public void worksAroundDsymutilLTOStackOverflowBug() {
+    BuckConfig buckConfig =
+        FakeBuckConfig.builder()
+            .setSections("[apple]", "work_around_dsymutil_lto_stack_overflow_bug = true")
+            .build();
+    AppleCxxPlatform appleCxxPlatform = buildAppleCxxPlatformWithConfig(buckConfig);
+    BuildRuleResolver ruleResolver = new TestActionGraphBuilder();
+    SourcePathResolver resolver =
+        DefaultSourcePathResolver.from(new SourcePathRuleFinder(ruleResolver));
+    assertEquals(
+        ImmutableList.of(
+            "/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/dsymutil", "-num-threads=1"),
+        appleCxxPlatform.getDsymutil().getCommandPrefix(resolver));
+  }
+
   @Test
   public void invalidFlavorCharactersInSdkAreEscaped() {
     AppleSdkPaths appleSdkPaths =