Do not pass --gc-sections for debug builds.

The user may want to call "unused" functions during debugging.

This fix does not cover the new cmake toolchain. afaict CMake does not
give us the control needed to fix this bug without entirely reverting
the feature in that mode (we can control linker flags, but not per
mode).

Bug: android/ndk#1813
Test: expanded the gc-sections test
Change-Id: I095e2e70f522f4dbd5021f0efab49dfd86cfba51

build/cmake/android-legacy.toolchain.cmake

@@ -332,6 +332,9 @@ set(ANDROID_COMPILER_FLAGS_DEBUG)
 set(ANDROID_COMPILER_FLAGS_RELEASE)
 set(ANDROID_LINKER_FLAGS)
 set(ANDROID_LINKER_FLAGS_EXE)
+set(ANDROID_LINKER_FLAGS_RELEASE)
+set(ANDROID_LINKER_FLAGS_RELWITHDEBINFO)
+set(ANDROID_LINKER_FLAGS_MINSIZEREL)
 
 # STL.
 set(ANDROID_CXX_STANDARD_LIBRARIES)
@@ -347,7 +350,7 @@ elseif(ANDROID_STL STREQUAL none)
   list(APPEND ANDROID_COMPILER_FLAGS_CXX "-nostdinc++")
   list(APPEND ANDROID_LINKER_FLAGS "-nostdlib++")
 else()
-  message(FATAL_ERROR "Invalid Android STL: ${ANDROID_STL}.")
+  message(FATAL_ERROR "Invalid STL: ${ANDROID_STL}.")
 endif()
 
 if(CMAKE_HOST_SYSTEM_NAME STREQUAL Linux)
@@ -459,8 +462,12 @@ if (NOT ANDROID_ALLOW_UNDEFINED_VERSION_SCRIPT_SYMBOLS)
 endif()
 
 list(APPEND ANDROID_LINKER_FLAGS -Wl,--fatal-warnings)
-list(APPEND ANDROID_LINKER_FLAGS -Wl,--gc-sections)
-list(APPEND ANDROID_LINKER_FLAGS_EXE -Wl,--gc-sections)
+
+# --gc-sections should not be present for debug builds because that can strip
+# functions that the user may want to evaluate while debugging.
+list(APPEND ANDROID_LINKER_FLAGS_RELEASE -Wl,--gc-sections)
+list(APPEND ANDROID_LINKER_FLAGS_RELWITHDEBINFO -Wl,--gc-sections)
+list(APPEND ANDROID_LINKER_FLAGS_MINSIZEREL -Wl,--gc-sections)
 
 # Debug and release flags.
 list(APPEND ANDROID_COMPILER_FLAGS_RELEASE -O3)
