This is an archive of the discontinued LLVM Phabricator instance.

[libunwind] Haiku: Initial support
ClosedPublic

Authored by trungnt2910 on Aug 14 2023, 6:38 AM.

Details

Reviewers
brad
nielx
kallisti5
MaskRay
korli
Group Reviewers
Restricted Project
Commits
rG976dbae24638: [libunwind] Haiku: Initial support
Summary

Adds build support for Haiku OS to libunwind.

libunwind can now be built on Haiku OS with clang 12. make check-unwind also passes.

Diff Detail

Event Timeline

trungnt2910 created this revision.Aug 14 2023, 6:38 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 14 2023, 6:38 AM
trungnt2910 requested review of this revision.Aug 14 2023, 6:38 AM

Ran the diff through git-clang-format, code looks a bit strange though.

trungnt2910 added reviewers: brad, nielx, kallisti5, Restricted Project.Sep 1 2023, 4:46 PM
brad added a comment.Sep 1 2023, 5:00 PM

diffs need to have context, .e.g. git diff -U999999

Added the full context.

Seems like git format-patch does not include it by default...

brad added inline comments.Sep 1 2023, 5:18 PM
libunwind/include/__libunwind_config.h
41

Fix the indentation.

libunwind/src/config.h
54

Fix the indentation.

trungnt2910 added inline comments.Sep 1 2023, 5:46 PM
libunwind/include/__libunwind_config.h
41

The indentation is intended.

Using the indentation that looks correct would make the build fail due to clang-format: https://reviews.llvm.org/B252866

libunwind/src/config.h
54

Same as above.

brad added inline comments.Sep 1 2023, 5:52 PM
libunwind/include/__libunwind_config.h
41

The indentation is intended.

Using the indentation that looks correct would make the build fail due to clang-format: https://reviews.llvm.org/B252866

Don't worry about that. That's not an absolute requirement and matching the surrounding formatting is more important.

brad added a comment.Sep 1 2023, 6:10 PM

The llvm-libunwind-shared.cfg.in chunk looks like it should probably be a separate commit, but make the sames changes for llvm-libunwind-merged.cfg.in and llvm-libunwind-static.cfg.in.

I don't know if the tests have ever been run on OpenBSD but this looks like it would help other OS's that do not have libdl.

trungnt2910 marked 3 inline comments as done.
trungnt2910 added inline comments.
libunwind/include/__libunwind_config.h
41

Reverted the indentation changes.

The llvm-libunwind-shared.cfg.in chunk looks like it should probably be a separate commit, but make the sames changes for llvm-libunwind-merged.cfg.in and llvm-libunwind-static.cfg.in.

I don't know if the tests have ever been run on OpenBSD but this looks like it would help other OS's that do not have libdl.

Must it be in a different patch? The changes are relatively minor but required to test libunwind on Haiku (and therefore confirm that this patch is functional).

brad set the repository for this revision to rG LLVM Github Monorepo.
brad added a subscriber: llvm-commits.
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptSep 1 2023, 10:41 PM
Herald added 1 blocking reviewer(s): Restricted Project. · View Herald Transcript
brad added a comment.Sep 2 2023, 10:22 PM

I am not a libunwind developer, but it looks good to me and looking at the libraries mentioned they do check out (libroot / libbsd).

So, Haiku uses libroot instead of libc? The changes seem fine, but I hope that a Haiku contributor that is not the author can verify.

So, Haiku uses libroot instead of libc? The changes seem fine, but I hope that a Haiku contributor that is not the author can verify.

This is correct. On Haiku, the C and POSIX functions are in libroot (excluding network-related ones, which are in a separate library.)

brad added a reviewer: korli.Sep 3 2023, 6:37 PM
brad added a comment.Sep 5 2023, 5:09 AM

@MaskRay Able to Ok this?

MaskRay accepted this revision.Sep 5 2023, 1:10 PM
This revision is now accepted and ready to land.Sep 5 2023, 1:10 PM
This revision was automatically updated to reflect the committed changes.