Rollback of commit 6af85020b520a9dd2bd913562b16716c29c3dbc3.

*** Reason for rollback ***

Breaks ios_test targets.

*** Original change description ***

Add two binary size optimizations when --compilation_mode=opt is specified:
1. Symbol strippings. A new strip action is registered that uses Darwin tool /usr/bin/strip to remove the symbol table of the linked binary.
2. Dead-code strippings, which uses linker flag "--dead_strip" to remove unreachable code in binary link action.

RELNOTES: Perform symbol and dead code strippings on linked binaries generated by ObjC rules.

--
MOS_MIGRATED_REVID=96551473

6 files changed, 53 insertions, 161 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/BinaryLinkingTargetFactory.java b/src/main/java/com/google/devtools/build/lib/rules/objc/BinaryLinkingTargetFactory.java
index b1a4bce67d..b59bbe30c9 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/BinaryLinkingTargetFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/BinaryLinkingTargetFactory.java
@@ -77,9 +77,8 @@ abstract class BinaryLinkingTargetFactory implements RuleConfiguredTargetFactory
         ObjcRuleClasses.intermediateArtifacts(ruleContext);
 
     XcodeProvider.Builder xcodeProviderBuilder = new XcodeProvider.Builder();
-    NestedSetBuilder<Artifact> filesToBuild =
-        NestedSetBuilder.<Artifact>stableOrder()
-            .add(intermediateArtifacts.strippedSingleArchitectureBinary());
+    NestedSetBuilder<Artifact> filesToBuild = NestedSetBuilder.<Artifact>stableOrder()
+        .add(intermediateArtifacts.singleArchitectureBinary());
 
     new CompilationSupport(ruleContext)
         .registerJ2ObjcCompileAndArchiveActions(objcProvider)
@@ -166,27 +165,24 @@ abstract class BinaryLinkingTargetFactory implements RuleConfiguredTargetFactory
     CompilationArtifacts compilationArtifacts =
         CompilationSupport.compilationArtifacts(ruleContext);
 
