[libunwind][include] Add some missing definitions to <unwind.h>.

[cdavis5x]

Summary

Add these declarations which should be present in <unwind.h>,
but aren't. Not that it matters, since most of the time we'll be using
Clang's <unwind.h> anyway.

[krytarowski]

NetBSD uses typedef void *_Unwind_Ptr; unconditionally in its <unwind.h>... if that has to be matched.

Additionally: typedef long _Unwind_Word;.

[mstorsjo]

include/unwind.h

50

What other reference is this list of typedefs for _Unwind_Ptr based on? I don't see any of these cases in clang's unwind.h at least.

185

Minor nitpick: Would it make sense to move the typedef _Unwind_Personality_Fn __personality_routine; out of the ifdef, since the same one can be used for both cases?

[cdavis5x]

include/unwind.h

50

Where did I get those from...? These seem to be for ARM EHABI, but I can't find them anymore in the GCC source. They aren't in Clang's header, either. I wrote this a while ago... did something change in the meantime?

I could probably get away with removing those special cases if we don't really need them.

[cdavis5x]

• Add NetBSD-specific definitions.
• Pull out common declaration of __personality_routine.

[cdavis5x]

In D50413#1191726, @krytarowski wrote:

NetBSD uses typedef void *_Unwind_Ptr; unconditionally in its <unwind.h>... if that has to be matched.

Done.

Additionally: typedef long _Unwind_Word;.

Done.

[krytarowski]

In D50413#1192475, @cdavis5x wrote:

In D50413#1191726, @krytarowski wrote:

NetBSD uses typedef void *_Unwind_Ptr; unconditionally in its <unwind.h>... if that has to be matched.

Done.

Additionally: typedef long _Unwind_Word;.

Done.

I recommend to get a review of this by @joerg

[mstorsjo]

@cdavis5x I presume the fact that this one turned out tricky is blocking submitting the SEH unwinding patch.

Would it be worth to rework that patch to just use the basic types just like libunwind does today, e.g. having _Unwind_GetRegionStart return plain uintptr_t instead of _Unwind_Ptr, which also would be consistent with the existing entry points in Unwind-EHABI.cpp, Unwind-sjlj.c and UnwindLevel1.c, in order to be able to submit it before this one gets sorted out?

[cdavis5x]

In D50413#1195101, @mstorsjo wrote:

@cdavis5x I presume the fact that this one turned out tricky is blocking submitting the SEH unwinding patch.

Would it be worth to rework that patch to just use the basic types just like libunwind does today, e.g. having _Unwind_GetRegionStart return plain uintptr_t instead of _Unwind_Ptr, which also would be consistent with the existing entry points in Unwind-EHABI.cpp, Unwind-sjlj.c and UnwindLevel1.c, in order to be able to submit it before this one gets sorted out?

Sounds reasonable.

[cdavis5x]

Ping.

[cdavis5x]

Ping.

[cdavis5x]

Ping.

Are y'all waiting for me to do something?

[joerg]

Is there a reason for defining them? As in: does anything outside libunwind use them? I haven't seen such software yet.

[kristina]

This doesn't seem like a great idea to me, it's a negligible amount of type safety gained over using stdint.h types most people are more familiar with anyway. If you need something niche like those typedefs it's probably better to include the header and define them yourself in your application.

[krytarowski]

From my point of view, the number of people who have idea about these types and why do they differ between ABIs and OSes is so low, that it's even hard to get someone to review it here.

[rnk]

Do we still need or want this? It doesn't look like it would hurt.

[kristina]

