Make -c opt dead strip code for Apple builds by default.

Previously `objc_enable_binary_stripping` and `-c opt` were required to both
symbol strip and dead strip code. Symbol stripping and dead code stripping are very
different things. Dead code stripping is the elimination of dead code and should be done
in an optimized build. Unused code can be marked as "not dead" by using `exported symbols`
in the Apple build rules. Symbol stripping is the removal of symbol tables from an
executable.

`objc_enable_binary_stripping` now only controls symbol stripping.

Eventually I hope we deprecate `objc_enable_binary_stripping` and just use `strip=`.

Closes #13489.

PiperOrigin-RevId: 375023891

Author: dmaclach

src/main/java/com/google/devtools/build/lib/rules/objc/CompilationSupport.java

@@ -552,10 +552,11 @@ private FeatureConfiguration getFeatureConfiguration(
             .addAll(OBJC_ACTIONS)
             .add(CppRuleClasses.LANG_OBJC);
 
-    if (configuration.getFragment(ObjcConfiguration.class).shouldStripBinary()) {
+    ObjcConfiguration objcConfiguration = configuration.getFragment(ObjcConfiguration.class);
+    if (objcConfiguration.shouldDeadCodeStripBinary()) {
       activatedCrosstoolSelectables.add(DEAD_STRIP_FEATURE_NAME);
     }
-    if (configuration.getFragment(ObjcConfiguration.class).generateLinkmap()) {
+    if (objcConfiguration.generateLinkmap()) {
       activatedCrosstoolSelectables.add(GENERATE_LINKMAP_FEATURE_NAME);
     }
     // Add a feature identifying the Xcode version so CROSSTOOL authors can enable flags for
@@ -1148,9 +1149,9 @@ private StrippingType getStrippingType(ExtraLinkArgs extraLinkArgs) {
    * <p>Dsym bundle is generated if {@link ObjcConfiguration#generateDsym()} is set.
    *
    * <p>When Bazel flags {@code --compilation_mode=opt} and {@code --objc_enable_binary_stripping}
-   * are 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}).
+   * are specified, symbol stripping (using {@code /usr/bin/strip}) occurs. When Bazel {@code
+   * --compilation_mode=opt} is specified dead-code stripping (using linker flags: {@code
+   * -dead_strip}) occurs.
    *
    * @param objcProvider common information about this rule's attributes and its dependencies
    * @param ccLinkingContexts the linking contexts from this rule's dependencies

src/main/java/com/google/devtools/build/lib/rules/objc/ObjcCommandLineOptions.java

@@ -145,9 +145,8 @@ public class ObjcCommandLineOptions extends FragmentOptions {
       documentationCategory = OptionDocumentationCategory.OUTPUT_PARAMETERS,
       effectTags = {OptionEffectTag.ACTION_COMMAND_LINES},
       help =
-          "Whether to perform symbol and dead-code strippings on linked binaries. Binary "
-              + "strippings will be performed if both this flag and --compilation_mode=opt are "
-              + "specified.")
+          "Whether to perform symbol stripping on linked binaries. Stripping will be performed if"
+              + " both this flag and --compilation_mode=opt are specified.")
   public boolean enableBinaryStripping;
 
   @Option(

src/main/java/com/google/devtools/build/lib/rules/objc/ObjcConfiguration.java

@@ -188,14 +188,23 @@ public ImmutableList<String> getCopts() {
   }
 
   /**
-   * Returns whether to perform symbol and dead-code strippings on linked binaries. The strippings
-   * are performed iff --compilation_mode=opt and --objc_enable_binary_stripping are specified.
+   * Returns whether to perform symbol stripping on linked binaries. The stripping is performed iff
+   * --compilation_mode=opt and --objc_enable_binary_stripping are specified.
    */
   @Override
   public boolean shouldStripBinary() {
     return this.enableBinaryStripping && getCompilationMode() == CompilationMode.OPT;
   }
 
+  /**
+   * Returns whether to perform dead code elimination on linked binaries. The elimination is
+   * performed iff --compilation_mode=opt.
+   */
+  @Override
+  public boolean shouldDeadCodeStripBinary() {
+    return getCompilationMode() == CompilationMode.OPT;
+  }
+
   /**
    * Returns the flag-supplied certificate name to be used in signing or {@code null} if no such
    * certificate was specified.

src/main/java/com/google/devtools/build/lib/starlarkbuildapi/apple/ObjcConfigurationApi.java

@@ -106,9 +106,15 @@ public interface ObjcConfigurationApi<ApplePlatformTypeApiT extends ApplePlatfor
   @StarlarkMethod(
       name = "should_strip_binary",
       structField = true,
-      doc = "Returns whether to perform symbol and dead-code strippings on linked binaries.")
+      doc = "Returns whether to perform symbol stripping on linked binaries.")
   boolean shouldStripBinary();
 
+  @StarlarkMethod(
+      name = "should_dead_code_strip_binary",
+      structField = true,
+      doc = "Returns whether to perform dead code elimination on linked binaries.")
+  boolean shouldDeadCodeStripBinary();
+
   @StarlarkMethod(
       name = "signing_certificate_name",
       structField = true,

src/test/shell/bazel/apple/bazel_objc_test.sh

@@ -118,7 +118,8 @@ EOF
       fail "Timestamp of contents of archive file should be zero"
 }
 
-function test_strip_symbols() {
+# Verifies that unused code is dead stripped with `-c opt`.
+function test_symbols_dead_stripped_opt() {
   setup_objc_test_support
 
   rm -rf ios
@@ -131,9 +132,94 @@ int addOne(int num);
 int addOne(int num) {
   return num + 1;
 }
- int main(int argc, char *argv[]) {
+int main(int argc, char *argv[]) {
+  NSAutoreleasePool * pool = [[NSAutoreleasePool alloc] init];
+  int retVal = UIApplicationMain(argc, argv, nil, nil);
+  [pool release];
+  return retVal;
+}
+EOF
+
+  cat >ios/BUILD <<EOF
+apple_binary(name = 'app',
+             deps = [':main'],
+             platform_type = 'ios')
+objc_library(name = 'main',
+             non_arc_srcs = ['main.m'])
+EOF
+
+  bazel build --verbose_failures \
+      --apple_platform_type=ios \
+      --ios_sdk_version=$IOS_SDK_VERSION \
+      --compilation_mode=opt \
+      //ios:app >$TEST_log 2>&1 || fail "should pass"
+  ls bazel-out/apl-ios_x86_64-opt/bin/ios/app_lipobin \
+    || fail "should generate lipobin (stripped binary)"
+  ! nm bazel-out/apl-ios_x86_64-opt/bin/ios/app_lipobin | grep addOne \
+    || fail "should fail to find symbol addOne"
+}
+
+# Verifies that unused code is *not* dead stripped with `-c dbg`.
+function test_symbols_not_dead_stripped_dbg() {
+  setup_objc_test_support
+
+  rm -rf ios
+  mkdir -p ios
+
+  cat >ios/main.m <<EOF
+#import <UIKit/UIKit.h>
+
+/* function declaration */
+int addOne(int num);
+int addOne(int num) {
+  return num + 1;
+}
+int main(int argc, char *argv[]) {
+  NSAutoreleasePool * pool = [[NSAutoreleasePool alloc] init];
+  int retVal = UIApplicationMain(argc, argv, nil, nil);
+  [pool release];
+  return retVal;
+}
+EOF
+
+  cat >ios/BUILD <<EOF
+apple_binary(name = 'app',
+             deps = [':main'],
+             platform_type = 'ios')
+objc_library(name = 'main',
+             non_arc_srcs = ['main.m'])
+EOF
+
+  bazel build --verbose_failures \
+      --apple_platform_type=ios \
+      --ios_sdk_version=$IOS_SDK_VERSION \
+      --compilation_mode=dbg \
+      //ios:app >$TEST_log 2>&1 || fail "should pass"
+  ls bazel-out/apl-ios_x86_64-dbg/bin/ios/app_lipobin \
+    || fail "should generate lipobin"
+  nm bazel-out/apl-ios_x86_64-dbg/bin/ios/app_lipobin | grep addOne \
+    || fail "should find symbol addOne"
+}
+
+# Verifies that symbols are stripped with `objc_enable_binary_stripping` and `-c opt`.
+function test_symbols_stripped_opt_with_objc_enable_binary_stripping() {
+  setup_objc_test_support
+
+  rm -rf ios
+  mkdir -p ios
+
+  cat >ios/main.m <<EOF
+#import <UIKit/UIKit.h>
+
+/* function declaration */
+int addOne(int num) __attribute__((noinline));
+int addOne(int num) {
+  return num + 1;
+}
+int main(int argc, char *argv[]) {
   NSAutoreleasePool * pool = [[NSAutoreleasePool alloc] init];
   int retVal = UIApplicationMain(argc, argv, nil, nil);
+  NSLog(@"%d", addOne(2));
   [pool release];
   return retVal;
 }
@@ -159,6 +245,94 @@ EOF
     || fail "should fail to find symbol addOne"
 }
 
+# Verifies that symbols are not stripped with just `-c opt`.
+function test_symbols_not_stripped_opt() {
+  setup_objc_test_support
+
+  rm -rf ios
+  mkdir -p ios
+
+  cat >ios/main.m <<EOF
+#import <UIKit/UIKit.h>
+
+/* function declaration */
+int addOne(int num) __attribute__((noinline));
+int addOne(int num) {
+  return num + 1;
+}
+int main(int argc, char *argv[]) {
+  NSAutoreleasePool * pool = [[NSAutoreleasePool alloc] init];
+  int retVal = UIApplicationMain(argc, argv, nil, nil);
+  NSLog(@"%d", addOne(2));
+  [pool release];
+  return retVal;
+}
+EOF
+
+  cat >ios/BUILD <<EOF
+apple_binary(name = 'app',
+             deps = [':main'],
+             platform_type = 'ios')
+objc_library(name = 'main',
+             non_arc_srcs = ['main.m'])
+EOF
+
+  bazel build --verbose_failures \
+      --apple_platform_type=ios \
+      --ios_sdk_version=$IOS_SDK_VERSION \
+      --compilation_mode=opt \
+      //ios:app >$TEST_log 2>&1 || fail "should pass"
+  ls bazel-out/apl-ios_x86_64-opt/bin/ios/app_lipobin \
+    || fail "should generate lipobin (stripped binary)"
+  nm bazel-out/apl-ios_x86_64-opt/bin/ios/app_lipobin | grep addOne \
+    || fail "should fail to find symbol addOne"
+}
+
+
+# Verifies that symbols are not stripped with `objc_enable_binary_stripping` and `-c dbg`.
+function test_symbols_not_stripped_dbg() {
+  setup_objc_test_support
+
+  rm -rf ios
+  mkdir -p ios
+
+  cat >ios/main.m <<EOF
+#import <UIKit/UIKit.h>
+
+/* function declaration */
+int addOne(int num) __attribute__((noinline));
+int addOne(int num) {
+  return num + 1;
+}
+int main(int argc, char *argv[]) {
+  NSAutoreleasePool * pool = [[NSAutoreleasePool alloc] init];
+  int retVal = UIApplicationMain(argc, argv, nil, nil);
+  NSLog(@"%d", addOne(2));
+  [pool release];
+  return retVal;
+}
+EOF
+
+  cat >ios/BUILD <<EOF
+apple_binary(name = 'app',
+             deps = [':main'],
+             platform_type = 'ios')
+objc_library(name = 'main',
+             non_arc_srcs = ['main.m'])
+EOF
+
+  bazel build --verbose_failures \
+      --apple_platform_type=ios \
+      --ios_sdk_version=$IOS_SDK_VERSION \
+      --objc_enable_binary_stripping=true \
+      --compilation_mode=dbg \
+      //ios:app >$TEST_log 2>&1 || fail "should pass"
+  ls bazel-out/apl-ios_x86_64-dbg/bin/ios/app_lipobin \
+    || fail "should generate lipobin"
+  nm bazel-out/apl-ios_x86_64-dbg/bin/ios/app_lipobin | grep addOne \
+    || fail "should find symbol addOne"
+}
+
 function test_cc_test_depending_on_objc() {
   setup_objc_test_support