Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.

ARM: fix stack frame management #4641

Merged
merged 3 commits into from
May 3, 2016
Merged

ARM: fix stack frame management #4641

merged 3 commits into from
May 3, 2016

Conversation

myungjoo
Copy link

@myungjoo myungjoo commented Apr 28, 2016

Fix #4638

There have been a lot of ASM functions that may break stack unwinding.

Tested along with @parjong 's #4503 (merged)
and modified #4581 (appended in included in this PR)

Signed-off-by: MyungJoo Ham myungjoo.ham@samsung.com

@myungjoo
Copy link
Author

I'll continue working on from Line 799. there are more functions to be fixed.

@myungjoo
Copy link
Author

myungjoo commented Apr 28, 2016

Now, it seems that I've reviewed and corrected every single ASM/ARM function that is going to be unwinded.

I assume that JIT_Stelem_Ref_NotExactMatch is not going to be unwinded.

@myungjoo
Copy link
Author

TEST RESULTS:

bash-3.2# ./corerun csc.exe /lib:. /r:mscorlib.dll throw.cs
Microsoft (R) Visual C# Compiler version 42.42.42.42
Copyright (C) Microsoft Corporation. All rights reserved.

throw.cs(6,42): warning CS7095: Filter expression is a constant, consider removing the filter
throw.cs(6,31): warning CS0168: The variable 'e' is declared but never used
bash-3.2# ./corerun throw.exe
Hello World!
bash-3.2# cat throw.cs
namespace ConsoleApplication { public class Program {

  public static void Main(string[] args) {
    try {
      throw new System.Exception("Throw");
    } catch (System.Exception e)  when ( true ) {
      System.Console.WriteLine("Hello World!");
    }
  }

} }
bash-3.2# ./corerun seh_tests.exe 
Testcase successful
bash-3.2# uname -r -o
3.10.65 GNU/Linux
bash-3.2# cat /proc/cpuinfo
Processor   : ARMv7 Processor rev 5 (v7l)
processor   : 0
model name  : ARMv7 Processor rev 5 (v7l)
BogoMIPS    : 2002.62
Features    : swp half thumb fastmult vfp edsp neon vfpv3 tls vfpv4 idiva idivt vfpd32 
CPU implementer : 0x41
CPU architecture: 7
CPU variant : 0x0
CPU part    : 0xc07
CPU revision    : 5

Hardware    : Z3
Revision    : 0000
Serial      : 0000c6bf00009200
bash-3.2#

Based: dfcb7f0 + #4503 + Modified #4581.

Modified #4581 is:

diff --git a/src/vm/arm/ehhelpers.S b/src/vm/arm/ehhelpers.S
index dfe1b11..c32a06b 100644
--- a/src/vm/arm/ehhelpers.S
+++ b/src/vm/arm/ehhelpers.S
@@ -47,6 +47,8 @@ OFFSET_OF_FRAME=(4 + SIZEOF__GSCookie)
         //
         // Runtime check for 8-byte alignment. 
         PROLOG_STACK_SAVE r7
+        // We lose stack unwindability here by configuring fp(r7) incorrectely
+        // here.
         and r0, r7, #4
         sub sp, sp, r0

@@ -99,6 +101,7 @@ OFFSET_OF_FRAME=(4 + SIZEOF__GSCookie)
         NESTED_ENTRY CallEHFunclet, _TEXT, NoHandler

         PROLOG_PUSH  "{r4-r11, lr}"
+        PROLOG_STACK_SAVE_OFFSET  r7, #12
         alloc_stack  4

         // On entry:
@@ -124,7 +127,8 @@ OFFSET_OF_FRAME=(4 + SIZEOF__GSCookie)
         // frame pointer for accessing the locals in the parent method.
         NESTED_ENTRY CallEHFilterFunclet, _TEXT, NoHandler

-        PROLOG_PUSH  "{lr}"
+        PROLOG_PUSH  "{r7, lr}"
+        PROLOG_STACK_SAVE r7
         alloc_stack  4

         // On entry:
@@ -140,6 +144,6 @@ OFFSET_OF_FRAME=(4 + SIZEOF__GSCookie)
         blx r2

         free_stack   4
-        EPILOG_POP   "{pc}"
+        EPILOG_POP   "{r7, pc}"

         NESTED_END CallEHFilterFunclet, _TEXT

@myungjoo myungjoo changed the title [WIP] ARM: fix stack frame management ARM: fix stack frame management Apr 28, 2016
@myungjoo
Copy link
Author

myungjoo commented May 3, 2016

@dotnet-bot test OSX x64 Checked Build Please

@myungjoo
Copy link
Author

myungjoo commented May 3, 2016

As @parjong's #4581 looks better if based on this PR, I'm importing and rebasing his patch and putting here. (will be appended to this PR soon)

@myungjoo myungjoo force-pushed the fix/4638 branch 4 times, most recently from e21618e to 6019ca3 Compare May 3, 2016 06:02
myungjoo and others added 2 commits May 3, 2016 15:02
Fix #4638

Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
This commit revises the implementation of CallEHFunclet to establish
stack frame appropriately.

This commit allows libunwind to work correctly for the frame created by
CallEHFunclet.

This revises the PR suggested by dotnet#4581, which fixes #4579

Edited-by: MyungJoo Ham <myungjoo.ham@samsung.com>
@@ -287,7 +287,10 @@ LOCAL_LABEL(LNullThis):
//
NESTED_ENTRY TheUMEntryPrestub,_TEXT,NoHandler

push {r0-r4,lr}
.save {r0-r4,r7,r8,lr}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we use PROLOG_PUSH followed by PROLOG_STACK_SAVE_OFFSET here and at several other places below? We have a convention to not to use the unwinder pseudoinstructions directly in the .S files.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure we can. The patchset is updated to use the macro.

Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
@myungjoo
Copy link
Author

myungjoo commented May 3, 2016

@dotnet-bot Test OSX x64 Checked Build and Test Please

@janvorli
Copy link
Member

janvorli commented May 3, 2016

LGTM, thanks!

@parjong
Copy link

parjong commented May 3, 2016

@janvorli OSX x64 Checked Build and Test failed with "No space left on device" error as follows:

BEGIN EXECUTION /Users/dotnet-bot/j/workspace/dotnet_coreclr/master/checked_osx_tst_prtest/bin/tests/Windows_NT.x64.Checked/Tests/coreoverlay/corerun Collect_Default_1.exe 0 {0x5943147-0x10cc56480} ASSERT [PROCESS] at /Users/dotnet-bot/j/workspace/dotnet_coreclr/master/checked_osx_prtest/src/pal/src/thread/process.cpp.1848: sem_open(/clrct000144800000000057289b62) failed: 28 (No space left on device) ./Collect_Default_1.sh: line 62: 83072 Trace/BPT trap: 5 $_DebuggerFullPath "$CORE_ROOT/corerun" Collect_Default_1.exe $CLRTestExecutionArguments $Host_Args Expected: 100 Actual: 133 END EXECUTION - FAILED

Is there any issue with test VM?

@janvorli
Copy link
Member

janvorli commented May 3, 2016

@parjong yes, we have issues with OSX CI machines at the moment, most of the test runs are failing. The issue is not a disk space problem though, it is a semaphore leak that was accumulating over time and now it has hit.
The issue is being worked on.

@parjong
Copy link

parjong commented May 3, 2016

@janvorli Thanks you for comment 👍

@janvorli
Copy link
Member

janvorli commented May 3, 2016

I am going to merge this in since the change is completely unrelated to OSX and doesn't touch any OSX related files.

@janvorli janvorli merged commit 57185b4 into dotnet:master May 3, 2016
picenka21 pushed a commit to picenka21/runtime that referenced this pull request Feb 18, 2022
ARM: fix stack frame management

Fix dotnet/coreclr#4638
This commit revises the implementation of CallEHFunclet to establish
stack frame appropriately.
This commit allows libunwind to work correctly for the frame created by
CallEHFunclet.

Commit migrated from dotnet/coreclr@57185b4
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants