[libunwind][Modules] Add unwind_arm_ehabi.h and unwind_itanium.h to the unwind module)

[iana]

Summary

Add unwind_arm_ehabi.h and unwind_itanium.h to the unwind module and use angle includes to include them.

[ldionne]

libunwind/include/unwind_arm_ehabi.h

16–18

Why is this needed? We only include those after checking #if defined(_LIBUNWIND_ARM_EHABI).

[iana]

libunwind/include/unwind_arm_ehabi.h

16–18

The module will build all of its headers, so without these guards you'll get a lot of redeclaration errors between the two unwind_ headers.

It might maybe work to put them in independent modules and mark them as conflicting. I don't really think that's the right thing to do since these two headers are just implementation details for unwind.h.

We could make them textual, but then they don't get put in the pcm which isn't what we want.

We could make them excluded and then one or the other should be pulled into unwind when it builds, but that's a fragile mechanism since some other module could unexpectedly include them. It's also not really what we're trying to do, they should be part of unwind and not really excluded.

So I think the right thing to do is make each of these headers standalone and able to be built in the same module.

[ldionne]

libunwind/include/unwind_arm_ehabi.h

16–18

The problem is that this is a general pattern we have in a lot of places. Today, we use headers as a mechanism to (quite textually) separate code into a different physical location. These headers are implementation details and they are never intended to be included directly by the user. I'm not a modules expert, but it looks to me like those are effectively textual headers that are meant to be copy-pasted into unwind.h depending on the API we want to provide. If so, we should make them textual even if that has a negative impact on the generated PCM, since that is the proper semantics of those headers.

[iana]

libunwind/include/unwind_arm_ehabi.h

16–18

Err, it would be in the unwind PCM since unwind.h includes it, it just has the potential to get copied into other PCMs. @Bigcheese suggested we make it private textual to prevent that, let me give it a try. I still feel a little icky about making the module behavior differ from textual behavior (modules will emit an error if the unwind_ headers are directly included but textual won't), but it's not so bad.

I guess it's open to interpretation, but I see textual headers as things like assert.h where it's expected that you can include it multiple times with different values of NDEBUG in your code, or for X Macro generator headers. The unwind_ headers don't really fit either of those descriptions which is why I feel like they're a bad fit for textual, especially when it's a relatively small change to make them standalone.

That said, I can be out voted, what's really important to me is these belong to a module. I'll give private textual a try and see if it works out.

[iana]

Make the unwind_ files private textual headers and revert the changes to them.

[iana]

libunwind/include/unwind_arm_ehabi.h

16–18

It looks like private textual works.

[ldionne]

Can you please just rebase onto main and re-upload to give this one last CI spin? There's a Windows failure I don't understand. I think it's unrelated but rebasing and running the CI again should confirm that.

[iana]

Rebasing

[iana]

In D144323#4204034, @ldionne wrote:

Can you please just rebase onto main and re-upload to give this one last CI spin? There's a Windows failure I don't understand. I think it's unrelated but rebasing and running the CI again should confirm that.

You got it!

[ldionne]

LGTM, the failures in the libc++ CI are known flakes and the Windows one is a Clangd issue, most likely unrelated.

libunwind/include/libunwind.modulemap

 module libunwind [system] {
   header "libunwind.h"
   export *
 }
 
 module unwind [system] {
   header "__libunwind_config.h"
   header "unwind.h"
+  header "unwind_arm_ehabi.h"
+  header "unwind_itanium.h"
+
   export *
 }

libunwind/include/unwind.h

typedef enum {
   _UA_HANDLER_FRAME = 4,
   _UA_FORCE_UNWIND = 8,
   _UA_END_OF_STACK = 16 // gcc extension to C++ ABI
 } _Unwind_Action;
 
 typedef struct _Unwind_Context _Unwind_Context;   // opaque
 
 #if defined(_LIBUNWIND_ARM_EHABI)
-#include "unwind_arm_ehabi.h"
+#include <unwind_arm_ehabi.h>
 #else
-#include "unwind_itanium.h"
+#include <unwind_itanium.h>
 #endif
 
 typedef _Unwind_Reason_Code (*_Unwind_Stop_Fn)
     (int version,
      _Unwind_Action actions,
      _Unwind_Exception_Class exceptionClass,
      _Unwind_Exception* exceptionObject,
      struct _Unwind_Context* context,

libunwind/include/unwind_arm_ehabi.h

 //===----------------------------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //
 // C++ ABI Level 1 ABI documented at:
 //   https://github.com/ARM-software/abi-aa/blob/main/ehabi32/ehabi32.rst
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef __ARM_EHABI_UNWIND_H__
 #define __ARM_EHABI_UNWIND_H__
 
+#include <__libunwind_config.h>
+
+#ifdef _LIBUNWIND_ARM_EHABI
+
+#include <stdint.h>
+
 typedef uint32_t _Unwind_State;
 
 static const _Unwind_State _US_VIRTUAL_UNWIND_FRAME   = 0;
 static const _Unwind_State _US_UNWIND_FRAME_STARTING  = 1;
 static const _Unwind_State _US_UNWIND_FRAME_RESUME    = 2;
 static const _Unwind_State _US_ACTION_MASK            = 3;
 /* Undocumented flag for force unwinding. */
 static const _Unwind_State _US_FORCE_UNWIND           = 8;
void _Unwind_SetIP(struct _Unwind_Context *context, uintptr_t value) {
   uintptr_t thumb_bit = _Unwind_GetGR(context, 15) & ((uintptr_t)0x1);
   _Unwind_SetGR(context, 15, value | thumb_bit);
 }
 
 #ifdef __cplusplus
 }
 #endif
 
+#endif // _LIBUNWIND_ARM_EHABI
+
 #endif // __ARM_EHABI_UNWIND_H__

libunwind/include/unwind_itanium.h

 //===----------------------------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //
 // C++ ABI Level 1 ABI documented at:
 //   https://itanium-cxx-abi.github.io/cxx-abi/abi-eh.html
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef __ITANIUM_UNWIND_H__
 #define __ITANIUM_UNWIND_H__
 
+#include <__libunwind_config.h>
+
+#ifndef _LIBUNWIND_ARM_EHABI
+
+#include <stdint.h>
+
 struct _Unwind_Context;   // opaque
 struct _Unwind_Exception; // forward declaration
 typedef struct _Unwind_Exception _Unwind_Exception;
 typedef uint64_t _Unwind_Exception_Class;
 
 struct _Unwind_Exception {
   _Unwind_Exception_Class exception_class;
   void (*exception_cleanup)(_Unwind_Reason_Code reason,
extern void _Unwind_SetGR(struct _Unwind_Context *context, int index,
                           uintptr_t new_value);
 extern uintptr_t _Unwind_GetIP(struct _Unwind_Context *context);
 extern void _Unwind_SetIP(struct _Unwind_Context *, uintptr_t new_value);
 
 #ifdef __cplusplus
 }
 #endif
 
+#endif // !_LIBUNWIND_ARM_EHABI
+
 #endif // __ITANIUM_UNWIND_H__