Auto populate the namespace list to rename in desktop packaging (#1000)

* Change desktop packaging to automatically get the list of C++ namespaces to rename.

This prevents some namespaces from accidentally being omitted. Instead,
the script will rename all namespaces except for std:: and __gnu_cxx.

* Add additional namespaces to ignore.

* Add readme note

* After packaging, scan the libraries to ensure no leaking namespaces.

Also upgrade demumble to 1.2.2 which handles more MSVC demangling cases.

* Add missing parallel install

* Fix a bug in merge_libraries that did not rename all symbols.

* Fix a bug that failed to find namespaces with tildes.

* Also parse `RTTI Class Heirarchy Descriptor` on windows.
Fix windows BFD target.

* Add '$' to top-level namespace search for Darwin.

This fixes grpc_resource_quota_arg_vtable::$_0::__invoke

* Fix renaming of Windows symbols with RTTI info, etc.

* Add tests for merge_libraries script.

* Fix Windows symbol renaming for odd C++ symbols like RTTI descriptors

* Ignore underscores as well to fix a Windows packging issue

* Don't detect namespaces from external symbols.

* Don't auto-add namespaces with underscores.

* Fix namespace detection to only rename top-level; add test.

Remove strict_cpp as it should no longer be required for Windows.

* Add test for add_automatic_namespaces

* Allow the list of allowed namespaces to be determined programatically.

Author: jonsimantov

.github/workflows/cpp-packaging.yml

@@ -28,8 +28,8 @@ on:
 
 env:
   # Packaging prerequisites
-  # Demumble 1.1.0 released Nov 13, 2018
-  demumbleVer: "1.1.0"
+  # Demumble version from March 22, 2022
+  demumbleVer: "df938e45c2b0e064fb5323d88b692d03b451d271"
   # Use SHA256 for hashing files.
   hashCommand: "sha256sum"
   # Xcode version 13.3.1 is the version we build the SDK with.
@@ -94,6 +94,11 @@ jobs:
         if: runner.os == 'macOS'
         run: sudo xcode-select -s /Applications/Xcode_${{ env.xcodeVersion }}.app/Contents/Developer
 
+      - name: Setup python
+        uses: actions/setup-python@v2
+        with:
+          python-version: 3.7
+
       - name: Fetch and build binutils
         run: |
           set +e
@@ -154,7 +159,7 @@ jobs:
         with:
           repository: nico/demumble
           path: demumble-src
-          ref: v${{ env.demumbleVer }}
+          ref: ${{ env.demumbleVer }}
 
       - name: build demumble
         run: |
@@ -544,6 +549,9 @@ jobs:
           cd sdk-src
           python scripts/gha/install_prereqs_desktop.py --ssl boringssl
           cd ..
+          if [[ $(uname) == "Darwin"* ]]; then
+            brew install parallel
+          fi
 
       - name: postprocess and package built SDK
         run: |
@@ -605,6 +613,48 @@ jobs:
           name: firebase-cpp-sdk-${{ matrix.sdk_platform }}${{ matrix.suffix}}-package
           path: firebase-cpp-sdk-${{ matrix.sdk_platform }}${{ matrix.suffix}}-package.tgz
 
+      - name: Check for unrenamed namespaces
+        shell: bash
+        run: |
+          set +e
+          if [[ "${{ matrix.sdk_platform }}" == "darwin" ]]; then
+            nm=bin/llvm-nm
+          elif [[ "${{ matrix.sdk_platform }}" == "windows" ]]; then
+            if [[ "${{ matrix.sdk_platform }}" == *"x64"* ]]; then
+              nm="bin/nm --target pe-x86-64"
+            else
+              nm="bin/nm --target pe-i386"
+            fi
+          else  # sdk_platform = linux
+            nm=bin/nm
+          fi
+          ${nm} `find firebase-cpp-sdk-*-package/libs/${{ matrix.sdk_platform }} -type f` | grep '^[0-9a-fA-F][0-9a-fA-F]* ' | cut -d' ' -f3- > raw_symbols.txt
+          if [[ "${{ matrix.sdk_platform }}" == "windows" ]]; then
+            cat raw_symbols.txt | sort | uniq | bin/demumble | grep '^[a-zA-Z_]*::' > cpp_symbols.txt
+          elif [[ "${{ matrix.sdk_platform }}" == "darwin" ]]; then
+            # remove leading _ on mach-o symbols
+            cat raw_symbols.txt | sort | uniq | sed 's/^_//' | bin/c++filt | grep '^[a-zA-Z_]*::' > cpp_symbols.txt
+          else  # linux
+            cat raw_symbols.txt | sort | uniq | bin/c++filt | grep '^[a-zA-Z_]*::' > cpp_symbols.txt
+          fi
+          # cpp_symbols.txt contains a list of all C++ symbols, sorted and deduped
+          # get a list of all top-level namespaces (except system namespaces)
+          cat cpp_symbols.txt | cut -d: -f1 | sort | uniq > cpp_namespaces.txt
+          echo "List of all namespaces found:"
+          cat cpp_namespaces.txt
+          # Filter out renamed namespaces (f_b_*), standard namespaces, and the public namespace (firebase::).
+          # These are all specified in the respective scripts, package.sh and merge_libraries.py
+          echo $(sdk-src/build_scripts/desktop/package.sh -R)'.*' > allow_namespaces.txt
+          sdk-src/build_scripts/desktop/package.sh -N >> allow_namespaces.txt
+          python sdk-src/scripts/print_allowed_namespaces.py >> allow_namespaces.txt
+          allow_namespaces=$(cat allow_namespaces.txt | tr '\n' '|')
+          cat cpp_namespaces.txt | grep -vE "^(${allow_namespaces})$" > extra_namespaces.txt
+          # If there are any namespaces in this file, print an error.
+          if [[ -s extra_namespaces.txt ]]; then
+            echo '::error ::Unrenamed C++ namespaces in ${{ matrix.sdk_platform }}${{ matrix.suffix }} package:%0A'$(cat extra_namespaces.txt | tr '\n' ' ' | sed 's/ /%0A/g')
+            exit 1
+          fi
+
       - name: cleanup build artifacts
         if: |
           (

build_scripts/desktop/package.sh

@@ -19,6 +19,8 @@ options:
   -j, run merge_libraries jobs in parallel
   -v, enable verbose mode
   -L, use LLVM binutils
+  -R, print rename prefix and exit
+  -N, print allowed namespaces and exit
 example:
   build_scripts/desktop/package.sh -b firebase-cpp-sdk-linux -p linux -o package_out -v x86 -j"
 }
@@ -42,6 +44,11 @@ use_llvm_binutils=0
 
 readonly SUPPORTED_PLATFORMS=(linux windows darwin)
 
+# String to prepend to all hidden symbols.
+readonly rename_string=f_b_
+# Comma-separated list of c++ namespaces to allow.
+readonly allow_cpp_namespaces=firebase
+
 abspath(){
     if [[ -d $1 ]]; then
         echo "$(cd "$1"; pwd -P)"
@@ -50,7 +57,7 @@ abspath(){
     fi
 }
 
-while getopts "f:b:o:p:d:m:P:t:hjLv" opt; do
+while getopts "f:b:o:p:d:m:P:t:NRhjLv" opt; do
     case $opt in
         f)
             binutils_format=$OPTARG
@@ -94,6 +101,14 @@ while getopts "f:b:o:p:d:m:P:t:hjLv" opt; do
         t)
             tools_path=$OPTARG
             ;;