-    ObjcCommon.Builder builder =
-        new ObjcCommon.Builder(ruleContext)
-            .setCompilationAttributes(new CompilationAttributes(ruleContext))
-            .setResourceAttributes(new ResourceAttributes(ruleContext))
-            .setCompilationArtifacts(compilationArtifacts)
-            .addDefines(ruleContext.getTokenizedStringListAttr("defines"))
-            .addDepObjcProviders(
-                ruleContext.getPrerequisites("deps", Mode.TARGET, ObjcProvider.class))
-            .addDepCcHeaderProviders(
-                ruleContext.getPrerequisites("deps", Mode.TARGET, CppCompilationContext.class))
-            .addDepCcLinkProviders(
-                ruleContext.getPrerequisites("deps", Mode.TARGET, CcLinkParamsProvider.class))
-            .addDepObjcProviders(
-                ruleContext.getPrerequisites("bundles", Mode.TARGET, ObjcProvider.class))
-            .addNonPropagatedDepObjcProviders(
-                ruleContext.getPrerequisites(
-                    "non_propagated_deps", Mode.TARGET, ObjcProvider.class))
-            .setIntermediateArtifacts(intermediateArtifacts)
-            .setAlwayslink(false)
-            .addExtraImportLibraries(ObjcRuleClasses.j2ObjcLibraries(ruleContext))
-            .setLinkedBinary(intermediateArtifacts.strippedSingleArchitectureBinary());
+    ObjcCommon.Builder builder = new ObjcCommon.Builder(ruleContext)
+        .setCompilationAttributes(new CompilationAttributes(ruleContext))
+        .setResourceAttributes(new ResourceAttributes(ruleContext))
+        .setCompilationArtifacts(compilationArtifacts)
+        .addDefines(ruleContext.getTokenizedStringListAttr("defines"))
+        .addDepObjcProviders(ruleContext.getPrerequisites("deps", Mode.TARGET, ObjcProvider.class))
+        .addDepCcHeaderProviders(
+            ruleContext.getPrerequisites("deps", Mode.TARGET, CppCompilationContext.class))
+        .addDepCcLinkProviders(
+            ruleContext.getPrerequisites("deps", Mode.TARGET, CcLinkParamsProvider.class))
+        .addDepObjcProviders(
+            ruleContext.getPrerequisites("bundles", Mode.TARGET, ObjcProvider.class))
+        .addNonPropagatedDepObjcProviders(
+            ruleContext.getPrerequisites("non_propagated_deps", Mode.TARGET, ObjcProvider.class))
+        .setIntermediateArtifacts(intermediateArtifacts)
+        .setAlwayslink(false)
+        .addExtraImportLibraries(ObjcRuleClasses.j2ObjcLibraries(ruleContext))
+        .setLinkedBinary(intermediateArtifacts.singleArchitectureBinary());
 
     if (ObjcRuleClasses.objcConfiguration(ruleContext).generateDebugSymbols()) {
       builder.setBreakpadFile(intermediateArtifacts.breakpadSym());
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/CompilationSupport.java b/src/main/java/com/google/devtools/build/lib/rules/objc/CompilationSupport.java
index 1f346ed7a7..9aaffb63b2 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/CompilationSupport.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/CompilationSupport.java
@@ -33,7 +33,6 @@ import static com.google.devtools.build.lib.rules.objc.ObjcRuleClasses.CLANG_PLU
 import static com.google.devtools.build.lib.rules.objc.ObjcRuleClasses.DSYMUTIL;
 import static com.google.devtools.build.lib.rules.objc.ObjcRuleClasses.NON_ARC_SRCS_TYPE;
 import static com.google.devtools.build.lib.rules.objc.ObjcRuleClasses.SRCS_TYPE;
-import static com.google.devtools.build.lib.rules.objc.ObjcRuleClasses.STRIP;
 import static com.google.devtools.build.lib.rules.objc.ObjcRuleClasses.SWIFT;
 
 import com.google.common.annotations.VisibleForTesting;
@@ -52,9 +51,7 @@ import com.google.devtools.build.lib.analysis.actions.CommandLine;
 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.config.CompilationMode;
 import com.google.devtools.build.lib.collect.nestedset.NestedSet;
-import com.google.devtools.build.lib.packages.TargetUtils;
 import com.google.devtools.build.lib.rules.cpp.LinkerInputs;
 import com.google.devtools.build.lib.rules.java.J2ObjcConfiguration;
 import com.google.devtools.build.lib.rules.objc.ObjcCommon.CompilationAttributes;
@@ -381,15 +378,8 @@ final class CompilationSupport {
   }
 
   /**
-   * Registers any actions necessary to link this rule and its dependencies.
-   *
-   * <p>Dsym bundle and breakpad files are generated if
-   * {@link ObjcConfiguration#generateDebugSymbols()} is set.
-   *
-   * <p>When Bazel flag {@code --compilation_mode=opt} is specified, additional optimizations
-   * will be performed on the linked binary: all-symbol stripping (using {@code /usr/bin/strip})
-   * and dead-code stripping (using linker flags: {@code -dead_strip} and
-   * {@code -no_dead_strip_inits_and_terms}).
+   * Registers any actions necessary to link this rule and its dependencies. Debug symbols are
+   * generated if {@link ObjcConfiguration#generateDebugSymbols()} is set.
    *
    * @param objcProvider common information about this rule's attributes and its dependencies
    * @param extraLinkArgs any additional arguments to pass to the linker
@@ -415,19 +405,8 @@ final class CompilationSupport {
 
   private void registerLinkAction(ObjcProvider objcProvider, ExtraLinkArgs extraLinkArgs,
       Iterable<Artifact> extraLinkInputs, Optional<Artifact> dsymBundle) {
-    ObjcConfiguration objcConfiguration = ObjcRuleClasses.objcConfiguration(ruleContext);
-    IntermediateArtifacts intermediateArtifacts =
-        ObjcRuleClasses.intermediateArtifacts(ruleContext);
-    CompilationMode compilationMode = objcConfiguration.getCompilationMode();
-
-    // When compilation_mode=opt, the unstripped binary containing debug symbols is generated by the
-    // linker, which also needs the debug symbols for dead-code removal. The binary is also used to
-    // generate dSYM bundle if --objc_generate_debug_symbol is specified. A symbol strip action is
-    // later registered to strip the symbol table from the unstripped binary.
-    Artifact binaryToLink =
-        compilationMode == CompilationMode.OPT
-            ? intermediateArtifacts.unstrippedSingleArchitectureBinary()
-            : intermediateArtifacts.strippedSingleArchitectureBinary();
+    Artifact linkedBinary =
+        ObjcRuleClasses.intermediateArtifacts(ruleContext).singleArchitectureBinary();
 
     ImmutableList<Artifact> ccLibraries = ccLibraries(objcProvider);
     ruleContext.registerAction(
@@ -435,8 +414,8 @@ final class CompilationSupport {
             .setMnemonic("ObjcLink")
             .setShellCommand(ImmutableList.of("/bin/bash", "-c"))
             .setCommandLine(
-                linkCommandLine(extraLinkArgs, objcProvider, binaryToLink, dsymBundle, ccLibraries))
-            .addOutput(binaryToLink)
+                linkCommandLine(extraLinkArgs, objcProvider, linkedBinary, dsymBundle, ccLibraries))
+            .addOutput(linkedBinary)
             .addOutputs(dsymBundle.asSet())
             .addTransitiveInputs(objcProvider.get(LIBRARY))
             .addTransitiveInputs(objcProvider.get(IMPORTED_LIBRARY))
@@ -444,24 +423,6 @@ final class CompilationSupport {
             .addInputs(ccLibraries)
             .addInputs(extraLinkInputs)
             .build(ruleContext));
-
-    if (compilationMode == CompilationMode.OPT) {
-      // For test targets, only debug symbols are stripped off, since /usr/bin/strip is not able
-      // to strip off all symbols in XCTest bundle.
-      boolean isTestTarget = TargetUtils.isTestRule(ruleContext.getRule());
-      Iterable<String> stripArgs =
-          isTestTarget ? ImmutableList.of("-S") : ImmutableList.<String>of();
-      Artifact strippedBinary = intermediateArtifacts.strippedSingleArchitectureBinary();
-
-      ruleContext.registerAction(
-          ObjcRuleClasses.spawnOnDarwinActionBuilder()
-              .setMnemonic("ObjcBinarySymbolStrip")
-              .setExecutable(STRIP)
-              .setCommandLine(symbolStripCommandLine(stripArgs, binaryToLink, strippedBinary))
-              .addOutput(strippedBinary)
-              .addInput(binaryToLink)
-              .build(ruleContext));
-    }
   }
 
   private ImmutableList<Artifact> ccLibraries(ObjcProvider objcProvider) {
@@ -472,15 +433,6 @@ final class CompilationSupport {
     return ccLibraryBuilder.build();
   }
 
-  private static CommandLine symbolStripCommandLine(
-      Iterable<String> extraFlags, Artifact unstrippedArtifact, Artifact strippedArtifact) {
-    return CustomCommandLine.builder()
-        .add(extraFlags)
-        .addExecPath("-o", strippedArtifact)
-        .addPath(unstrippedArtifact.getExecPath())
-        .build();
-  }
-
   private CommandLine linkCommandLine(ExtraLinkArgs extraLinkArgs,
       ObjcProvider objcProvider, Artifact linkedBinary, Optional<Artifact> dsymBundle,
       ImmutableList<Artifact> ccLibraries) {
@@ -495,20 +447,10 @@ final class CompilationSupport {
     } else {
       commandLine.addPath(CLANG);
     }
-
-    // Do not perform code stripping on tests because XCTest binary is linked not as an executable
-    // but as a bundle without any entry point.
-    boolean isTestTarget = TargetUtils.isTestRule(ruleContext.getRule());
-    if (objcConfiguration.getCompilationMode() == CompilationMode.OPT && !isTestTarget) {
-      commandLine.add("-dead_strip").add("-no_dead_strip_inits_and_terms");
-    }
-
     commandLine
         .add(IosSdkCommands.commonLinkAndCompileFlagsForClang(objcProvider, objcConfiguration))
-        .add("-Xlinker")
-        .add("-objc_abi_version")
-        .add("-Xlinker")
-        .add("2")
+        .add("-Xlinker").add("-objc_abi_version")
+        .add("-Xlinker").add("2")
         .add("-fobjc-link-runtime")
         .add(IosSdkCommands.DEFAULT_LINKER_FLAGS)
         .addBeforeEach("-framework", frameworkNames(objcProvider))
@@ -715,50 +657,22 @@ final class CompilationSupport {
   private CompilationSupport registerDsymActions() {
     IntermediateArtifacts intermediateArtifacts =
         ObjcRuleClasses.intermediateArtifacts(ruleContext);
-    ObjcConfiguration objcConfiguration = ObjcRuleClasses.objcConfiguration(ruleContext);
-    CompilationMode compilationMode = objcConfiguration.getCompilationMode();
-
     Artifact dsymBundle = intermediateArtifacts.dsymBundle();
-    Artifact linkedBinary =
-        compilationMode == CompilationMode.OPT
-            ? intermediateArtifacts.unstrippedSingleArchitectureBinary()
-            : intermediateArtifacts.strippedSingleArchitectureBinary();
     Artifact debugSymbolFile = intermediateArtifacts.dsymSymbol();
-    Artifact dsymPlist = intermediateArtifacts.dsymPlist();
-
-    PathFragment dsymOutputDir =
-        replaceSuffix(
-            dsymBundle.getExecPath(), IntermediateArtifacts.TMP_DSYM_BUNDLE_SUFFIX, ".app.dSYM");
-    PathFragment dsymPlistZipEntry = dsymPlist.getExecPath().relativeTo(dsymOutputDir);
-    PathFragment debugSymbolFileZipEntry =
-        debugSymbolFile
-            .getExecPath()
-            .replaceName(linkedBinary.getFilename())
-            .relativeTo(dsymOutputDir);
-
-    StringBuilder unzipDsymCommand = new StringBuilder();
-    unzipDsymCommand
-        .append(
-            String.format(
-                "unzip -p %s %s > %s",
-                dsymBundle.getExecPathString(),
-                dsymPlistZipEntry,
-                dsymPlist.getExecPathString()))
-        .append(
-            String.format(
-                " && unzip -p %s %s > %s",
-                dsymBundle.getExecPathString(),
-                debugSymbolFileZipEntry,
-                debugSymbolFile.getExecPathString()));
-
-    ruleContext.registerAction(
-        new SpawnAction.Builder()
-            .setMnemonic("UnzipDsym")
-            .setShellCommand(unzipDsymCommand.toString())
-            .addInput(dsymBundle)
-            .addOutput(dsymPlist)
-            .addOutput(debugSymbolFile)
-            .build(ruleContext));
+    ruleContext.registerAction(new SpawnAction.Builder()
+        .setMnemonic("UnzipDsym")
+        .setProgressMessage("Unzipping dSYM file: " + ruleContext.getLabel())
+        .setExecutable(new PathFragment("/usr/bin/unzip"))
+        .addInput(dsymBundle)
+        .setCommandLine(CustomCommandLine.builder()
+            .add(dsymBundle.getExecPathString())
+            .add("-d")
+            .add(stripSuffix(dsymBundle.getExecPathString(),
+                IntermediateArtifacts.TMP_DSYM_BUNDLE_SUFFIX) + ".app.dSYM")
+            .build())
+        .addOutput(intermediateArtifacts.dsymPlist())
+        .addOutput(debugSymbolFile)
+        .build(ruleContext));
 
     Artifact dumpsyms = ruleContext.getPrerequisiteArtifact(":dumpsyms", Mode.HOST);
     Artifact breakpadFile = intermediateArtifacts.breakpadSym();
@@ -777,14 +691,9 @@ final class CompilationSupport {
     return this;
   }
 
-  private PathFragment replaceSuffix(PathFragment path, String suffix, String newSuffix) {
+  private String stripSuffix(String str, String suffix) {
     // TODO(bazel-team): Throw instead of returning null?
-    String name = path.getBaseName();
-    if (name.endsWith(suffix)) {
-      return path.replaceName(name.substring(0, name.length() - suffix.length()) + newSuffix);
-    } else {
-      return null;
-    }
+    return str.endsWith(suffix) ? str.substring(0, str.length() - suffix.length()) : null;
   }
 
   /**
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/IntermediateArtifacts.java b/src/main/java/com/google/devtools/build/lib/rules/objc/IntermediateArtifacts.java
index 133745b364..b52de70399 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/IntermediateArtifacts.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/IntermediateArtifacts.java
@@ -119,24 +119,13 @@ final class IntermediateArtifacts {
 
   /**
    * The artifact which is the binary (or library) which is comprised of one or more .a files linked
-   * together. Compared to the artifact returned by {@link #unstrippedSingleArchitectureBinary},
-   * this artifact is stripped of symbol table when --compilation_mode=opt is specified.
+   * together.
    */
-  public Artifact strippedSingleArchitectureBinary() {
+  public Artifact singleArchitectureBinary() {
     return appendExtension("_bin");
   }
 
   /**
-   * The artifact which is the binary (or library) which is comprised of one or more .a files linked
-   * together. It also contains full debug symbol information, compared to the artifact returned
-   * by {@link #strippedSingleArchitectureBinary}. This artifact will serve as input for the symbol
-   * strip action and is only created when --compilation_mode=opt is specified.
-   */
-  public Artifact unstrippedSingleArchitectureBinary() {
-    return appendExtension("_bin_unstripped");
-  }
-
-  /**
    * Lipo binary generated by combining one or more linked binaries. This binary is the one included
    * in generated bundles and invoked as entry point to the application.
    */
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcConfiguration.java b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcConfiguration.java
index 9c303cda5c..c16d632090 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcConfiguration.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcConfiguration.java
@@ -49,8 +49,7 @@ public class ObjcConfiguration extends BuildConfiguration.Fragment {
 
   @VisibleForTesting
   static final ImmutableList<String> OPT_COPTS =
-      ImmutableList.of(
-          "-Os", "-DNDEBUG=1", "-Wno-unused-variable", "-Winit-self", "-Wno-extra", "-g");
+      ImmutableList.of("-Os", "-DNDEBUG=1", "-Wno-unused-variable", "-Winit-self", "-Wno-extra");
 
   private final String iosSdkVersion;
   private final String iosMinimumOs;
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcRuleClasses.java b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcRuleClasses.java
index 5207f1ecf0..8e41f35aad 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcRuleClasses.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcRuleClasses.java
@@ -67,7 +67,6 @@ public class ObjcRuleClasses {
   static final PathFragment LIPO = new PathFragment(BIN_DIR + "/lipo");
   static final PathFragment IBTOOL = new PathFragment(IosSdkCommands.IBTOOL_PATH);
   static final PathFragment SWIFT_STDLIB_TOOL = new PathFragment(BIN_DIR + "/swift-stdlib-tool");
-  static final PathFragment STRIP = new PathFragment(BIN_DIR + "/strip");
 
   private static final PathFragment JAVA = new PathFragment("/usr/bin/java");
 
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/ReleaseBundlingSupport.java b/src/main/java/com/google/devtools/build/lib/rules/objc/ReleaseBundlingSupport.java
index d2d1a9e2c7..e10dee00f6 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/ReleaseBundlingSupport.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/ReleaseBundlingSupport.java
@@ -217,7 +217,7 @@ public final class ReleaseBundlingSupport {
     } else {
       maybeSignedIpa = registerBundleSigningActions(ipaOutput);
     }
-
+    
     registerEmbedLabelPlistAction();
 
     BundleMergeControlBytes bundleMergeControlBytes = new BundleMergeControlBytes(
@@ -417,8 +417,8 @@ public final class ReleaseBundlingSupport {
     } else {
       extraBundleFiles = ImmutableList.of();
     }
-
-    String primaryBundleId = null;
+    
+    String primaryBundleId = null; 
     String fallbackBundleId = null;
 
     if (ruleContext.attributes().isAttributeValueExplicitlySpecified("bundle_id")) {
@@ -463,7 +463,7 @@ public final class ReleaseBundlingSupport {
     NestedSetBuilder<Artifact> linkedBinariesBuilder = NestedSetBuilder.<Artifact>stableOrder()
         .addTransitive(attributes.dependentLinkedBinaries());
     if (linkedBinary == LinkedBinary.LOCAL_AND_DEPENDENCIES) {
-      linkedBinariesBuilder.add(intermediateArtifacts.strippedSingleArchitectureBinary());
+      linkedBinariesBuilder.add(intermediateArtifacts.singleArchitectureBinary());
     }
     return linkedBinariesBuilder.build();
   }
@@ -700,14 +700,14 @@ public final class ReleaseBundlingSupport {
         .add("Frameworks")
         .addPath(ObjcRuleClasses.SWIFT_STDLIB_TOOL)
         .add("--platform").add(IosSdkCommands.swiftPlatform(objcConfiguration))
-        .addExecPath("--scan-executable", intermediateArtifacts.strippedSingleArchitectureBinary());
+        .addExecPath("--scan-executable", intermediateArtifacts.singleArchitectureBinary());
 
     ruleContext.registerAction(
         ObjcRuleClasses.spawnJavaOnDarwinActionBuilder(attributes.swiftStdlibToolDeployJar())
             .setMnemonic("SwiftStdlibCopy")
             .setCommandLine(commandLine.build())
             .addOutput(intermediateArtifacts.swiftFrameworksFileZip())
-            .addInput(intermediateArtifacts.strippedSingleArchitectureBinary())
+            .addInput(intermediateArtifacts.singleArchitectureBinary())
             .build(ruleContext));
   }