From 4f9365af9c3b78ed64dbadcde15f8ef3a34aada6 Mon Sep 17 00:00:00 2001 From: Arpad Borsos Date: Wed, 26 Aug 2020 12:13:01 +0200 Subject: [PATCH 1/4] fix: Add a linker script to enforce exported symbols --- CMakeLists.txt | 10 +++++++++- src/exports.map | 4 ++++ 2 files changed, 13 insertions(+), 1 deletion(-) create mode 100644 src/exports.map diff --git a/CMakeLists.txt b/CMakeLists.txt index 21adb7b1c..dc9bce140 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -338,11 +338,19 @@ target_include_directories(sentry # as `PUBLIC`, so libraries that depend on sentry get these too: # `-E`: To have all symbols in the dynamic symbol table. # `--build-id`: To have a build-id in the ELF object. +# `--version-script`: The given script makes sure only `sentry_*` symbols are exported # FIXME: cmake 3.13 introduced target_link_options +target_link_libraries(sentry PUBLIC + "$<$,$>:-Wl,--build-id=sha1,--version-script=${PROJECT_SOURCE_DIR}/src/exports.map>") + option(SENTRY_EXPORT_SYMBOLS "Export symbols for modulefinder and symbolizer" ON) if(SENTRY_EXPORT_SYMBOLS) target_link_libraries(sentry PUBLIC - "$<$,$>:-Wl,-E,--build-id=sha1>") + "$<$,$>:-Wl,-E>") +else() + set(CMAKE_C_VISIBILITY_PRESET hidden) + set(CMAKE_CXX_VISIBILITY_PRESET hidden) + set(CMAKE_VISIBILITY_INLINES_HIDDEN 1) endif() #respect CMAKE_SYSTEM_VERSION diff --git a/src/exports.map b/src/exports.map new file mode 100644 index 000000000..a361e4251 --- /dev/null +++ b/src/exports.map @@ -0,0 +1,4 @@ +{ + global: sentry_*; + local: *; +}; From 6fbad89621f8bfeb93f064dad67de803c85610c7 Mon Sep 17 00:00:00 2001 From: Mischan Toosarani-Hausberger Date: Sat, 4 Feb 2023 15:29:20 +0100 Subject: [PATCH 2/4] fix: separate unit-test build + static/dynamic linker settings --- CMakeLists.txt | 44 +++++++++++++++++++++++---------------- tests/unit/CMakeLists.txt | 1 + 2 files changed, 27 insertions(+), 18 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index dc9bce140..f4552b30e 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -334,24 +334,6 @@ target_include_directories(sentry "$" ) -# The modulefinder and symbolizer need these two settings, and they are exported -# as `PUBLIC`, so libraries that depend on sentry get these too: -# `-E`: To have all symbols in the dynamic symbol table. -# `--build-id`: To have a build-id in the ELF object. -# `--version-script`: The given script makes sure only `sentry_*` symbols are exported -# FIXME: cmake 3.13 introduced target_link_options -target_link_libraries(sentry PUBLIC - "$<$,$>:-Wl,--build-id=sha1,--version-script=${PROJECT_SOURCE_DIR}/src/exports.map>") - -option(SENTRY_EXPORT_SYMBOLS "Export symbols for modulefinder and symbolizer" ON) -if(SENTRY_EXPORT_SYMBOLS) - target_link_libraries(sentry PUBLIC - "$<$,$>:-Wl,-E>") -else() - set(CMAKE_C_VISIBILITY_PRESET hidden) - set(CMAKE_CXX_VISIBILITY_PRESET hidden) - set(CMAKE_VISIBILITY_INLINES_HIDDEN 1) -endif() #respect CMAKE_SYSTEM_VERSION if(WIN32) @@ -609,3 +591,29 @@ if(SENTRY_BUILD_EXAMPLES) add_test(NAME sentry_example COMMAND sentry_example) endif() + +# Make sure that we do not expose more symbols than from our API. This must be a PRIVATE setting or otherwise projects +# using the Native SDK as a sub-project will inherit these settings, which, for something as invasive as the definition +# of the symbol-surface is almost never a good idea. Previously this was set to PUBLIC, which led to the introduction of +# switches to turn it off. By making it private it is also no longer necessary to turn this off for projects using +# libsentry as a static library, which shouldn't have any issues with exported symbols, but projects had issues because +# they inherited our public settings when linking their executables with out static lib. +# +# We are doing this at the end of the build-script since some sub-directories are inheriting link-libraries from the +# sentry target, with those they will inherit the export map too, which doesn't make sense for an executable like a +# unit-test that tries to find all symbols in its reproducibly, some of which are not in the sentry_ namespace. +# +# Used linker parameters: +# `--build-id`: To have a build-id in the ELF object. +# `--version-script`: The given script makes sure only `sentry_*` symbols are exported +# FIXME: cmake 3.13 introduced target_link_options (blocked by Android) + +option(SENTRY_EXPORT_SYMBOLS "Export symbols for modulefinder and symbolizer" ON) +if(SENTRY_EXPORT_SYMBOLS AND NOT SENTRY_BUILD_SHARED_LIBS) + # FIXME: why was this introduced in the first place? is this client project convenience? + target_link_libraries(sentry PUBLIC + "$<$,$>:-Wl,-E,--build-id=sha1>") +else() + target_link_libraries(sentry PRIVATE + "$<$,$>:-Wl,--build-id=sha1,--version-script=${PROJECT_SOURCE_DIR}/src/exports.map>") +endif() \ No newline at end of file diff --git a/tests/unit/CMakeLists.txt b/tests/unit/CMakeLists.txt index 7c9dc6d17..93acedf27 100644 --- a/tests/unit/CMakeLists.txt +++ b/tests/unit/CMakeLists.txt @@ -54,6 +54,7 @@ target_link_libraries(sentry_test_unit PRIVATE ${SENTRY_LINK_LIBRARIES} ${SENTRY_INTERFACE_LINK_LIBRARIES} "$<$:rt>" + "$<$,$>:-Wl,-E,--build-id=sha1>" ) if(MINGW) From efa657f2a706ae13d7be3f7f89e8d75493bfc5a5 Mon Sep 17 00:00:00 2001 From: Mischan Toosarani-Hausberger Date: Thu, 9 Feb 2023 15:42:12 +0100 Subject: [PATCH 3/4] Remove option SENTRY_EXPORT_SYMBOLS and limit exported symbols to the... ...`sentry_`-prefix as a Native SDK `PRIVATE` linker setting. --- CMakeLists.txt | 23 +++++------------------ 1 file changed, 5 insertions(+), 18 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index f4552b30e..67331dd4c 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -592,28 +592,15 @@ if(SENTRY_BUILD_EXAMPLES) add_test(NAME sentry_example COMMAND sentry_example) endif() -# Make sure that we do not expose more symbols than from our API. This must be a PRIVATE setting or otherwise projects -# using the Native SDK as a sub-project will inherit these settings, which, for something as invasive as the definition -# of the symbol-surface is almost never a good idea. Previously this was set to PUBLIC, which led to the introduction of -# switches to turn it off. By making it private it is also no longer necessary to turn this off for projects using -# libsentry as a static library, which shouldn't have any issues with exported symbols, but projects had issues because -# they inherited our public settings when linking their executables with out static lib. -# -# We are doing this at the end of the build-script since some sub-directories are inheriting link-libraries from the -# sentry target, with those they will inherit the export map too, which doesn't make sense for an executable like a -# unit-test that tries to find all symbols in its reproducibly, some of which are not in the sentry_ namespace. +# Limit the exported symbols when sentry is built as a shared library to those with a "sentry_" prefix: +# - we do this at the end of the file as to not affect subdirectories reading target_link_libraries from the parent. +# - we do this as PRIVATE since our version script does not make sense in any other project that adds us. # # Used linker parameters: # `--build-id`: To have a build-id in the ELF object. -# `--version-script`: The given script makes sure only `sentry_*` symbols are exported +# `--version-script`: version script either hides "foreign" symbols or defers them as unknown ("U") to system libraries. # FIXME: cmake 3.13 introduced target_link_options (blocked by Android) - -option(SENTRY_EXPORT_SYMBOLS "Export symbols for modulefinder and symbolizer" ON) -if(SENTRY_EXPORT_SYMBOLS AND NOT SENTRY_BUILD_SHARED_LIBS) - # FIXME: why was this introduced in the first place? is this client project convenience? - target_link_libraries(sentry PUBLIC - "$<$,$>:-Wl,-E,--build-id=sha1>") -else() +if(SENTRY_BUILD_SHARED_LIBS) target_link_libraries(sentry PRIVATE "$<$,$>:-Wl,--build-id=sha1,--version-script=${PROJECT_SOURCE_DIR}/src/exports.map>") endif() \ No newline at end of file From ede1a1a7bf7afc121dd3665c97f05fcd54688017 Mon Sep 17 00:00:00 2001 From: Mischan Toosarani-Hausberger Date: Thu, 9 Feb 2023 15:46:14 +0100 Subject: [PATCH 4/4] Update changelog --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index dbbff5ccd..a551085f0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ ## Unreleased +**Breaking changes**: + +- When built as a shared library for Android or Linux, the Native SDK limits the export of symbols to the `sentry_`-prefix. The option `SENTRY_EXPORT_SYMBOLS` is no longer available and the linker settings are constrained to the Native SDK and no longer `PUBLIC` to parent projects. ([#363](https://github.com/getsentry/sentry-native/pull/363)) + **Features**: - A session may be ended with a different status code. ([#801](https://github.com/getsentry/sentry-native/pull/801))