[PATCH] D38819: [libunwind] Add support for dwarf unwinding on windows on x86_64

[Saleem Abdulrasool via Phabricator via cfe-commits]

compnerd added inline comments.


================
Comment at: include/unwind.h:125
   uintptr_t private_2; // holds sp that phase1 found for phase2 to use
-#ifndef __LP64__
+#if !defined(__LP64__) && !defined(_WIN64)
   // The implementation of _Unwind_Exception uses an attribute mode on the
----------------
I think I would prefer that we do this generically as:

    #if __POINTER_WIDTH__ == 32


================
Comment at: src/AddressSpace.hpp:145
 public:
-#ifdef __LP64__
+#if defined(__LP64__) || defined(_WIN64)
   typedef uint64_t pint_t;
----------------
I think I prefer the generic:

    #if __POINTER_WIDTH__ == 64


================
Comment at: src/AddressSpace.hpp:197
 inline uintptr_t LocalAddressSpace::getP(pint_t addr) {
-#ifdef __LP64__
+#if defined(__LP64__) || defined(_WIN64)
   return get64(addr);
----------------
Same.


================
Comment at: src/UnwindRegistersRestore.S:68
 #
+#if defined(_WIN32)
+# On entry, thread_state pointer is in rcx
----------------
This is confusing.  Why is this `_WIN32`?  Shouldn't this be `_WIN64`?


================
Comment at: src/UnwindRegistersRestore.S:72
+  movq  56(%rcx), %rax # rax holds new stack pointer
+  subq  $16, %rax
+  movq  %rax, 56(%rcx)
----------------
Hmm, why is this `$16`?  The `$rsp` was adjusted by `$8` in the `setjmp`.


================
Comment at: src/UnwindRegistersSave.S:66
 DEFINE_LIBUNWIND_FUNCTION(unw_getcontext)
+#if defined(_WIN32)
+  movq  %rax,   (%rcx)
----------------
Shouldn't this be `_WIN64`?


https://reviews.llvm.org/D38819