This is an archive of the discontinued LLVM Phabricator instance.

[libunwind] Add module maps for libunwind
ClosedPublic

Authored by iana on Oct 5 2022, 10:00 PM.

Details

Summary

Add module maps for the libunwind headers. unwind_arm_ehabi.h and unwind_itanium.h aren't covered because they don't get installed on all platforms.

Diff Detail

Event Timeline

iana created this revision.Oct 5 2022, 10:00 PM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptOct 5 2022, 10:00 PM
Herald added a reviewer: Restricted Project. · View Herald Transcript
iana requested review of this revision.Oct 5 2022, 10:00 PM
Herald added a project: Restricted Project. · View Herald TranscriptOct 5 2022, 10:00 PM
MaskRay added a subscriber: MaskRay.Oct 6 2022, 3:21 PM
MaskRay added inline comments.
libunwind/include/mach-o/compact_unwind_encoding.modulemap
2

Why is this not called unwind.something?

Can you state the motivation for this change?

iana added a comment.Oct 6 2022, 3:27 PM

Can you state the motivation for this change?

It's so that usr/include/module.modulemap can include these modulemap files on Apple platforms (or other platforms that want them).

libunwind/include/libunwind.modulemap
2

libunwind.h includes Availability.h/AvailabilityMacros.h which are part of the Darwin.Availability module on Apple platforms.

The Darwin module will include the clang builtin header unwind.h, which does include_next to this unwind.h.

All together the module hierarchy will go libunwind -> Darwin -> _Builtin_unwind (coming soon) -> unwind.

libunwind/include/mach-o/compact_unwind_encoding.modulemap
2

Because on Apple platforms it's part of the MachO module. This file is already present on Apple platforms, I'm just upstreaming it.

iana added a comment.Oct 7 2022, 2:24 PM

The buildable error looks like an infrastructure problem where Phabricator doesn't know the cmake/ninja command to build libunwind.

iana marked an inline comment as done.Oct 7 2022, 2:24 PM

If this doesn't make sense in open source, just let me know and I'll move it to the Apple fork.

I think the default name module.modulemap (loaded by -fimplicit-module-maps) is more appropriate for the upstream and it helps layering check features in Clang.
The name libunwind.modulemap is indeed special and seems to platform specific. I don't know how to assess compact_unwind_encoding.modulemap and the MachO.* name instead of libunwind.*/unwind.*.

iana abandoned this revision.Oct 20 2022, 10:03 PM

Alright then, I'll bump this to the Apple fork. I don't know that there really is a great way to upstream this. If we call the top level module map file module.modulemap then that will install usr/include/module.modulemap which I don't think clang should do. Anything else (like libunwind.modulemap) will require OS/SDK coordination.

I think the default name module.modulemap (loaded by -fimplicit-module-maps) is more appropriate for the upstream and it helps layering check features in Clang.
The name libunwind.modulemap is indeed special and seems to platform specific. I don't know how to assess compact_unwind_encoding.modulemap and the MachO.* name instead of libunwind.*/unwind.*.

Does that mean that no library rooted at /usr/include can provide its own modulemap? For example, libc++ provides its own modulemap and calls it module.modulemap, but it can do that because it is rooted at /usr/include/c++/v1. If it were rooted at /usr/include, it would collide with any other system-provided module.modulemap file (arguably like a C standard library modulemap).

I am not sure that the current convention of calling those files module.modulemap makes sense, but I'm happy to be corrected.

libc++ does have a modulemap that it provides for all platforms and this is upstream -- I don't see why libunwind should be any different, but again I am happy to be corrected.

iana added a comment.EditedOct 24 2022, 1:32 PM

I think the default name module.modulemap (loaded by -fimplicit-module-maps) is more appropriate for the upstream and it helps layering check features in Clang.
The name libunwind.modulemap is indeed special and seems to platform specific. I don't know how to assess compact_unwind_encoding.modulemap and the MachO.* name instead of libunwind.*/unwind.*.

Does that mean that no library rooted at /usr/include can provide its own modulemap? For example, libc++ provides its own modulemap and calls it module.modulemap, but it can do that because it is rooted at /usr/include/c++/v1. If it were rooted at /usr/include, it would collide with any other system-provided module.modulemap file (arguably like a C standard library modulemap).

Yes that's correct. There can be only one owner per directory, and libunwind doesn't own /usr/include. All it can do is provide a module map that the owner can include from module.modulemap. That will be done by putting extern module libunwind "libunwind.modulemap" and extern module unwind "libunwind.modulemap" in the OS's /usr/include/module.modulemap.

I am not sure that the current convention of calling those files module.modulemap makes sense, but I'm happy to be corrected.

That's just the compiler magic name.

libc++ does have a modulemap that it provides for all platforms and this is upstream -- I don't see why libunwind should be any different, but again I am happy to be corrected.

libc++ owns /usr/include/c++/v1, and thus owns /usr/include/c++/v1/module.modulemap which it indeed provides. libunwind has to be different, because it doesn't own any directories at all.

Good point about libunwind headers are usually installed in /usr/include , shared with other system headers.
Is there any information how Mach-O platforms use the modulemap file?
IIUC non-Mach-O platforms will probably not benefit from libunwind.modulemap.

IIUC non-Mach-O platforms will probably not benefit from libunwind.modulemap.

Why not? Isn't it the same as with libc++ providing its own modulemap? Sorry if that's a naive question, but I don't see how any of this is specific to mach-o.

libunwind/include/mach-o/compact_unwind_encoding.modulemap
2

I actually don't quite understand why we need this at all. We don't need to install that in usr/include, since it's only used to build the libunwind library.

iana reclaimed this revision.Oct 25 2022, 1:51 PM
iana added a comment.Oct 25 2022, 1:54 PM

Good point about libunwind headers are usually installed in /usr/include , shared with other system headers.
Is there any information how Mach-O platforms use the modulemap file?
IIUC non-Mach-O platforms will probably not benefit from libunwind.modulemap.

In the Apple platforms (I assume that's what you mean by Mach-O), it will consume these module maps from the usr/include/module.modulemap file that the OS provides like this.

extern module libunwind "libunwind.modulemap"
extern module unwind "libunwind.modulemap"

module Darwin [system] {
...
}
// and so on and so forth

Non-Apple platforms would do the same from their usr/include/module.modulemap file, if any other platform has that. (I don't know if Linux or Windows or any other platform has modularized their OS headers)

MaskRay accepted this revision.Oct 25 2022, 4:15 PM

Good point about libunwind headers are usually installed in /usr/include , shared with other system headers.
Is there any information how Mach-O platforms use the modulemap file?
IIUC non-Mach-O platforms will probably not benefit from libunwind.modulemap.

In the Apple platforms (I assume that's what you mean by Mach-O), it will consume these module maps from the usr/include/module.modulemap file that the OS provides like this.

extern module libunwind "libunwind.modulemap"
extern module unwind "libunwind.modulemap"

module Darwin [system] {
...
}
// and so on and so forth

Non-Apple platforms would do the same from their usr/include/module.modulemap file, if any other platform has that. (I don't know if Linux or Windows or any other platform has modularized their OS headers)

OK, thanks. I see that several projects including libc++/libc++abi install module.modulemap files. If other platforms want to use module.modulemap, they can use the export syntax.

This revision is now accepted and ready to land.Oct 25 2022, 4:15 PM
iana marked an inline comment as done.Oct 25 2022, 8:37 PM
iana added inline comments.
libunwind/include/mach-o/compact_unwind_encoding.modulemap
2

This file installs in usr/include/mach-o, where usr/include/macho-o/module.modulemap extern's it.

module MachO [system] [extern_c] {
...
  extern module compact_unwind_encoding "compact_unwind_encoding.modulemap"

  module arch {
    header "arch.h"
    export *
  }
...
}

And again this file is already present in Apple's version of clang, I thought it made sense to upstream it.

This revision was landed with ongoing or failed builds.Oct 26 2022, 10:39 PM
This revision was automatically updated to reflect the committed changes.
iana marked an inline comment as done.