This is an archive of the discontinued LLVM Phabricator instance.

LLGS Android target support
ClosedPublic

Authored by achien_bsi on Nov 7 2014, 7:45 AM.

Details

Summary

Added Android x86_64 support to the build compiled using standalone NDK.

It needs a separate ANDROID_NDK define instead of re-using ANDROID and/or ANDROID defines because different version of NDK for different targets (arm vs x86 for example) seem to have some include path differences.

This change does require some changes in LLVM. The patch for that was submitted separately.

Diff Detail

Event Timeline

achien_bsi updated this revision to Diff 15922.Nov 7 2014, 7:45 AM
achien_bsi retitled this revision from to LLGS Android target support.
achien_bsi updated this object.
achien_bsi edited the test plan for this revision. (Show Details)
achien_bsi added a subscriber: Unknown Object (MLST).
jingham edited edge metadata.Nov 7 2014, 10:48 AM

Most of this seems okay, but in a couple of places you either write stub versions of functions that are missing from the Android NDK. std::to_string is done in the linux/Host.cpp, which seems like an appropriate place to put missing functions, but timegm and posix_openpt you do in the place where it is used (CXXFormatterFunctions.cpp and PseudoTerminal.cpp) which doesn’t seem right. All these platform deficiencies should be gathered up in one place so that they can be shared among the source files. Having them sprinkled throughout the sources will make the host dependencies harder to understand.

Also, I don’t know what your plans are for the EditLineAndroid, but if you are intending to just leave it as a stub, then call it EditLineStub or something like that, since there’s nothing Android specific about it.

Jim

clayborg requested changes to this revision.Nov 7 2014, 11:23 AM
clayborg edited edge metadata.

Changes:
1 - Remove all Android editline stuff and just define LLDB_DISABLE_LIBEDIT when building using the Android NDK.
2 - You must implement HostThreadPosix or a native way to run threads when running on Android otherwise, LLDB won't run. It makes heavy use of threading and this is required as these classes are used to create threads within LLDB itself. The better solution if Android isn't POSIX based is to implement your own subclass of HostNativeThreadBase and then modify HostNativeThread.h to include the correct version for Andriod.
3 - Move timegm() and any other wrapper functions into the host layer somewhere and don't inline them in functions. Like Jim requested, move these all into single file and then #include the android host header as needed.

This revision now requires changes to proceed.Nov 7 2014, 11:23 AM
achien_bsi updated this revision to Diff 15941.Nov 7 2014, 3:50 PM
achien_bsi edited edge metadata.

moved std::to_string into Android.h file

removed timegm and posix_openpt (they may have been required when I was working with Arm NDK previously but this commit only focus on x86_64 so if it is needed in the future they will be inside an Android.cpp file)

removed EditLineAndroid and define LLDB_DISABLE_LIBEDIT for Android

removed the ifndef ANDROID_NDK in HostThreadPosix file - the define in HostThreadPosix was done before all the refactoring that's been going on. Android actually just use HostThreadLinux with a few details stubbed out and need to be filled in later.

clayborg accepted this revision.Nov 7 2014, 4:38 PM
clayborg edited edge metadata.

Looks good.

This revision is now accepted and ready to land.Nov 7 2014, 4:38 PM
achien_bsi closed this revision.Nov 11 2014, 3:41 PM
thakis added a subscriber: thakis.Jan 15 2015, 5:56 PM
thakis added inline comments.
cmake/LLDBDependencies.cmake
100

This doesn't look correct. Now every file that includes LLDBDependencies will have a custom command that builds tools/lldb/source/LLDB_vers.c. With ninja, this causes "ninja: warning: multiple rules generate tools/lldb/source/LLDB_vers.c. builds involving this target will not be correct; continuing anyway" for example (since there's more than one CMakeLists.txt that includes LLDBDependencies.txt).

Maybe this rule could stay where it was?

thakis added inline comments.Apr 28 2015, 1:55 PM
cmake/LLDBDependencies.cmake
100

Ping? This is still broken 4 months later.