+        N)
+            echo "${allow_cpp_namespaces}" | tr ',' '\n'
+            exit 0
+            ;;
+        R)
+            echo "${rename_string}"
+            exit 0
+            ;;
         h)
             usage
             exit 0
@@ -208,32 +223,6 @@ readonly deps_hidden_firebase_firestore="
 */firestore-build/*/grpc-build/third_party/re2/*${subdir}${prefix}re2.${ext}
 "
 
-# List of C++ namespaces to be renamed, so as to not conflict with the
-# developer's own dependencies.
-readonly -a rename_namespaces=(flatbuffers flexbuffers reflection ZLib bssl uWS absl google
-                               base_raw_logging ConnectivityWatcher grpc
-                               grpc_access_token_credentials grpc_alts_credentials
-                               grpc_alts_server_credentials grpc_auth_context
-                               grpc_channel_credentials grpc_channel_security_connector
-                               grpc_chttp2_hpack_compressor grpc_chttp2_stream grpc_chttp2_transport
-                               grpc_client_security_context grpc_composite_call_credentials
-                               grpc_composite_channel_credentials grpc_core grpc_deadline_state
-                               grpc_google_default_channel_credentials grpc_google_iam_credentials
-                               grpc_google_refresh_token_credentials grpc_impl grpc_local_credentials
-                               grpc_local_server_credentials grpc_md_only_test_credentials
-                               grpc_message_compression_algorithm_for_level
-                               grpc_oauth2_token_fetcher_credentials grpc_plugin_credentials
-                               grpc_server_credentials grpc_server_security_connector
-                               grpc_server_security_context
-                               grpc_service_account_jwt_access_credentials grpc_ssl_credentials
-                               grpc_ssl_server_credentials grpc_tls_credential_reload_config
-                               grpc_tls_server_authorization_check_config GrpcUdpListener leveldb
-                               leveldb_filterpolicy_create_bloom leveldb_writebatch_iterate strings
-                               TlsCredentials TlsServerCredentials tsi snappy re2)
-
-# String to prepend to all hidden symbols.
-readonly rename_string=f_b_
-
 readonly demangle_cmds=${tools_path}/c++filt,${tools_path}/demumble
 if [[ ${use_llvm_binutils} -eq 1 ]]; then
     readonly binutils_objcopy=${tools_path}/llvm-objcopy
@@ -260,14 +249,11 @@ merge_libraries_params=(
     --binutils_objcopy_cmd=${binutils_objcopy}
     --demangle_cmds=${demangle_cmds}
     --platform=${platform}
-    --hide_cpp_namespaces=$(echo "${rename_namespaces[*]}" | sed 's| |,|g')
+    --auto_hide_cpp_namespaces
+    --ignore_cpp_namespaces="${allow_cpp_namespaces}"
 )
 cache_param=--cache=${cache_file}
 
-if [[ ${platform} == "windows" ]]; then
-    # Windows has a hard time with strict C++ demangling.
-    merge_libraries_params+=(--nostrict_cpp)
-fi
 if [[ ${verbose} -eq 1 ]]; then
     merge_libraries_params+=(--verbosity=3)
 fi

release_build_files/readme.md

@@ -634,6 +634,12 @@ workflow use only during the development of your app, not for publicly shipping
 code.
 
 ## Release Notes
+### Upcoming Release
+-   Changes
+    - General (Desktop): Fixed an issue with embedded dependencies that could
+      cause duplicate symbol linker errors in conjunction with other libraries
+      ([#989](https://github.com/firebase/firebase-cpp-sdk/issues/989)).
+
 ### 9.3.0
 -   Changes
     - General (Android,Linux): Fixed a concurrency bug where waiting for an

scripts/merge_libraries.py

@@ -104,8 +104,10 @@
                     "will autodetect target format. If you want to specify "
                     "different input and output formats, separate them with a comma.")
 
-# Never rename 'std::' by default when --auto_hide_cpp_namespaces is enabled.
-IMPLICIT_CPP_NAMESPACES_TO_IGNORE = {"std"}
+# Never rename system namespaces by default when --auto_hide_cpp_namespaces is enabled.
+IMPLICIT_CPP_NAMESPACES_TO_IGNORE = {"std", "type_info",
+                                     "__gnu_cxx", "stdext",
+                                     "cxxabiv1", "__cxxabiv1"}
 
 DEFAULT_ENCODING = "ascii"
 
@@ -416,7 +418,7 @@ def get_top_level_namespaces(demangled_symbols):
   # It will specifically not match second-level or deeper namespaces:
   #   matched_namespace::unmatched_namespace::Class::Method()
   regex_top_level_namespaces = re.compile(
-      r"(?<![a-zA-Z0-9_])(?<!::)([a-zA-Z_][a-zA-Z0-9_]*)::(?=[a-zA-Z_])")
+      r"(?<![a-zA-Z0-9_])(?<!::)([a-zA-Z_~\$][a-zA-Z0-9_]*)::(?=[a-zA-Z_~` \$])")
   found_namespaces = set()
   for cppsymbol in demangled_symbols:
     m = regex_top_level_namespaces.findall(cppsymbol)
@@ -441,6 +443,8 @@ def add_automatic_namespaces(symbols):
   logging.debug("Scanning for top-level namespaces.")
   demangled_symbols = [demangle_symbol(symbol) for symbol in symbols]
   hide_namespaces = get_top_level_namespaces(demangled_symbols)
+  # strip out top-level namespaces beginning with an underscore
+  hide_namespaces = set([n for n in hide_namespaces if n[0] != "_"])
   ignore_namespaces = set(FLAGS.ignore_cpp_namespaces)
   ignore_namespaces.update(IMPLICIT_CPP_NAMESPACES_TO_IGNORE)
   logging.debug("Ignoring namespaces: %s", " ".join(
@@ -533,25 +537,29 @@ def read_symbols_from_archive(archive_file):
   return (defined_symbols, all_symbols)
 
 
-def symbol_includes_cpp_namespace(cpp_symbol, namespace):
+def symbol_includes_top_level_cpp_namespace(cpp_symbol, namespace):
   """Returns true if the C++ symbol contains the namespace in it.
 
   This means the symbol is within the namespace, or the namespace as
   an argument type, return type, template type, etc.
 
+  If FLAGS.strict_cpp == True, this will only return true if the namespace
+  is at the top level of the symbol.
+
   Args:
     cpp_symbol: C++ symbol to check.
     namespace: Namespace to look for in the C++ symbol.
   Returns:
-    True if the symbol includes the C++ namespace in some way, False otherwise.
+    True if the symbol includes the C++ namespace at the top level (or anywhere,
+    if FLAGS.strict_cpp == False), False otherwise.
   """
   # Early out if the namespace isn't in the mangled symbol.
   if namespace not in cpp_symbol:
     return False
   if not FLAGS.strict_cpp:
     # If we aren't being fully strict about C++ symbol renaming,
     # we can use this placeholder method.
-    if FLAGS.platform == "windows" and ("@%s@@" % namespace) in cpp_symbol:
+    if FLAGS.platform == "windows" and re.search(r"[^a-z_]%s@@" % namespace, cpp_symbol):
       return True
     elif (FLAGS.platform != "windows" and
           ("%d%s" % (len(namespace), namespace)) in cpp_symbol):
@@ -566,7 +574,8 @@ def symbol_includes_cpp_namespace(cpp_symbol, namespace):
     return True
   # Or, check if the demangled symbol has "namespace::" preceded by a non-
   # alphanumeric character. This avoids a false positive on "notmynamespace::".
-  regex = re.compile("[^0-9a-zA-Z_]%s::" % namespace)
+  # Also don't allow a namespace :: right before the name.
+  regex = re.compile("[^0-9a-zA-Z_:]%s::" % namespace)
   if re.search(regex, demangled):
     return True
 
@@ -626,7 +635,7 @@ def rename_symbol(symbol):
     new_symbol = symbol
     if FLAGS.platform in ["linux", "android", "darwin", "ios"]:
       for ns in FLAGS.hide_cpp_namespaces:
-        if symbol_includes_cpp_namespace(symbol, ns):
+        if symbol_includes_top_level_cpp_namespace(symbol, ns):
           # Linux and Darwin: To rename "namespace" to "prefixnamespace",
           # change all instances of "9namespace" to "15prefixnamespace".
           # (the number is the length of the namespace name)
@@ -637,12 +646,12 @@ def rename_symbol(symbol):
       new_renames[symbol] = new_symbol
     elif FLAGS.platform == "windows":
       for ns in FLAGS.hide_cpp_namespaces:
-        if symbol_includes_cpp_namespace(symbol, ns):
+        if symbol_includes_top_level_cpp_namespace(symbol, ns):
           # Windows: To rename "namespace" to "prefixnamespace",
-          # change all instances of "@namespace@@" to "@prefixnamespace@@".
+          # change all instances of "[^a-z_]namespace@@" to "[^a-z]prefixnamespace@@",
           # See https://msdn.microsoft.com/en-us/library/56h2zst2.aspx
           new_ns = FLAGS.rename_string + ns
-          new_symbol = re.sub("@%s@@" % ns, "@%s@@" % new_ns, new_symbol)
+          new_symbol = re.sub(r"(?<=[^a-z_])%s@@" % ns, r"%s@@" % new_ns, new_symbol)
       new_renames[symbol] = new_symbol
   else:
     if FLAGS.platform == "windows" and symbol.startswith("$LN"):
@@ -949,6 +958,7 @@ def main(argv):
       # Scan through all input libraries for C++ symbols matching any of the
       # hide_cpp_namespaces.
       cpp_symbols = set()
+      all_defined_symbols = set()
       for input_path in input_paths + additional_input_paths:
         logging.debug("Scanning for C++ symbols in %s", input_path[1])
         if os.path.abspath(input_path[1]) in _cache["symbols"]:
@@ -962,10 +972,11 @@ def main(argv):
         cpp_symbols.update(all_symbols
                            if FLAGS.rename_external_cpp_symbols
                            else defined_symbols)
+        all_defined_symbols.update(defined_symbols)
 
       # If we are set to scan for namespaces, do that now.
       if FLAGS.auto_hide_cpp_namespaces:
-        add_automatic_namespaces(defined_symbols)
+        add_automatic_namespaces(all_defined_symbols)
 
       for symbol in cpp_symbols:
         if is_cpp_symbol(symbol):