This is an archive of the discontinued LLVM Phabricator instance.

[libcxxabi] Add support for building and testing the unwinder.
ClosedPublic

Authored by danalbert on Jul 9 2014, 8:12 PM.

Details

Reviewers
jroelofs
Summary

Note: The unwinder currently only works on Darwin and on ARM Linux. Non-ARM Linux support is not yet implemented, and will fail to build.

Diff Detail

Event Timeline

danalbert updated this revision to Diff 11237.Jul 9 2014, 8:12 PM
danalbert retitled this revision from to [libcxxabi] Add support for building and testing the unwinder..
danalbert updated this object.
danalbert edited the test plan for this revision. (Show Details)
danalbert added a reviewer: jroelofs.
danalbert added a subscriber: Unknown Object (MLST).

jroelofs: I know you had mentioned that the unwinder might be moving to compiler-rt. Any idea about timeline on that?

src/Unwind/CMakeLists.txt
89

Do we want it to just be libunwind? Could also build it in to libc++abi (or compiler-rt), but I assume we want it to be a different library.

jroelofs edited edge metadata.Jul 10 2014, 6:48 AM

(non-ARM Linux won't build by design).

Why?

src/Unwind/CMakeLists.txt
9

Sort them?

16

I'm very 'green' when it comes to cmake, but this looks odd. Should the language be set to some ASM thing instead of C?

30

Sort them?

What about:
include/libunwind.h
include/unwind.h
include/mach-o/compact_unwind_encoding.h

32

would this append_if be better if it lived just below the set for LIBUNWIND_SORUCES?

66

lib/buildit has a big list of EXTRA_FLAGS. Are those taken care of here?

EXTRA_FLAGS="-fstrict-aliasing -Wstrict-aliasing=2 \
  -Wsign-conversion -Wshadow -Wconversion -Wunused-variable \
  -Wmissing-field-initializers -Wchar-subscripts -Wmismatched-tags \
  -Wmissing-braces -Wshorten-64-to-32 -Wsign-compare \
  -Wstrict-aliasing=2 -Wstrict-overflow=4 -Wunused-parameter \
  -Wnewline-eof"
73

can this be something like "-install_name /usr/lib/lib${OUTPUT_NAME}.1.dylib" to avoid having to set that in multiple places?

test/lit.cfg
209

This changes the order of link_flags when not using llvm_unwinder. Is that okay?

In D4448#6, @jroelofs wrote:

(non-ARM Linux won't build by design).

Why?

Because there is no support for it. It isn't actually my code that prevents it from building, but rather the check at src/Unwind/UnwindCursor.hpp:1278 which requires either _LIBUNWIND_SUPPORT_COMPACT_UNWIND, _LIBUNWIND_SUPPORT_DWARF_UNWIND, or LIBCXXABI_AM_EHABI. The config that was added with the EHABI change left the dwarf and compact defines set to 0 because neither is available yet (for Linux). I'm working on fixing this right now, but it will be a different patch.

src/Unwind/CMakeLists.txt
16

I had done that originally, but it didn't work when building in tree. I'm also pretty new to cmake, but I looked at compiler-rt and this is how they handle it.

30

I think I may have mistakenly added those to the cxxabi target rather than the unwinder... I'll fix that.

test/lit.cfg
209

I was wondering the same thing. I'll try to find more info.

Ah ok. Now I remember you mentioning this on #llvm.

src/Unwind/CMakeLists.txt
16

ok

Well, phabricator didn't quite do what I intended with that....

danalbert added inline comments.Jul 10 2014, 3:34 PM
src/Unwind/CMakeLists.txt
66

Fixed these in a separate patch since they should have been there regardless of this one. Committed as r212768.

jroelofs added inline comments.Jul 10 2014, 3:53 PM
src/Unwind/CMakeLists.txt
66

cool

danalbert updated this revision to Diff 11298.Jul 10 2014, 4:13 PM
danalbert updated this object.
danalbert edited edge metadata.
jroelofs accepted this revision.Jul 11 2014, 6:36 AM
jroelofs edited edge metadata.

LGTM

This revision is now accepted and ready to land.Jul 11 2014, 6:36 AM
danalbert closed this revision.Jul 11 2014, 8:44 AM

Submitted as r212824.