Probably not (to disambiguate, as in, it wouldn't hurt).

I'm personally against adding more and more typedefs for things that are just stdint/inttypes.h stuff and I say here the change is insignificant enough to warrant having to define even more types. It's not causing any problems but unless there are actual custom composite types that belong in that header, I don't see the benefit of them, if everyone starts wanting to typedef trivial types in unwind.h, it could be a maintainability issue. Not talking about this patch specifically but about a precedent of whether trivial types should get more and more aliases, polluting the header, and whether sticking to standard types and aliasing them in implementation is perhaps a better approach. Since the header gets installed everywhere and there isn't something like unifdef removing otherwise dead code. SEH patch was of much bigger significance so in that case it was fine (though I still like insisting on avoiding custom typedefs like DWORD for uint32_t just because rest of LLVM follows the same practice, while new composite types can be a necessity, more and more trivial type aliases isn't).

That's my stance on it. Patch itself is sound however, ignoring what I said above.

[rnk]

So, basically, if we have no internal users for these typedefs, there's no reason to clutter up headers that get installed into system header directories everywhere with dead ifdefed out code. Sounds good to me, let's not do it.

[cdavis5x]

Consensus seems clear to me. Patch abandoned.

include/unwind.h

 #endif
 
 #if defined(__APPLE__)
 #define LIBUNWIND_UNAVAIL __attribute__ (( unavailable ))
 #else
 #define LIBUNWIND_UNAVAIL
 #endif
 
+#if defined(__NetBSD__)
+typedef long _Unwind_Word;
+#else
+typedef uintptr_t _Unwind_Word;
+#endif
+typedef intptr_t _Unwind_Sword;
+typedef uintptr_t _Unwind_Internal_Ptr;
+typedef uint64_t _Unwind_Exception_Class;
+
+typedef intptr_t _sleb128_t;
+typedef uintptr_t _uleb128_t;
+
+#if _LIBUNWIND_ARM_EHABI
+#if defined(__FreeBSD__)
+typedef void *_Unwind_Ptr;
+#elif defined(__linux__)
+typedef unsigned long *_Unwind_Ptr;
+#else
+typedef uintptr_t _Unwind_Ptr;
+#endif
+#elif defined(__NetBSD__)
+typedef void *_Unwind_Ptr;
+#else
+typedef uintptr_t _Unwind_Ptr;
+#endif
+
 typedef enum {
   _URC_NO_REASON = 0,
   _URC_OK = 0,
   _URC_FOREIGN_EXCEPTION_CAUGHT = 1,
   _URC_FATAL_PHASE2_ERROR = 2,
   _URC_FATAL_PHASE1_ERROR = 3,
   _URC_NORMAL_STOP = 4,
   _URC_END_OF_STACK = 5,
struct _Unwind_Control_Block {
   long long int :0; /* Enforce the 8-byte alignment */
 } __attribute__((__aligned__(8)));
 
 typedef _Unwind_Reason_Code (*_Unwind_Stop_Fn)
       (_Unwind_State state,
        _Unwind_Exception* exceptionObject,
        struct _Unwind_Context* context);
 
-typedef _Unwind_Reason_Code (*__personality_routine)
+typedef _Unwind_Reason_Code (*_Unwind_Personality_Fn)
       (_Unwind_State state,
        _Unwind_Exception* exceptionObject,
        struct _Unwind_Context* context);
 #else
 struct _Unwind_Context;   // opaque
 struct _Unwind_Exception; // forward declaration
 typedef struct _Unwind_Exception _Unwind_Exception;
 

 typedef _Unwind_Reason_Code (*_Unwind_Stop_Fn)
     (int version,
      _Unwind_Action actions,
      uint64_t exceptionClass,
      _Unwind_Exception* exceptionObject,
      struct _Unwind_Context* context,
      void* stop_parameter );
 
-typedef _Unwind_Reason_Code (*__personality_routine)
+typedef _Unwind_Reason_Code (*_Unwind_Personality_Fn)
       (int version,
        _Unwind_Action actions,
        uint64_t exceptionClass,
        _Unwind_Exception* exceptionObject,
        struct _Unwind_Context* context);
 #endif
+typedef _Unwind_Personality_Fn __personality_routine;
 
 #ifdef __cplusplus
 extern "C" {
 #endif
 
 //
 // The following are the base functions documented by the C++ ABI
 //