Merge pull request #2469 from jorisv/topic/hidden_symbols

Hide all symbols by default

Author: jorisv

CHANGELOG.md

@@ -9,6 +9,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 ### Added
 - Add `pinocchio_python_parser` target ([#2475](https://github.com/stack-of-tasks/pinocchio/pull/2475))
 
+### Changed
+- On GNU/Linux and macOS, hide all symbols by default ([#2469](https://github.com/stack-of-tasks/pinocchio/pull/2469))
+
 ## [3.3.0] - 2024-11-06
 
 ### Added

bindings/python/CMakeLists.txt

@@ -53,6 +53,7 @@ function(PINOCCHIO_PYTHON_BINDINGS_SPECIFIC_TYPE scalar_name)
     ${PYTHON_LIB_NAME}
     PROPERTIES PREFIX ""
                DEFINE_SYMBOL "${PYWRAP}_EXPORTS"
+               CXX_VISIBILITY_PRESET hidden
                VISIBILITY_INLINES_HIDDEN ON)
 
   # Do not report:
@@ -67,10 +68,10 @@ function(PINOCCHIO_PYTHON_BINDINGS_SPECIFIC_TYPE scalar_name)
   target_compile_options(${PYTHON_LIB_NAME} PRIVATE ${PRIVATE_OPTIONS})
 
   set(PINOCCHIO_PYTHON_CONTEXT_FILE_VALUE "pinocchio/bindings/python/context/${scalar_name}.hpp")
-  target_compile_options(
+  target_compile_definitions(
     ${PYTHON_LIB_NAME}
-    PRIVATE -DPINOCCHIO_PYTHON_CONTEXT_FILE="${PINOCCHIO_PYTHON_CONTEXT_FILE_VALUE}"
-            -DPINOCCHIO_PYTHON_MODULE_NAME=${PYTHON_LIB_NAME})
+    PRIVATE PINOCCHIO_PYTHON_CONTEXT_FILE="${PINOCCHIO_PYTHON_CONTEXT_FILE_VALUE}"
+            PINOCCHIO_PYTHON_MODULE_NAME=${PYTHON_LIB_NAME})
 
   set_target_properties(${PYTHON_LIB_NAME} PROPERTIES VERSION ${PROJECT_VERSION})
   if(BUILD_WITH_COMMIT_VERSION)
@@ -183,6 +184,14 @@ if(BUILD_PYTHON_INTERFACE)
 
   if(BUILD_WITH_CODEGEN_SUPPORT)
     pinocchio_python_bindings_specific_type(cppadcg cppadcg)
+
+    # On osx, use default visiblitiy because of an issue with thread_local storage defined in
+    # cppad/cg/cg.hpp header (see
+    # https://github.com/stack-of-tasks/pinocchio/pull/2469#issuecomment-2461845127)
+    if(APPLE)
+      set_target_properties(${PYWRAP}_cppadcg PROPERTIES CXX_VISIBILITY_PRESET default)
+    endif()
+
     # CPPAD_DEBUG_AND_RELEASE allow to mix debug and release versions of CppAD in the same program.
     target_compile_definitions(
       ${PYWRAP}_cppadcg PRIVATE PYCPPAD_EXCLUDE_EIGEN_NUMTRAITS_SPECIALIZATION

development/scripts/misc/common_symbols.py

@@ -0,0 +1,48 @@
+#! /usr/bin/env python3
+# Find common dynamics public symbols between shared libraries.
+
+import argparse
+import itertools
+import pathlib
+import subprocess
+import typing
+
+
+def generate_symbols(shared_library: pathlib.Path) -> typing.Set[str]:
+    # Show symbol
+    # -D: Dynamic
+    # -C: Demangled
+    # -U: Defined
+    # -W: Non weak
+    result = subprocess.run(
+        ["nm", "-DCUW", "--format=sysv", str(shared_library)],
+        capture_output=True,
+        text=True,
+    )
+    output = result.stdout
+    lines_split = (line.split("|") for line in output.splitlines() if "|" in line)
+    # Only keep lines with exported (upper case) symbols.
+    # `u` is also a global symbol, but if we always build with compatible libraries,
+    # there is no issue to find it in many places.
+    return set([line[0].strip() for line in lines_split if line[2].strip().isupper()])
+
+
+if __name__ == "__main__":
+    parser = argparse.ArgumentParser(
+        prog="common_symbol",
+        description="Find common dynamics public symbols between shared libraries.",
+    )
+    parser.add_argument("shared_libraries", nargs="+", type=pathlib.Path)
+
+    args = parser.parse_args()
+    symbols = [
+        (shared_library, generate_symbols(shared_library))
+        for shared_library in args.shared_libraries
+    ]
+
+    for lib1, lib2 in itertools.combinations(symbols, 2):
+        print(f"Common symbols between {lib1[0]} and {lib2[0]}")
+        common_symbols = lib1[1].intersection(lib2[1])
+        for common in common_symbols:
+            print(f"\t{common}")
+        print()

include/pinocchio/algorithm/contact-cholesky.hpp

@@ -104,9 +104,7 @@ namespace pinocchio
     ///
     /// \brief Default constructor
     ///
-    ContactCholeskyDecompositionTpl()
-    {
-    }
+    ContactCholeskyDecompositionTpl() = default;
 
     ///
     /// \brief Constructor from a model.
@@ -463,35 +461,10 @@ namespace pinocchio
     ///@}
 
     template<typename S1, int O1>
-    bool operator==(const ContactCholeskyDecompositionTpl<S1, O1> & other) const
-    {
-      bool is_same = true;
-
-      if (nv != other.nv || num_contacts != other.num_contacts)
-        return false;
-
-      if (
-        D.size() != other.D.size() || Dinv.size() != other.Dinv.size() || U.rows() != other.U.rows()
-        || U.cols() != other.U.cols())
-        return false;
-
-      is_same &= (D == other.D);
-      is_same &= (Dinv == other.Dinv);
-      is_same &= (U == other.U);
-
-      is_same &= (parents_fromRow == other.parents_fromRow);
-      is_same &= (nv_subtree_fromRow == other.nv_subtree_fromRow);
-      is_same &= (last_child == other.last_child);
-      //        is_same &= (rowise_sparsity_pattern == other.rowise_sparsity_pattern);
-
-      return is_same;
-    }
+    bool operator==(const ContactCholeskyDecompositionTpl<S1, O1> & other) const;
 
     template<typename S1, int O1>
-    bool operator!=(const ContactCholeskyDecompositionTpl<S1, O1> & other) const
-    {
-      return !(*this == other);
-    }
+    bool operator!=(const ContactCholeskyDecompositionTpl<S1, O1> & other) const;
     PINOCCHIO_COMPILER_DIAGNOSTIC_POP
 
   protected:
@@ -702,10 +675,17 @@ namespace pinocchio
 
 } // namespace pinocchio
 
-#include "pinocchio/algorithm/contact-cholesky.hxx"
-
+// Because of a GCC bug we should NEVER define a function that use ContactCholeskyDecompositionTpl
+// before doing the explicit template instantiation.
+// If we don't take care, GCC will not accept any visibility attribute when declaring the
+// explicit template instantiation of the ContactCholeskyDecompositionTpl class.
+// The warning message will look like this: type attributes ignored after type is already defined
+// [-Wattributes] A minimal code example is added on the PR
+// (https://github.com/stack-of-tasks/pinocchio/pull/2469)
 #if PINOCCHIO_ENABLE_TEMPLATE_INSTANTIATION
   #include "pinocchio/algorithm/contact-cholesky.txx"
 #endif // PINOCCHIO_ENABLE_TEMPLATE_INSTANTIATION
 
+#include "pinocchio/algorithm/contact-cholesky.hxx"
+
 #endif // ifndef __pinocchio_algorithm_contact_cholesky_hpp__

include/pinocchio/algorithm/contact-cholesky.hxx

@@ -625,6 +625,41 @@ namespace pinocchio
     return res;
   }
 
+  template<typename Scalar, int Options>
+  template<typename S1, int O1>
+  bool ContactCholeskyDecompositionTpl<Scalar, Options>::operator==(
+    const ContactCholeskyDecompositionTpl<S1, O1> & other) const
+  {
+    bool is_same = true;
+
+    if (nv != other.nv || num_contacts != other.num_contacts)
+      return false;
+
+    if (
+      D.size() != other.D.size() || Dinv.size() != other.Dinv.size() || U.rows() != other.U.rows()
+      || U.cols() != other.U.cols())
+      return false;
+
+    is_same &= (D == other.D);
+    is_same &= (Dinv == other.Dinv);
+    is_same &= (U == other.U);
+
+    is_same &= (parents_fromRow == other.parents_fromRow);
+    is_same &= (nv_subtree_fromRow == other.nv_subtree_fromRow);
+    is_same &= (last_child == other.last_child);
+    //        is_same &= (rowise_sparsity_pattern == other.rowise_sparsity_pattern);
+
+    return is_same;
+  }
+
+  template<typename Scalar, int Options>
+  template<typename S1, int O1>
+  bool ContactCholeskyDecompositionTpl<Scalar, Options>::operator!=(
+    const ContactCholeskyDecompositionTpl<S1, O1> & other) const
+  {
+    return !(*this == other);
+  }
+
   namespace details
   {
 

pixi.lock

@@ -175,7 +175,8 @@ casadi = { features = ["casadi", "py312"], solve-group = "py312" }
 autodiff = { features = ["autodiff", "py312"], solve-group = "py312" }
 extra = { features = ["extra", "py312"], solve-group = "py312" }
 openmp = { features = ["openmp", "py312"], solve-group = "py312" }
-codegen = { features = ["codegen", "py312"], solve-group = "py312" }
+# codegen need autodiff
+codegen = { features = ["autodiff", "codegen", "py312"], solve-group = "py312" }
 mpfr = { features = ["mpfr", "py312"], solve-group = "py312" }
 sdf = { features = ["sdf", "py312"], solve-group = "py312" }
 py39 = { features = ["py39"], solve-group = "py39" }

pixi.toml

@@ -97,7 +97,9 @@ function(PINOCCHIO_TARGET target_name)
       ${LIB_NAME}
       PROPERTIES LINKER_LANGUAGE CXX
                  INSTALL_RPATH "\$ORIGIN"
-                 VERSION ${PROJECT_VERSION})
+                 VERSION ${PROJECT_VERSION}
+                 CXX_VISIBILITY_PRESET hidden
+                 VISIBILITY_INLINES_HIDDEN ON)
   endif()
 
   if(ENABLE_TEMPLATE_INSTANTIATION AND NOT ARGS_INTERFACE)