Merge "Do not pass --gc-sections for debug builds." into snap-temp-L14400000958252147snap-temp-L14400000958252147

7 files changed, 93 insertions, 9 deletions

diff --git a/build/cmake/android-legacy.toolchain.cmake b/build/cmake/android-legacy.toolchain.cmake
index 3a0dd7131..b0d875812 100644
--- a/build/cmake/android-legacy.toolchain.cmake
+++ b/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)
@@ -350,7 +353,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)
@@ -453,8 +456,12 @@ if(ANDROID_PLATFORM_LEVEL LESS 30)
 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)
@@ -538,6 +545,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}")
@@ -597,6 +607,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.
diff --git a/build/cmake/flags.cmake b/build/cmake/flags.cmake
index 387f93eea..6af7e0018 100644
--- a/build/cmake/flags.cmake
+++ b/build/cmake/flags.cmake
@@ -46,6 +46,9 @@ if(CMAKE_SYSTEM_VERSION LESS 30)
 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")
 
diff --git a/build/core/default-build-commands.mk b/build/core/default-build-commands.mk
index 98ee438fa..208d78a16 100644
--- a/build/core/default-build-commands.mk
+++ b/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) \
@@ -123,6 +121,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 =
diff --git a/docs/changelogs/Changelog-r25.md b/docs/changelogs/Changelog-r25.md
index ef0c538e8..463873423 100644
--- a/docs/changelogs/Changelog-r25.md
+++ b/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`.
diff --git a/ndk/testing/flag_verifier.py b/ndk/testing/flag_verifier.py
index cb514a26c..163b6a622 100644
--- a/ndk/testing/flag_verifier.py
+++ b/ndk/testing/flag_verifier.py
@@ -36,9 +36,15 @@ class FlagVerifierResult:
         """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):
diff --git a/tests/build/gc_sections/test.py b/tests/build/gc_sections/test.py
index 419cae12b..8bb436a58 100644
--- a/tests/build/gc_sections/test.py
+++ b/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 @@ from ndk.testing.flag_verifier import FlagVerifier
 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"
+    )
diff --git a/tests/build/gc_sections/test_config.py b/tests/build/gc_sections/test_config.py
new file mode 100644
index 000000000..869627d3a
--- /dev/null
+++ b/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