Update libunwind to v1.6.2

[am11]

All changes upstream since v1.5-rc2 libunwind/libunwind@v1.5-rc2...v1.6.2.

Method:

1. #!/usr/bin/env bash
   
   # backup existing libunwind
   cd src/coreclr/pal/src
   mv libunwind libunwind_old
   
   # clone new version branch
   git clone https://github.com/libunwind/libunwind --single-branch --depth 1 --branch v1.6.2
   
   # cp files and directories from the backup
   cp -r libunwind_old/{libunwind-version.txt,CMakeLists.txt,configure.cmake,config.h.in} libunwind
   cp -r libunwind_old/src/{CMakeLists.txt,oop} libunwind/src
   
   # delete .git/ dir and README.md file
   cd libunwind
   rm -rf .git README.md
   
   # make README.md symlink to README file
   ln -s README README.md

2. Manually update src/coreclr/pal/src/libunwind/libunwind-version.txt.
3. Manually apply changes from 1b5719c.

[am11]

@janvorli, this is ready for review, could you please take a look? libunwind-version.txt has the updated info, how this copy currently differs from upstream. Thanks!

[am11]

runtime/src/coreclr/pal/src/libunwind/src/oop/_OOP_find_proc_info.c

Line 15 in 57bfe47

_OOP_find_proc_info (

we carry this one-off API in our tree for few versions now. It would be nice to upstream it with a test+manual similar to libunwind/libunwind@a860906.

cc @mikem8361

[am11]

@janvorli, @jkotas, this is ready from my side, please let me know if there is any more feedback.

We could run the outerloop leg against this PR to make sure alpine arm64 library tests haven't regressed. Unfortunately the passing rate of the outerloop-linux pipeline is 0% in past six months:

so I'm not sure which specific tests we will be looking at to detect any regression..
it happens to be the same situation as it was the last time around #36909 (comment).

[janvorli]

This is now missing my changes that added storing the floating point registers. In both the thumb and non-thumb code. For the #else branch, just adding my two lines after the stmia should be enough. For the #if branch, we would need to make sure that the PC stored by the stmia matches the new semantics of the code from the libunwind repo, that means that it points after the nop. It seems that the easiest is to store the floating point registers first and then the floating point ones. A simple approach would be to add the following before the stmia:

    "adds %[base], #64\n" \
    "vstmia %[base], {d0-d15}\n" \
    "subs %[base], #64\n" \

[am11]

@janvorli, thanks, is it the correct patch:

diff --git a/src/coreclr/pal/src/libunwind/include/libunwind-arm.h b/src/coreclr/pal/src/libunwind/include/libunwind-arm.h
index 9108dd9d9a3..c5c2b3072ac 100644
--- a/src/coreclr/pal/src/libunwind/include/libunwind-arm.h
+++ b/src/coreclr/pal/src/libunwind/include/libunwind-arm.h
@@ -280,6 +280,9 @@ unw_tdep_context_t;
   unsigned long *unw_base = unw_ctx->regs;                              \
   __asm__ __volatile__ (                                                \
     "mov r0, #0\n"                                                      \
+    "adds %[base], #64\n"                                               \
+    "vstmia %[base], {d0-d15}\n"                                        \
+    "subs %[base], #64\n"                                               \
     "stmia %[base], {r0-r15}\n"                                         \
     "nop\n" /* align return address to value stored by stmia */         \
     : [r0] "=r" (r0) : [base] "r" (unw_base) : "memory");               \
@@ -295,6 +298,9 @@ unw_tdep_context_t;
     "nop\n"                                                             \
     ".code 32\n"                                                        \
     "mov r0, #0\n"                                                      \
+    "adds %[base], #64\n"                                               \
+    "vstmia %[base], {d0-d15}\n"                                        \
+    "subs %[base], #64\n"                                               \
     "stmia %[base], {r0-r14}\n"                                         \
     "adr r0, ret%=+1\n"                                                 \
     "str r0, [%[base], #60]\n"                                          \

[janvorli]

Yes, that it correct. However the thumb branch (the #else one) can be simpler, since the PC is computed and stored separately there. So we can use the following form, saving one subs.

@@ -295,6 +298,9 @@ unw_tdep_context_t;
     "nop\n"                                                             \
     ".code 32\n"                                                        \
     "mov r0, #0\n"                                                      \
     "stmia %[base], {r0-r14}\n"                                         \
     "adr r0, ret%=+1\n"                                                 \
     "str r0, [%[base], #60]\n"  
+    "adds %[base], #64\n"  
+    "vstmia %[base], {d0-d15}\n"                                        \
     "orr r0, pc, #1\n"                                                  \
     "bx r0\n" 

We could possibly get rid of the subs in the #if branch too if we made the writes backwards (instead of stmia we would use stmda), but I think it is not worth making the code less readable that way.

[am11]

Applied and rebased. All arm changes are in a single commit: 0ca7809.

[vtjnash]

Even better, I think you can directly replace the nop with vstmia %[base], {d0-d15}\n without needing to adjust the base at all

[am11]

@janvorli, can we add a test case in Pri0 category for regular PRs that will:

• make use of arm32 fp registers and hit this path
• fail without applying your patch

I am not sure if it is as trivial as changing this test project\s property from 1 to 0

runtime/src/tests/JIT/IL_Conformance/Convert/TestConvertFromIntegral.csproj

Line 4 in 61b5c92

<CLRTestPriority>1</CLRTestPriority>

? (as that was the project which was modified as part of the original fix 1b5719c#diff-3df4332914893fb234891c86871f559e8d91d5b604df4073c56cdc36e9217b51)

[janvorli]

The thing is that I think we cannot enforce using non-volatile floating point registers (D8..D15) in the jitted code. Using volatile ones doesn't cause issues, as we don't need to unwind it. Also, the register needs to be modified by the native code we unwind from to cause harm. Managed code uses the windows style unwinder, so it works unrelated to this change.
We were hitting the issue in one of the tests with JIT stress enabled - the JIT stress resulted in using the D8 register. I think that might be just a coincidence though. I believe we run tests with jit stress enabled nightly or on some regular basis, so I'll check if that test still reproduces the problem so that we have some coverage.

[janvorli]

Oh, though you may also now need to update the register constraint to let the compiler know that you are now clobbering that register

@vtjnash good point!

[janvorli]

@am11 I have verified that this update works with the TestConvertFromIntegral test by manually stepping through the unwinder.

[am11]

@janvorli, thank you for all the help. :)

[janvorli]

LGTM, thank you!