[PATCH] D33878: Handle NetBSD specific _Unwind_Ptr

[Saleem Abdulrasool via Phabricator via llvm-commits]

compnerd requested changes to this revision.
compnerd added inline comments.
This revision now requires changes to proceed.


================
Comment at: lib/builtins/gcc_personality_v0.c:27
 
+#if defined(__NetBSD__)
+#define TYPE_UNWIND_PTR void*
----------------
krytarowski wrote:
> This line must be `defined(__NetBSD__) && !defined(__clang__)` as the failure is GCC specific. Clang uses custom `unwind.h` header.
`clang` doesn't really use a custom `unwind.h` per se.  It is one of the compiler vended headers, and unless you specifically setup the build, the compiler vended header can be used over the external unwinder.  That said, this macro should only be needed at one site, so we should just inline the difference.


================
Comment at: lib/builtins/gcc_personality_v0.c:215-216
 
-    uintptr_t pc = _Unwind_GetIP(context)-1;
-    uintptr_t funcStart = _Unwind_GetRegionStart(context);
+    uintptr_t pc = (uintptr_t)_Unwind_GetIP(context) - 1;
+    uintptr_t funcStart = (uintptr_t)_Unwind_GetRegionStart(context);
     uintptr_t pcOffset = pc - funcStart;
----------------
Interesting that this has never been an issue, but the explicit cast here is fine.


================
Comment at: lib/builtins/gcc_personality_v0.c:248
             _Unwind_SetGR(context, __builtin_eh_return_data_regno(0),
-                          (uintptr_t)exceptionObject);
+                          (TYPE_UNWIND_PTR)exceptionObject);
             _Unwind_SetGR(context, __builtin_eh_return_data_regno(1), 0);
----------------
`_Unwind_SetGR` is declared as: `void _Unwind_SetGR(struct _Unwind_Context *, int, _Unwind_Word);`

So this cast should not be needed.


================
Comment at: lib/builtins/gcc_personality_v0.c:250
             _Unwind_SetGR(context, __builtin_eh_return_data_regno(1), 0);
-            _Unwind_SetIP(context, (funcStart + landingPad));
+            _Unwind_SetIP(context, (TYPE_UNWIND_PTR)(funcStart + landingPad));
             return _URC_INSTALL_CONTEXT;
----------------
Yes, `_Unwind_SetIP` is declared as `void _Unwind_SetIP(struct _Unwind_Context *, _Unwind_Ptr);`, and if NetBSD declares it as `void *`, then the explicit cast is reasonable.

Didn't NetBSD use GCC at one point?  It seems that GCC too defines `_Unwind_Ptr` as `unsigned int __attribute__((__mode__(__pointer__)))`, making it effectively the same as `uintptr_t`.

I'm tempted to say that this is a bug in the unwinder in NetBSD.  However, adding a condition here for NetBSD and a cast is not too terrible.


Repository:
  rL LLVM

https://reviews.llvm.org/D33878