Fix unwinder visibility for most ABIs.

When we switched to using a linker script for libgcc in r19 we broke
`-Wl,--exclude-libs,libgcc.a`, because `--exclude-libs` has no effect
on the contents of linker scripts. As such, libgcc was not being
hidden as expected.

The good news is that on the one platform this causes the most trouble
(Arm32, since it uses the LLVM unwinder for exception handling rather
than libgcc) this didn't cause any issues. Since libunwind was linked
first (and the `--exclude-libs` worked there, since the actual library
was named), the libgcc unwind symbols did not end up in the output at
all.

So while this has not caused any issues, it is still a good idea for
us to link libgcc hidden to guard against any future unwind ABI
breaks.

Test: Added a test that checks ndk-build output with readelf
Bug: android/ndk#1092
Change-Id: I5c8203afd29f0b7b272a51aadfc1a5ca6cbd33e4

build/cmake/android.toolchain.cmake

@@ -347,7 +347,7 @@ if(ANDROID_LD STREQUAL lld)
 endif()
 
 # Don't re-export libgcc symbols in every binary.
-list(APPEND ANDROID_LINKER_FLAGS -Wl,--exclude-libs,libgcc.a)
+list(APPEND ANDROID_LINKER_FLAGS -Wl,--exclude-libs,libgcc_real.a)
 list(APPEND ANDROID_LINKER_FLAGS -Wl,--exclude-libs,libatomic.a)
 
 # STL.

build/core/default-build-commands.mk

@@ -84,7 +84,7 @@ endef
 
 cmd-strip = $(PRIVATE_STRIP) $(PRIVATE_STRIP_MODE) $(call host-path,$1)
 
-TARGET_LIBGCC = -lgcc -Wl,--exclude-libs,libgcc.a
+TARGET_LIBGCC = -lgcc -Wl,--exclude-libs,libgcc_real.a
 TARGET_LIBATOMIC = -latomic -Wl,--exclude-libs,libatomic.a
 TARGET_LDLIBS := -lc -lm
 

docs/BuildSystemMaintainers.md

@@ -416,9 +416,10 @@ preserved. By default, only symbols in used sections will be included in the
 linked binary.
 
 If this behavior is not desired for your build system, ensure that these flags
-are at least used for libgcc.a and libunwind.a (libunwind is only used for
-ARM32). This is necessary to avoid unwinding bugs on ARM32. See [Unwinding] for
-more information.
+are at least used for `libgcc_real.a` (`libgcc.a` is a linker script, and
+`--exclude-libs` does not have any effect on the contents of linker scripts) and
+`libunwind.a` (libunwind is only used for ARM32). This is necessary to avoid
+unwinding bugs on ARM32. See [Unwinding] for more information.
 
 [visibility]: https://gcc.gnu.org/wiki/Visibility
 
@@ -518,8 +519,8 @@ these symbols from a shared library. If this library was built with the wrong
 unwinder, it is possible for one unwinder to call into the other. As they are
 not compatible, this will likely result in either a crash or a failed unwind. To
 avoid this problem, libraries should always be built with
-`-Wl,--exclude-libs,libgcc.a` and `-Wl,--exclude-libs,libunwind.a` (the latter
-is only necessary for 32-bit ARM) to ensure that unwind symbols are not
+`-Wl,--exclude-libs,libgcc_real.a` and `-Wl,--exclude-libs,libunwind.a` (the
+latter is only necessary for 32-bit ARM) to ensure that unwind symbols are not
 re-exported from shared libraries.
 
 Even with the above precautions, it is still possible for an improperly built

docs/changelogs/Changelog-r21.md

@@ -93,10 +93,15 @@ For Android Studio issues, follow the docs on the [Android Studio site].
    of limited use today. Also note that the above code will behave correctly
    even on pre-r21 because calling an undefined function in make returns the
    empty string, so the else case will be taken.
+ * [Issue 1092]: Fixed hiding of unwinder symbols in outputs of ndk-build and
+   CMake. Maintainers of third-party build systems should apply similar fixes
+   when using NDK r19 and above to guard against possible future compatibility
+   issues.
 
 [FORTIFY in Android]: https://android-developers.googleblog.com/2017/04/fortify-in-android.html
 [Issue 1004]: https://github.com/android-ndk/ndk/issues/1004
 [Issue 1028]: https://github.com/android/ndk/issues/1028
+[Issue 1092]: https://github.com/android/ndk/issues/1092
 [Issue 744]: https://github.com/android/ndk/issues/744
 [Issue 855]: https://github.com/android-ndk/ndk/issues/855
 [Issue 859]: https://github.com/android-ndk/ndk/issues/859

ndk/hosts.py

@@ -17,7 +17,6 @@
 from __future__ import absolute_import
 
 import enum
-import os
 import sys
 
 
@@ -51,11 +50,7 @@ def get_host_tag(ndk_path: str) -> str:
     elif sys.platform == 'darwin':
         return 'darwin-x86_64'
     elif sys.platform == 'win32':
-        host_tag = 'windows-x86_64'
-        test_path = os.path.join(ndk_path, 'prebuilt', host_tag)
-        if not os.path.exists(test_path):
-            host_tag = 'windows'
-        return host_tag
+        return 'windows-x86_64'
     raise ValueError('Unknown host: {}'.format(sys.platform))
 
 

ndk/test/types.py

@@ -193,6 +193,7 @@ def run(self, obj_dir: str, _dist_dir: str,
         _prep_build_dir(self.test_dir, build_dir)
         with ndk.ext.os.cd(build_dir):
             module = imp.load_source('test', 'test.py')
+            assert self.platform is not None
             success, failure_message = module.run_test(  # type: ignore
                 self.ndk_path, self.abi, self.platform, self.config.linker, self.ndk_build_flags)
             if success:

tests/build/unwinder_hidden/project/jni/Android.mk

@@ -0,0 +1,7 @@
+LOCAL_PATH := $(call my-dir)
+
+include $(CLEAR_VARS)
+LOCAL_MODULE := foo
+LOCAL_SRC_FILES := foo.cpp
+LOCAL_CPPFLAGS := -fexceptions -frtti
+include $(BUILD_SHARED_LIBRARY)

tests/build/unwinder_hidden/project/jni/Application.mk

@@ -0,0 +1 @@
+APP_STL := c++_static

tests/build/unwinder_hidden/project/jni/foo.cpp

@@ -0,0 +1,3 @@
+void foo() {
+  throw 42;
+}

tests/build/unwinder_hidden/test.py

@@ -0,0 +1,82 @@
+#
+# Copyright (C) 2018 The Android Open Source Project
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+"""Check for correct link order from ndk-build.
+"""
+from pathlib import Path
+import re
+import subprocess
+from typing import Iterator, List, Optional, Tuple
+
+from ndk.abis import Abi
+import ndk.hosts
+from ndk.toolchains import LinkerOption
+
+
+def find_public_unwind_symbols(output: str) -> Iterator[str]:
+    """Returns an iterator over readelf lines with unwind symbols in them."""
+    #   274: 00000000000223d8     8 FUNC    GLOBAL DEFAULT   11 _Unwind_GetIP
+    # Group 1: Visibility
+    # Group 2: Name
+    readelf_regex = re.compile(r'^.*?(\S+)\s+\d+\s+(\S+)$')
+    for line in output.splitlines():
+        match = readelf_regex.match(line)
+        if match is None:
+            continue
+        visibility, name = match.groups()
+        if name.startswith('_Unwind') and visibility == 'DEFAULT':
+            yield name
+
+
+def readelf(ndk_path: Path, host: ndk.hosts.Host, library: Path,
+            *args: str) -> str:
+    """Runs readelf, returning the output."""
+    readelf_path = (ndk_path / 'toolchains/llvm/prebuilt' /
+                    ndk.hosts.get_host_tag(str(ndk_path)) / 'bin/llvm-readelf')
+    if host.is_windows:
+        readelf_path = readelf_path.with_suffix('.exe')
+
+    return subprocess.run(
+        [str(readelf_path), *args, str(library)],
+        check=True,
+        encoding='utf-8',
+        stdout=subprocess.PIPE,
+        stderr=subprocess.STDOUT).stdout
+
+
+def run_test(ndk_path: str, abi: Abi, platform: Optional[int],
+             linker: LinkerOption, build_flags: List[str]) -> Tuple[bool, str]:
+    """Check that unwinder symbols are hidden in outputs."""
+    ndk_build = Path(ndk_path) / 'ndk-build'
+    host = ndk.hosts.get_default_host()
+    if host.is_windows:
+        ndk_build = ndk_build.with_suffix('.cmd')
+    project_path = Path('project')
+    ndk_args = build_flags + [
+        f'APP_ABI={abi}',
+        f'APP_LD={linker.value}',
+        f'APP_PLATFORM=android-{platform}',
+    ]
+    subprocess.run(
+        [str(ndk_build), '-C', str(project_path)] + ndk_args,
+        check=True,
+        stdout=subprocess.PIPE,
+        stderr=subprocess.STDOUT)
+
+    library = project_path / 'libs' / str(abi) / 'libfoo.so'
+    readelf_output = readelf(Path(ndk_path), host, library, '-sW')
+    for symbol in find_public_unwind_symbols(readelf_output):
+        return False, f'Found public unwind symbol: {symbol}'
+    return True, ''