@@ -544,6 +551,9 @@ string(REPLACE ";" " " ANDROID_COMPILER_FLAGS_DEBUG   "${ANDROID_COMPILER_FLAGS_
 string(REPLACE ";" " " ANDROID_COMPILER_FLAGS_RELEASE "${ANDROID_COMPILER_FLAGS_RELEASE}")
 string(REPLACE ";" " " ANDROID_LINKER_FLAGS           "${ANDROID_LINKER_FLAGS}")
 string(REPLACE ";" " " ANDROID_LINKER_FLAGS_EXE       "${ANDROID_LINKER_FLAGS_EXE}")
+string(REPLACE ";" " " ANDROID_LINKER_FLAGS_RELEASE   "${ANDROID_LINKER_FLAGS_RELEASE}")
+string(REPLACE ";" " " ANDROID_LINKER_FLAGS_RELWITHDEBINFO "${ANDROID_LINKER_FLAGS_RELWITHDEBINFO}")
+string(REPLACE ";" " " ANDROID_LINKER_FLAGS_MINSIZEREL "${ANDROID_LINKER_FLAGS_MINSIZEREL}")
 
 if(ANDROID_CCACHE)
   set(CMAKE_C_COMPILER_LAUNCHER   "${ANDROID_CCACHE}")
@@ -603,6 +613,15 @@ set(CMAKE_ASM_FLAGS_RELEASE   "${ANDROID_COMPILER_FLAGS_RELEASE} ${CMAKE_ASM_FLA
 set(CMAKE_SHARED_LINKER_FLAGS "${ANDROID_LINKER_FLAGS} ${CMAKE_SHARED_LINKER_FLAGS}")
 set(CMAKE_MODULE_LINKER_FLAGS "${ANDROID_LINKER_FLAGS} ${CMAKE_MODULE_LINKER_FLAGS}")
 set(CMAKE_EXE_LINKER_FLAGS    "${ANDROID_LINKER_FLAGS} ${ANDROID_LINKER_FLAGS_EXE} ${CMAKE_EXE_LINKER_FLAGS}")
+set(CMAKE_SHARED_LINKER_FLAGS_RELEASE "${ANDROID_LINKER_FLAGS_RELEASE} ${CMAKE_SHARED_LINKER_FLAGS_RELEASE}")
+set(CMAKE_MODULE_LINKER_FLAGS_RELEASE "${ANDROID_LINKER_FLAGS_RELEASE} ${CMAKE_MODULE_LINKER_FLAGS_RELEASE}")
+set(CMAKE_EXE_LINKER_FLAGS_RELEASE    "${ANDROID_LINKER_FLAGS_RELEASE} ${CMAKE_EXE_LINKER_FLAGS_RELEASE}")
+set(CMAKE_SHARED_LINKER_FLAGS_RELWITHDEBINFO "${ANDROID_LINKER_FLAGS_RELWITHDEBINFO} ${CMAKE_SHARED_LINKER_FLAGS_RELWITHDEBINFO}")
+set(CMAKE_MODULE_LINKER_FLAGS_RELWITHDEBINFO "${ANDROID_LINKER_FLAGS_RELWITHDEBINFO} ${CMAKE_MODULE_LINKER_FLAGS_RELWITHDEBINFO}")
+set(CMAKE_EXE_LINKER_FLAGS_RELWITHDEBINFO    "${ANDROID_LINKER_FLAGS_RELWITHDEBINFO} ${CMAKE_EXE_LINKER_FLAGS_RELWITHDEBINFO}")
+set(CMAKE_SHARED_LINKER_FLAGS_MINSIZEREL "${ANDROID_LINKER_FLAGS_MINSIZEREL} ${CMAKE_SHARED_LINKER_FLAGS_MINSIZEREL}")
+set(CMAKE_MODULE_LINKER_FLAGS_MINSIZEREL "${ANDROID_LINKER_FLAGS_MINSIZEREL} ${CMAKE_MODULE_LINKER_FLAGS_MINSIZEREL}")
+set(CMAKE_EXE_LINKER_FLAGS_MINSIZEREL    "${ANDROID_LINKER_FLAGS_MINSIZEREL} ${CMAKE_EXE_LINKER_FLAGS_MINSIZEREL}")
 
 # Compatibility for read-only variables.
 # Read-only variables for compatibility with the other toolchain file.

build/cmake/flags.cmake

@@ -55,6 +55,9 @@ if (NOT ANDROID_ALLOW_UNDEFINED_VERSION_SCRIPT_SYMBOLS)
 endif()
 
 string(APPEND _ANDROID_NDK_INIT_LDFLAGS " -Wl,--fatal-warnings")
+# This should only be set for release modes, but CMake doesn't provide a way for
+# us to be that specific in the new toolchain file.
+# https://github.com/android/ndk/issues/1813
 string(APPEND _ANDROID_NDK_INIT_LDFLAGS " -Wl,--gc-sections")
 string(APPEND _ANDROID_NDK_INIT_LDFLAGS_EXE " -Wl,--gc-sections")
 

build/core/default-build-commands.mk

@@ -43,7 +43,6 @@ TARGET_DISABLE_FORMAT_STRING_CFLAGS := -Wno-error=format-security
 
 define cmd-build-shared-library
 $(PRIVATE_CXX) \
-    -Wl,--gc-sections \
     -Wl,-soname,$(notdir $(LOCAL_BUILT_MODULE)) \
     -shared \
     $(PRIVATE_LINKER_OBJECTS_AND_LIBRARIES) \
@@ -59,7 +58,6 @@ endef
 # this buggy behavior.
 define cmd-build-executable
 $(PRIVATE_CXX) \
-    -Wl,--gc-sections \
     -Wl,-rpath-link=$(call host-path,$(PRIVATE_SYSROOT_API_LIB_DIR)) \
     -Wl,-rpath-link=$(call host-path,$(TARGET_OUT)) \
     $(PRIVATE_LINKER_OBJECTS_AND_LIBRARIES) \
@@ -128,6 +126,10 @@ GLOBAL_LDFLAGS = \
     -target $(LLVM_TRIPLE)$(TARGET_PLATFORM_LEVEL) \
     -no-canonical-prefixes \
 
+ifeq ($(APP_OPTIM),release)
+    GLOBAL_LDFLAGS += -Wl,--gc-sections
+endif
+
 GLOBAL_CXXFLAGS = $(GLOBAL_CFLAGS) -fno-exceptions -fno-rtti
 
 TARGET_CFLAGS =

docs/changelogs/Changelog-r25.md

@@ -20,6 +20,16 @@ directly, see the [build system maintainers guide].
 
 [Issue 1751]: https://github.com/android/ndk/issues/1751
 
+## r25c
+
+* [Issue 1813]: `-Wl,--gc-sections` is no longer set by default for debug
+  builds. This behavior was removed because it could cause the linker to remove
+  functions that may be useful to evaluate during debugging. The new CMake
+  toolchain file (`-DANDROID_USE_LEGACY_TOOLCHAIN_FILE=OFF`, not the default
+  behavior) does not include this fix because it requires a CMake fix first.
+
+[Issue 1813]: https://github.com/android/ndk/issues/1813
+
 ## r25b
 
 * [Issue 1739]: Fixed C compatibility issue in `amidi/AMidi.h`.

ndk/testing/flag_verifier.py

@@ -36,9 +36,15 @@ def failed(self) -> bool:
         """Returns True if verification failed."""
         raise NotImplementedError
 
-    def make_test_result_tuple(self) -> tuple[bool, Optional[str]]:
+    def make_test_result_tuple(
+        self, message_prefix: str | None = None
+    ) -> tuple[bool, Optional[str]]:
         """Creates a test result tuple in the format expect by run_test."""
-        return not self.failed(), self.error_message
+        if message_prefix is None:
+            message = self.error_message
+        else:
+            message = f"{message_prefix}\n{self.error_message}"
+        return not self.failed(), message
 
 
 class FlagVerifierSuccess(FlagVerifierResult):

tests/build/gc_sections/test.py

@@ -13,9 +13,13 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 #
-"""Check that -Wl,--gc-sections is used.
+"""Check that -Wl,--gc-sections is used, but only on release builds.
+
+This flag should not be present for debug builds because that can strip functions that
+the user may want to evaluate while debugging.
 
 https://github.com/android/ndk/issues/1717
+https://github.com/android/ndk/issues/1813
 """
 from pathlib import Path
 from typing import Optional
@@ -27,5 +31,37 @@
 def run_test(ndk_path: str, config: BuildConfiguration) -> tuple[bool, Optional[str]]:
     """Checks correct --gc-sections use."""
     verifier = FlagVerifier(Path("project"), Path(ndk_path), config)
+    verifier.with_cmake_flag("-DCMAKE_BUILD_TYPE=Release")
+    verifier.with_ndk_build_flag("APP_DEBUG=false")
     verifier.expect_flag("-Wl,--gc-sections")
-    return verifier.verify().make_test_result_tuple()
+    passed, message = verifier.verify().make_test_result_tuple(
+        "With -DCMAKE_BUILD_TYPE=Release and APP_DEBUG=false"
+    )
+    if not passed:
+        return passed, message
+
+    verifier = FlagVerifier(Path("project"), Path(ndk_path), config)
+    verifier.with_cmake_flag("-DCMAKE_BUILD_TYPE=RelWithDebInfo")
+    verifier.expect_flag("-Wl,--gc-sections")
+    passed, message = verifier.verify_cmake().make_test_result_tuple(
+        "With -DCMAKE_BUILD_TYPE=RelWithDebInfo"
+    )
+    if not passed:
+        return passed, message
+
+    verifier = FlagVerifier(Path("project"), Path(ndk_path), config)
+    verifier.with_cmake_flag("-DCMAKE_BUILD_TYPE=MinSizeRel")
+    verifier.expect_flag("-Wl,--gc-sections")
+    passed, message = verifier.verify_cmake().make_test_result_tuple(
+        "With -DCMAKE_BUILD_TYPE=MinSizeRel"
+    )
+    if not passed:
+        return passed, message
+
+    verifier = FlagVerifier(Path("project"), Path(ndk_path), config)
+    verifier.with_cmake_flag("-DCMAKE_BUILD_TYPE=Debug")
+    verifier.with_ndk_build_flag("APP_DEBUG=true")
+    verifier.expect_not_flag("-Wl,--gc-sections")
+    return verifier.verify().make_test_result_tuple(
+        "With -DCMAKE_BUILD_TYPE=Debug and APP_DEBUG=true"
+    )

tests/build/gc_sections/test_config.py

@@ -0,0 +1,8 @@
+from ndk.test.buildtest.case import Test
+from ndk.test.spec import CMakeToolchainFile
+
+
+def build_broken(test: Test) -> tuple[str | None, str | None]:
+    if test.config.toolchain_file is CMakeToolchainFile.Default:
+        return "new CMake toolchain", "https://github.com/android/ndk/issues/1813"
+    return None, None