Page MenuHomePhabricator

[gn] Support for building libunwind
ClosedPublic

Authored by phosek on Apr 6 2019, 4:13 PM.

Details

Summary

This change introduces support for building libuwind. The library
build should be complete, but not all CMake options have been
replicated in GN. We also don't support tests yet.

Diff Detail

Repository
rL LLVM

Event Timeline

phosek created this revision.Apr 6 2019, 4:13 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 6 2019, 4:13 PM
thakis added inline comments.Apr 6 2019, 7:58 PM
llvm/utils/gn/secondary/libunwind/src/BUILD.gn
20 ↗(On Diff #194041)

probably need a comment here to keep the sync script happy?

23 ↗(On Diff #194041)

Does making this a source_set and having both libs depend on that not work for static_library?

51 ↗(On Diff #194041)

why do you remove config_nortti and then add -fno-rtti back in here? Can't you just not remove it in the first place?

77 ↗(On Diff #194041)

It looks for .h files, no matter if in a target or not. Does not adding this to public help the script? That'd surprise me.

phosek marked 3 inline comments as done.Apr 7 2019, 10:38 PM
phosek added inline comments.
llvm/utils/gn/secondary/libunwind/src/BUILD.gn
23 ↗(On Diff #194041)

I did that originally, but I changed it to build static and shared library separately, which follows the recent change to CMake build which also stopped using object library: https://github.com/llvm/llvm-project/commit/1de15f6f336dba6e26c18c224d1b53ba617f57f0#diff-2467f5cba1ecc34d53f4480cd654aa55 which is itself is a follow up to http://llvm.org/viewvc/llvm-project?view=revision&revision=356150.

The reason why we did that for libunwind, libc++abi and libunwind is because it's sometimes necessary to use different options for the static and shared library, e.g. on some platforms static library would be built without -fPIC but shared would still use it. In case of Fuchsia we also use libunwind_hermetic_static_library option which requires building static library objects with -fvisibility=hidden. This required a lot of complicated logic, not using the source set actually makes it simpler in this case.

51 ↗(On Diff #194041)

In CMake build, it's possible to re-enable RTTI using the LLVM_ENABLE_RTTI that some users actually use, especially when linking LLVM into other projects. Do we plan on providing the same flag in LLVM build? If yes, that would be an argument for setting -fno-rtti here explicitly because it should be independent of the global LLVM setup.

77 ↗(On Diff #194041)

I think the problem is that the CMake build uses ${CMAKE_CURRENT_SOURCE_DIR}/../include/libunwind.h while I tried using ../include/libunwind.h, the script doesn't seem to handle variables or ... Any suggestions how to work around it?

thakis added inline comments.Apr 8 2019, 4:40 AM
llvm/utils/gn/secondary/libunwind/src/BUILD.gn
51 ↗(On Diff #194041)

Not unless someone needs it. I'd default to not adding things until they're needed.

77 ↗(On Diff #194041)

Is the ${CMAKE_CURRENT_SOURCE_DIR} needed on the cmake side? Some places don't use it:

$ git grep '\.\..*\.h' 'clang*CMake*'
clang/tools/libclang/CMakeLists.txt: ../../include/clang-c/Index.h

phosek updated this revision to Diff 195897.Apr 19 2019, 12:21 PM
phosek marked 3 inline comments as done.
phosek updated this revision to Diff 197812.May 2 2019, 9:45 AM
thakis accepted this revision.May 2 2019, 9:58 AM
This revision is now accepted and ready to land.May 2 2019, 9:58 AM
This revision was automatically updated to reflect the committed changes.