Libunwind 1.5rc2 again #36988

sdmaclea · 2020-05-25T19:08:27Z

Split the libunwind upgrade to 1.5rc2 into two parts.

Support for UNWIND_CONTEXT_IS_UCONTEXT_T==0
Update libunwind 1.3rc1 to 1.5rc2

This first part is a requirement of the second part.

Do it separately to see if it causes the libraries regression which triggered the revert of the upgrade.

ghost · 2020-05-25T19:08:32Z

Tagging subscribers to this area: @ViktorHofer
Notify danmosemsft if you want to be subscribed.

sdmaclea · 2020-05-25T22:52:32Z

Interestingly the force UNWIND_CONTEXT_IS_UCONTEXT_T=0 broke x64, but not aarch64.

sdmaclea · 2020-05-26T17:30:29Z

@safern @jashook I am trying to recreate the failure which caused the revert. I picked up the change to run the libraries on arm64, but I am still seeing libraries on arm64 being skipped and not failing.

I tried to copy the correct line form your patch, but it is not working. Did I copy it wrong? Am I misunderstanding this should run arm64 libraries tests now?

sdmaclea · 2020-05-26T18:37:21Z

Per @safern #36909 reverted this change because it broke alpine arm64 which had no PR CI coverage.

@janvorli @am11 I am assuming the issue was that the uc_sigmask structure was not always the same size (and may have been different between the pal/src/libunwind and the pal/src/exception view). Rather than use macro magic, I have directly applied the patch for libunwind/libunwind#176, and updated the version file for the two patches I have applied on top of 1.5rc2.

This removes the Alpine != glibc, so glibc testing should uncover any issues.

am11 · 2020-05-26T19:07:40Z

Thank you!

We can open a PR in libunwind repo. Maintainer will review it (like they did for the previous fixes).

safern · 2020-05-26T19:14:18Z

@sdmaclea you would need to add Linux_musl_arm64 here in order to recreate the failure. https://github.com/dotnet/runtime/blob/master/eng/pipelines/runtime.yml#L906

I will update my PR to include musl_arm64 testing on runtime changes.

janvorli · 2020-05-26T19:43:30Z

I am assuming the issue was that the uc_sigmask structure was not always the same size

@sdmaclea so you were not able to repro the problem locally? I have Alpine ARM64 Linux distro on my local RPI3, so I can verify that if you want.

sdmaclea · 2020-05-26T20:12:20Z

@janvorli I spent a lot of time staring at all the differences in the aarch64 source, because I hadn't realized the issue was with arm64 alpine. When I finally found out it was an issue with arm64 Alpine, I realized it had to be an issue with this macro stuff. It is the only thing that affects Alpine directly. I was going to trigger a outerloop build on this PR to make sure everything passed.

If you would prefer to test it locally on a RPI3 that would be helpful.

sdmaclea · 2020-05-27T01:22:14Z

The full rolling build passed on this PR. The libraries Libraries Test Run release coreclr Linux_musl arm64 Release was included in the run and passed. I believe this this suite caused the original revert.

I believe it can be reviewed and committed.
I can revert the eng pipeline change if desired before merging.

sdmaclea · 2020-05-27T16:30:48Z

We can open a PR in libunwind repo. Maintainer will review it (like they did for the previous fixes).

The maintainer pulled the alpine patch into master & 1.5 last night.

sdmaclea · 2020-06-01T17:31:23Z

@janvorli Can we merge this, or do you want me to change something?

sdmaclea · 2020-06-01T17:32:57Z

I'll remove the change to eng/pipelines/runtime.yml. Looks like @safern's patch dropped this change, so it needs to be removed.

edited

Rebased, removed the pipleline change, removed reverted patch. No functional changes.

am11 · 2020-06-01T17:43:07Z

src/coreclr/src/pal/src/exception/seh-unwind.cpp

@@ -111,6 +111,22 @@ static void WinContextToUnwindContext(CONTEXT *winContext, unw_context_t *unwCon
    unwContext->regs[13] = winContext->Sp;
    unwContext->regs[14] = winContext->Lr;
    unwContext->regs[15] = winContext->Pc;
+#elif defined(HOST_ARM64)


@sdmaclea, was it not required in the previous version of libunwind?

Between 1.3rc1 and 1.5rc2, libunwind changed the definition of unw_context_t. Therefore #if UNWIND_CONTEXT_IS_UCONTEXT_T changed from being evaluated as true to false. Now this code is used.

This is functionally identical to the code above.

static void WinContextToUnwindContext(CONTEXT *winContext, unw_context_t *unwContext) { #define ASSIGN_REG(reg) MCREG_##reg(unwContext->uc_mcontext) = winContext->reg; ASSIGN_UNWIND_REGS #undef ASSIGN_REG }

See lines 43 & 89 above.

janvorli

LGTM, thank you!

ghost · 2020-06-01T19:48:48Z

Hello @sdmaclea!

Because this pull request has the auto-merge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (`@msftbot`) and give me an instruction to get started! Learn more here.

Libunwind 1.5rc2 again (dotnet/runtime#36988) * Add arm64 support for UNWIND_CONTEXT_IS_UCONTEXT_T==0 * Reapply libunwind 1.5rc2 * Fix Linux Alpine libunwind1.5rc2 Add libunwind to cross DAC (dotnet/runtime#37521) * Libunwind v1.5-rc1-28-g9165d2a1 Pull upstream libunwind which supports building on Windows * Alignas and typos libunwind/libunwind#186 * Update libunwind-version.txt * Add libunwind to cross DAC * Colocate Unix/Windows compiler config * Unify Windows/Unix configure.cmake

+ Auto renames for (dotnet/coreclr/dotnet#26080) + Rename _WIN64 define to BIT64 (dotnet#26080) + Remove stray sos reference (dotnet/runtime/dotnet#1875) + Fix MSVC warn as errors... (dotnet/runtime/dotnet#1876) + Remove unused/unnecessary defines (dotnet/runtime/dotnet#1878) + Rename CLR_CMAKE_PLATFORM* CLR_CMAKE_HOST* (dotnet/runtime/#1974) + Rename PAL_CMAKE_* CLR_CMAKE_* (dotnet/runtime/dotnet#1984) + Add alpinedac & linuxdac build options (dotnet/runtime/dotnet#2056) + Refactor CMake system to allow cross OS DAC compile (dotnet/runtime/#2054) + Add FEATURE_REMOTE_PROC_MEM (dotnet/runtime/dotnet#2244) + Fix compilation of dbgtransportsession (dotnet/runtime/dotnet#2247) + Auto renames from (dotnet/runtime/#2256) + Rename cross compilation related defines (dotnet/runtime/#2256) + Fix logic to disable mscordbi build (dotnet/runtime/#31745) + Fix unused variable warning (dotnet/runtime/#31747) + Remove GetRecycleMemoryInfo from DAC builds (dotnet/runtime/#31748) + Fix check.* cross compilation linker errors (dotnet/runtime/#31751) + Reduce PAL DAC exports (dotnet/runtime/#31749) + Add EMPTY_BASES_DECL (dotnet/runtime/dotnet#26980) + Disable linux unwinder on Windows (dotnet/runtime/#31777) + Add cross OS support for MAKEDLLNAME (dotnet/runtime/#31746) + Fix host compiler when cross OS DAC compiling (dotnet/runtime/#31775) + Fix arm64singlestepper #ifdef (dotnet/runtime/#31776) + Configure host features based on host OS (dotnet/runtime/#31778) + Configure Host OS APIs based on HOST macros (dotnet/runtime/#31774) + Use common CLR_CMAKE_* variables in more places (dotnet/runtime/#31659) + Add T_CRITICAL_SECTION for cross OS DAC compile (dotnet/runtime/#31908) + Fix arm cross OS DAC compilation (dotnet/runtime/#31903) + Add CrossOS DAC build back (dotnet/runtime/#33064) + Fix typo (dotnet/runtime/#33192) + Fix type layout whan Cross OS compiling (dotnet/runtime/#33487) + Partial revert auto renames from (dotnet/runtime/dotnet#2056) + Partial revert (dotnet/runtime/dotnet#26080) + Enable cross OS DBI build(dotnet/runtime/#35021) + Disable EMPTY_BASE_DECL except for cross OS DAC + Fix DBI type layout issues + Add utilcode_dbi + Consistently define DBI features + Define T_CRITICAL_SECTION during non-DAC builds + Port cross DAC unwind support to 3.1 + Libunwind 1.5rc2 again (dotnet/runtime#36988) + Add libunwind to cross DAC (dotnet/runtime#37521)

sdmaclea added NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) NO-REVIEW Experimental/testing PR, do NOT review it labels May 25, 2020

sdmaclea self-assigned this May 25, 2020

Dotnet-GitSync-Bot added the area-Infrastructure label May 25, 2020

sdmaclea added area-PAL-coreclr and removed area-Infrastructure labels May 25, 2020

jaredpar mentioned this pull request May 25, 2020

OSX machines are de-provisioned during CI / PR runs leading to failures #34472

Closed

sdmaclea force-pushed the libunwindAgain branch from 679f67a to 5796a05 Compare May 26, 2020 18:22

sdmaclea changed the title ~~WIP Libunwind again~~ Libunwind 1.5rc2 again May 26, 2020

sdmaclea requested review from janvorli and safern May 26, 2020 18:36

sdmaclea removed NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) NO-REVIEW Experimental/testing PR, do NOT review it labels May 26, 2020

sdmaclea added 3 commits June 1, 2020 13:36

Add arm64 support for UNWIND_CONTEXT_IS_UCONTEXT_T==0

d512833

Reapply libunwind 1.5rc2

48fb021

Fix Linux Alpine libunwind1.5rc2

55cccae

sdmaclea force-pushed the libunwindAgain branch from 5796a05 to 55cccae Compare June 1, 2020 17:36

am11 reviewed Jun 1, 2020

View reviewed changes

janvorli approved these changes Jun 1, 2020

View reviewed changes

sdmaclea added the auto-merge label Jun 1, 2020

ghost merged commit 581dc19 into dotnet:master Jun 1, 2020

am11 mentioned this pull request Sep 4, 2020

Update THIRD-PARTY-NOTICES file #41851

Merged

sdmaclea deleted the libunwindAgain branch September 26, 2020 16:53

ghost locked as resolved and limited conversation to collaborators Dec 9, 2020

This pull request was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Libunwind 1.5rc2 again #36988

Libunwind 1.5rc2 again #36988

sdmaclea commented May 25, 2020 •

edited

Loading

ghost commented May 25, 2020

sdmaclea commented May 25, 2020

sdmaclea commented May 26, 2020

sdmaclea commented May 26, 2020 •

edited

Loading

am11 commented May 26, 2020

safern commented May 26, 2020

janvorli commented May 26, 2020

sdmaclea commented May 26, 2020

sdmaclea commented May 27, 2020

sdmaclea commented May 27, 2020 •

edited

Loading

sdmaclea commented Jun 1, 2020

sdmaclea commented Jun 1, 2020 •

edited

Loading

am11 Jun 1, 2020

sdmaclea Jun 1, 2020 •

edited

Loading

janvorli left a comment

ghost commented Jun 1, 2020

Libunwind 1.5rc2 again #36988

Libunwind 1.5rc2 again #36988

Conversation

sdmaclea commented May 25, 2020 • edited Loading

ghost commented May 25, 2020

sdmaclea commented May 25, 2020

sdmaclea commented May 26, 2020

sdmaclea commented May 26, 2020 • edited Loading

am11 commented May 26, 2020

safern commented May 26, 2020

janvorli commented May 26, 2020

sdmaclea commented May 26, 2020

sdmaclea commented May 27, 2020

sdmaclea commented May 27, 2020 • edited Loading

sdmaclea commented Jun 1, 2020

sdmaclea commented Jun 1, 2020 • edited Loading

am11 Jun 1, 2020

Choose a reason for hiding this comment

sdmaclea Jun 1, 2020 • edited Loading

Choose a reason for hiding this comment

janvorli left a comment

Choose a reason for hiding this comment

ghost commented Jun 1, 2020

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

sdmaclea commented May 25, 2020 •

edited

Loading

sdmaclea commented May 26, 2020 •

edited

Loading

sdmaclea commented May 27, 2020 •

edited

Loading

sdmaclea commented Jun 1, 2020 •

edited

Loading

sdmaclea Jun 1, 2020 •

edited

Loading

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (`@msftbot`) and give me an instruction to get started! Learn more here.