[libunwind] Don't store a predecremented PC when using SEH

[mstorsjo]

Summary

This fixes unwinding in boundary cases on ARM with SEH.

In the case of ARM/Thumb, disp->ControlPc points at the following
instruction, with the thumb bit set. Thus by decrementing 1,
it still points at the next instruction. To achieve the desired
effect of pointing at the previous instruction, one first has to strip
out the thumb bit, then do the decrement by 1 to reach the previous
instruction.

When libcxxabi looks for call site ranges, it already does
_Unwind_GetIP(context) - 1 (in scan_eh_tab in
libcxxabi/src/cxa_personality.cpp) we shouldn't do the corresponding
- 1 multiple times.

In the case of libcxxabi on Thumb, funcStart (still in scan_eh_tab)
may have the thumb bit set. If the program counter address is
decremented both in libunwind (first removing the thumb bit, then
decremented), and then libcxxabi decrements it further, and compares
with a funcStart with the thumb bit set, it could point to one byte
before the start of the call site.

Thus: This modification makes libunwind with SEH work with libcxxabi
on Thumb, in settings where libunwind and libcxxabi worked fine with
Dwarf before.

For existing cases with libunwind with SEH (on x86_64 and aarch64),
this modification doesn't break any of my testcases.

[MaskRay]

I don't have an environment to verify and don't know much about SEH, but I assume that this is similar to UnwindCursor<A, R>::setInfoBasedOnIPRegister and you or @cdavis5x has verified the correctness.

When libcxxabi looks for call site ranges, it already does _Unwind_GetIP(context) - 1 (in scan_eh_tab in libcxxabi/src/cxa_personality.cpp) we shouldn't do the corresponding - 1 multiple times.

libgcc _Unwind_GetIPInfo sets ip_before_insn to 0 so libsupc++ from libstdc++ does an the - 1 subtraction. I assume that libc++abi is similar.

[manojgupta]

@danielkiss can you please review?

[mstorsjo]

In D126869#3559075, @MaskRay wrote:

I don't have an environment to verify and don't know much about SEH, but I assume that this is similar to UnwindCursor<A, R>::setInfoBasedOnIPRegister and you or @cdavis5x has verified the correctness.

Yeah I've tested it as far as I can, and @cdavis5x was the one who implemented the SEH parts in libunwind originally.

When libcxxabi looks for call site ranges, it already does _Unwind_GetIP(context) - 1 (in scan_eh_tab in libcxxabi/src/cxa_personality.cpp) we shouldn't do the corresponding - 1 multiple times.

libgcc _Unwind_GetIPInfo sets ip_before_insn to 0 so libsupc++ from libstdc++ does an the - 1 subtraction. I assume that libc++abi is similar.

Thanks for the further context on that issue! Yeah, although libc++abi doesn't call such a function, but seems to unconditionally do the -1 subtraction. libunwind's _Unwind_GetIPInfo uses UnwindCursor::isSignalFrame(), and the SEH version of that always returns false currently. So this makes those things more consistent too.

libunwind/src/Unwind-seh.cpp

if (ms_exc->ExceptionCode == STATUS_GCC_THROW) {
     // We can't interact with them (we don't know the original target frame
     // that we should pass on to RtlUnwindEx in _Unwind_Resume), so just
     // pass without calling our destructors here.
     return ExceptionContinueSearch;
   }
   if (!ctx) {
     __unw_init_seh(&cursor, disp->ContextRecord);
     __unw_seh_set_disp_ctx(&cursor, disp);
-    __unw_set_reg(&cursor, UNW_REG_IP, disp->ControlPc - 1);
+    __unw_set_reg(&cursor, UNW_REG_IP, disp->ControlPc);
     ctx = (struct _Unwind_Context *)&cursor;
 
     if (!IS_UNWINDING(ms_exc->ExceptionFlags)) {
       if (ours && ms_exc->NumberParameters > 1)
         action =  (_Unwind_Action)(_UA_CLEANUP_PHASE | _UA_FORCE_UNWIND);
       else
         action = _UA_SEARCH_PHASE;
     } else {

libunwind/src/UnwindCursor.hpp

#endif
   // need our own defition of inline placement new.
   static void *operator new(size_t, UnwindCursor<A, R> *p) { return p; }
 
 private:
 
   pint_t getLastPC() const { return _dispContext.ControlPc; }
   void setLastPC(pint_t pc) { _dispContext.ControlPc = pc; }
   RUNTIME_FUNCTION *lookUpSEHUnwindInfo(pint_t pc, pint_t *base) {
+#ifdef __arm__
+    // Remove the thumb bit; FunctionEntry ranges don't include the thumb bit.
+    pc &= ~1U;
+#endif
+    // If pc points exactly at the end of the range, we might resolve the
+    // next function instead. Decrement pc by 1 to fit inside the current
+    // function.
+    pc -= 1;
     _dispContext.FunctionEntry = RtlLookupFunctionEntry(pc,
                                                         &_dispContext.ImageBase,
                                                         _dispContext.HistoryTable);
     *base = _dispContext.ImageBase;
     return _dispContext.FunctionEntry;
   }
   bool getInfoFromSEH(pint_t pc);
   int stepWithSEHData() {