Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

i#2154 Android64: Link static c++ runtime library #7239

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

jackgallagher-arm
Copy link
Collaborator

@jackgallagher-arm jackgallagher-arm commented Jan 31, 2025

Android splits the C++ STL runtime into a separate library which is not provided as a standard system library. Applications are expected to statically link the STL library (libc++_static.a) or distribute the dynamic library (libc++_shared.so) with the application [1].

All C++ targets should now be linking the static runtime libraries.

[1] https://developer.android.com/ndk/guides/cpp-support#important_considerations

Issue: #2154, #1874

Android splits the C++ STL runtime into a separate library which is not
provided as a standard system library. Applications are expected to
statically link the STL library (libc++_static.a) or distribute the
dynamic library with the application.

The NDK C++ support guide recommends using the shared library C++
runtime if you are building a shared library or an application that uses
shared libraries [1] so I have set the DynamoRIO CMake scripts to do
that by default and added code to deal with deploying libc++_shared.so.

This is handled in two places:
 - drsyms CMakeLists.txt explicitly installs libc++_shared.so into
   ${INSTALL_EXT_LIB} with the other dependency libraries. This will
   ensure the library is included when generating a DynamoRIO package.

 - DynamoRIO_copy_target_to_device() now checks if a target it is
   copying is a C++ target and automatically copies the runtime library
   as well if it is.

[1] https://developer.android.com/ndk/guides/cpp-support#important_considerations

Issue: #2154, #1874
CMakeLists.txt Outdated
@@ -1833,6 +1833,9 @@ if (UNIX)
set(libmath m)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Android splits the C++ STL runtime into a separate library which is not provided as a standard system library. Applications are expected to statically link the STL library (libc++_static.a) or distribute the dynamic library with the application.

The NDK C++ support guide recommends using the shared library C++ runtime if you are building a shared library or an application that uses shared libraries [1] so I have set the DynamoRIO CMake scripts to do that by default and added code to deal with deploying libc++_shared.so.

Given all the pain around supporting clients using shared libraries on every platform (probably impossible on Mac #1285; pain on Android and Linux #5437 (comment)), if a static libc++ that is PIC is so readily available isn't it better to use that?

Lack of availability of a PIC build is one reason we did not push static libc++ before (same for libc; that's one reason we considered musl which is easier to distribute ourselves). Yes, we would need to ensure clients use the _static versions of all extension libraries esp C++ libraries such as drmemtrace; and we'd need --wrap for malloc, and would need static compression libs for drmemtrace, etc. But this is a path to make C++ clients work on Android without needing a private loader, avoiding all of those headaches and complexities.

Is there a PIC static libc/bionic as well?

Does this PR's shared libc++ let you run DR clients on Android64? I thought the private loader was still broken on Android. If this does let you make progress and run some things, maybe we put this in, but with comments about adding static support (either in addition to or instead of) since we expect private loader pain without a bunch of effort for something like #5437 (comment).

Should we file a separate issue on all-static-library Android? If the private loader is indeed still broken, I think it is worth trying the static library route.

Copy link
Collaborator Author

@jackgallagher-arm jackgallagher-arm Feb 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if a static libc++ that is PIC is so readily available isn't it better to use that?

I have changed it to use that.

Is there a PIC static libc/bionic as well?

There is a static libc.a in the NDK and I believe it is PIC. The NDK only includes one version of libc.a corresponding to the latest API_LEVEL. I don't know if using it has forwards/backwards compatibility implications though; whether binaries that use it will run on any Android version or if it will only run on the Android version that matches the API level. Comments here and here seem to imply it would be fine.

Should we file a separate issue on all-static-library Android? If the private loader is indeed still broken, I think it is worth trying the static library route.

Created #7265. I have some some experimenting with building all-static-library clients and I have managed to get the sample clients to build and the ones I have tested appear to be running okay so it looks like the quickest way forwards and it doesn't preclude implementing any of the other private loader solutions proposed in #5437.

@jackgallagher-arm jackgallagher-arm changed the title i#2154 Android64: Deploy c++ runtime library i#2154 Android64: Link static c++ runtime library Feb 10, 2025
# Use static C++ runtime libraries.
set(CMAKE_ANDROID_STL_TYPE "c++_static")
string(APPEND CMAKE_CXX_FLAGS_INIT " -stdlib=libc++")
string(APPEND CMAKE_CXX_STANDARD_LIBRARIES " -static-libstdc++")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about wrapping malloc: please see https://dynamorio.org/using.html#subsec_no_loader. In your tests I assume your static libc++ was using libc malloc which is not safe. I don't think we ever put in place CMake support for passing -wrap to the linker? Do you plan to add that in this PR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants