[libunwind] Don't define unw_fpreg_t to uint64_t for __ARM_DWARF_EH__

[mstorsjo]

Summary

The existing typedef of unw_fpreg_t to uint64_t might work and be correct for the ARM_EHABI case, but for dwarf, some cases in e.g. DwarfInstructions.hpp convert between double and unw_fpreg_t.

When converting implicitly between double and unw_fpreg_t (uint64_t), the values get interpreted as integers and converted to float and vice versa, while the correct thing would be to keep the same bit pattern.

Avoid the whole issue by using the same definition of unw_fpreg_t as all other architectures, when using dwarf unwinding on ARM.

AFAIK NetBSD uses dwarf unwinding on ARM, and I'm doing the same for MinGW/ARM.

I'm not aware of a testcase where the current setup of unw_fpreg_t would produce incorrect results, but building produces the following warnings:

src/libunwind.cpp:236:61: warning: format specifies type
      'double' but the argument has type 'unw_fpreg_t'
      (aka 'unsigned long long') [-Wformat]
                       static_cast<void *>(cursor), regNum, value);
                                                            ^~~~~

src/DwarfInstructions.hpp:178:20: warning: implicit conversion
      turns floating-point number into integer: 'double' to 'unw_fpreg_t' (aka 
      'unsigned long long') [-Wfloat-conversion]
                i, getSavedFloatRegister(addressSpace, registers, cfa,
                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

[mclow.lists]

I have no problem with the code change, but no context on whether or not it is correct.
I'm hoping some other people familiar with ARM and DWARF can chime in.

[compnerd]

double should be safe for ARM DWARF EH, though, technically, long double is more appropriate of a type definition (the FPU state that is saved should be the widest floating point type). That would be long double (aka FP80 on x86, FP128 on AArch64/PPC, FP64 elsewhere). This happens to work because ARM uses FP64 irrespective of DWARF or EHABI.

[mstorsjo]

I had to update the signature of assembly functions which have got the concrete type of unw_fpreg_t in the mangled name. I chose to use void* for these functions, as I believe the same functions also are used for ARM EHABI, to avoid the need for different mangled versions of the assembly functions depending on exception handling mechanism.

I crafted a testcase which uses dwarf exceptions on arm, and this patch fixed restoring float registers in a case which previously was broken. So that confirms what the compiler warning tried to say.

include/libunwind.h

struct unw_cursor_t {
   uint64_t data[_LIBUNWIND_CURSOR_SIZE];
 };
 typedef struct unw_cursor_t unw_cursor_t;
 
 typedef struct unw_addr_space *unw_addr_space_t;
 
 typedef int unw_regnum_t;
 typedef uintptr_t unw_word_t;
-#if defined(__arm__)
+#if defined(__arm__) && !defined(__ARM_DWARF_EH__)
 typedef uint64_t unw_fpreg_t;
 #else
 typedef double unw_fpreg_t;
 #endif
 
 struct unw_proc_info_t {
   unw_word_t  start_ip;         /* start address of function */
   unw_word_t  end_ip;           /* address after end of function */

src/Registers.hpp

 private:
   struct GPRs {
     uint32_t __r[13]; // r0-r12
     uint32_t __sp;    // Stack pointer r13
     uint32_t __lr;    // Link register r14
     uint32_t __pc;    // Program counter r15
   };
 
-  static void saveVFPWithFSTMD(unw_fpreg_t*);
-  static void saveVFPWithFSTMX(unw_fpreg_t*);
-  static void saveVFPv3(unw_fpreg_t*);
-  static void restoreVFPWithFLDMD(unw_fpreg_t*);
-  static void restoreVFPWithFLDMX(unw_fpreg_t*);
-  static void restoreVFPv3(unw_fpreg_t*);
+  static void saveVFPWithFSTMD(void*);
+  static void saveVFPWithFSTMX(void*);
+  static void saveVFPv3(void*);
+  static void restoreVFPWithFLDMD(void*);
+  static void restoreVFPWithFLDMX(void*);
+  static void restoreVFPv3(void*);
 #if defined(__ARM_WMMX)
-  static void saveiWMMX(unw_fpreg_t*);
+  static void saveiWMMX(void*);
   static void saveiWMMXControl(uint32_t*);
-  static void restoreiWMMX(unw_fpreg_t*);
+  static void restoreiWMMX(void*);
   static void restoreiWMMXControl(uint32_t*);
 #endif
   void restoreCoreAndJumpTo();
 
   // ARM registers
   GPRs _registers;
 
   // We save floating point registers lazily because we can't know ahead of

src/UnwindRegistersRestore.S

 @
 @ On entry:
 @  values pointer is in r0
 @
   .p2align 2
 #if defined(__ELF__)
   .fpu vfpv3-d16
 #endif
-DEFINE_LIBUNWIND_PRIVATE_FUNCTION(_ZN9libunwind13Registers_arm19restoreVFPWithFLDMDEPy)
+DEFINE_LIBUNWIND_PRIVATE_FUNCTION(_ZN9libunwind13Registers_arm19restoreVFPWithFLDMDEPv)
   @ VFP and iwMMX instructions are only available when compiling with the flags
   @ that enable them. We do not want to do that in the library (because we do not
   @ want the compiler to generate instructions that access those) but this is
   @ only accessed if the personality routine needs these registers. Use of
   @ these registers implies they are, actually, available on the target, so
   @ it's ok to execute.
   @ So, generate the instruction using the corresponding coprocessor mnemonic.
   vldmia r0, {d0-d15}
   JMP(lr)
 
 @
 @ static void libunwind::Registers_arm::restoreVFPWithFLDMX(unw_fpreg_t* values)
 @
 @ On entry:
 @  values pointer is in r0
 @
   .p2align 2
 #if defined(__ELF__)
   .fpu vfpv3-d16
 #endif
-DEFINE_LIBUNWIND_PRIVATE_FUNCTION(_ZN9libunwind13Registers_arm19restoreVFPWithFLDMXEPy)
+DEFINE_LIBUNWIND_PRIVATE_FUNCTION(_ZN9libunwind13Registers_arm19restoreVFPWithFLDMXEPv)
   vldmia r0, {d0-d15} @ fldmiax is deprecated in ARMv7+ and now behaves like vldmia
   JMP(lr)
 
 @
 @ static void libunwind::Registers_arm::restoreVFPv3(unw_fpreg_t* values)
 @
 @ On entry:
 @  values pointer is in r0
 @
   .p2align 2
 #if defined(__ELF__)
   .fpu vfpv3
 #endif
-DEFINE_LIBUNWIND_PRIVATE_FUNCTION(_ZN9libunwind13Registers_arm12restoreVFPv3EPy)
+DEFINE_LIBUNWIND_PRIVATE_FUNCTION(_ZN9libunwind13Registers_arm12restoreVFPv3EPv)
   vldmia r0, {d16-d31}
   JMP(lr)
 
 #if defined(__ARM_WMMX)
 
 @
 @ static void libunwind::Registers_arm::restoreiWMMX(unw_fpreg_t* values)
 @
 @ On entry:
 @  values pointer is in r0
 @
   .p2align 2
 #if defined(__ELF__)
   .arch armv5te
 #endif
-DEFINE_LIBUNWIND_PRIVATE_FUNCTION(_ZN9libunwind13Registers_arm12restoreiWMMXEPy)
+DEFINE_LIBUNWIND_PRIVATE_FUNCTION(_ZN9libunwind13Registers_arm12restoreiWMMXEPv)
   ldcl p1, cr0, [r0], #8  @ wldrd wR0, [r0], #8
   ldcl p1, cr1, [r0], #8  @ wldrd wR1, [r0], #8
   ldcl p1, cr2, [r0], #8  @ wldrd wR2, [r0], #8
   ldcl p1, cr3, [r0], #8  @ wldrd wR3, [r0], #8
   ldcl p1, cr4, [r0], #8  @ wldrd wR4, [r0], #8
   ldcl p1, cr5, [r0], #8  @ wldrd wR5, [r0], #8
   ldcl p1, cr6, [r0], #8  @ wldrd wR6, [r0], #8
   ldcl p1, cr7, [r0], #8  @ wldrd wR7, [r0], #8

src/UnwindRegistersSave.S

 @
 @ On entry:
 @  values pointer is in r0
 @
   .p2align 2
 #if defined(__ELF__)
   .fpu vfpv3-d16
 #endif
-DEFINE_LIBUNWIND_PRIVATE_FUNCTION(_ZN9libunwind13Registers_arm16saveVFPWithFSTMDEPy)
+DEFINE_LIBUNWIND_PRIVATE_FUNCTION(_ZN9libunwind13Registers_arm16saveVFPWithFSTMDEPv)
   vstmia r0, {d0-d15}
   JMP(lr)
 
 @
 @ static void libunwind::Registers_arm::saveVFPWithFSTMX(unw_fpreg_t* values)
 @
 @ On entry:
 @  values pointer is in r0
 @
   .p2align 2
 #if defined(__ELF__)
   .fpu vfpv3-d16
 #endif
-DEFINE_LIBUNWIND_PRIVATE_FUNCTION(_ZN9libunwind13Registers_arm16saveVFPWithFSTMXEPy)
+DEFINE_LIBUNWIND_PRIVATE_FUNCTION(_ZN9libunwind13Registers_arm16saveVFPWithFSTMXEPv)
   vstmia r0, {d0-d15} @ fstmiax is deprecated in ARMv7+ and now behaves like vstmia
   JMP(lr)
 
 @
 @ static void libunwind::Registers_arm::saveVFPv3(unw_fpreg_t* values)
 @
 @ On entry:
 @  values pointer is in r0
 @
   .p2align 2
 #if defined(__ELF__)
   .fpu vfpv3
 #endif
-DEFINE_LIBUNWIND_PRIVATE_FUNCTION(_ZN9libunwind13Registers_arm9saveVFPv3EPy)
+DEFINE_LIBUNWIND_PRIVATE_FUNCTION(_ZN9libunwind13Registers_arm9saveVFPv3EPv)
   @ VFP and iwMMX instructions are only available when compiling with the flags
   @ that enable them. We do not want to do that in the library (because we do not
   @ want the compiler to generate instructions that access those) but this is
   @ only accessed if the personality routine needs these registers. Use of
   @ these registers implies they are, actually, available on the target, so
   @ it's ok to execute.
   @ So, generate the instructions using the corresponding coprocessor mnemonic.
   vstmia r0, {d16-d31}
   JMP(lr)
 
 #if defined(_LIBUNWIND_ARM_WMMX)
 
 @
 @ static void libunwind::Registers_arm::saveiWMMX(unw_fpreg_t* values)
 @
 @ On entry:
 @  values pointer is in r0
 @
   .p2align 2
 #if defined(__ELF__)
   .arch armv5te
 #endif
-DEFINE_LIBUNWIND_PRIVATE_FUNCTION(_ZN9libunwind13Registers_arm9saveiWMMXEPy)
+DEFINE_LIBUNWIND_PRIVATE_FUNCTION(_ZN9libunwind13Registers_arm9saveiWMMXEPv)
   stcl p1, cr0, [r0], #8  @ wstrd wR0, [r0], #8
   stcl p1, cr1, [r0], #8  @ wstrd wR1, [r0], #8
   stcl p1, cr2, [r0], #8  @ wstrd wR2, [r0], #8
   stcl p1, cr3, [r0], #8  @ wstrd wR3, [r0], #8
   stcl p1, cr4, [r0], #8  @ wstrd wR4, [r0], #8
   stcl p1, cr5, [r0], #8  @ wstrd wR5, [r0], #8
   stcl p1, cr6, [r0], #8  @ wstrd wR6, [r0], #8
   stcl p1, cr7, [r0], #8  @ wstrd wR7, [r0], #8