Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor libunwind to work on osx #284

Merged
merged 39 commits into from Feb 18, 2015
Merged

Conversation

@kangaroo
Copy link

@kangaroo kangaroo commented Feb 17, 2015

Initial pass at OS X support.

It still unwinds on linux. It doesn't succeed on OS X yet, but its not failing to compile/crashing anymore.

Test:

basalt:debug plasma$ cat unw.cs 
 using System;

 class Program
    {
        static void A()
        {
            B();
        }

        static void B()
        {
            C();
        }

        static void C()
        {
            D();
        }

        static void D()
        {
            Console.WriteLine("About to unwind");
            var st = System.Environment.StackTrace;
            Console.WriteLine (st == null ? "null" : st);
            Console.WriteLine("Unwound");
        }

        static void Main(string[] args)
        {
            A();
        }
    }

Linux:

vagrant@vagrant-ubuntu-trusty-64:/vagrant/binaries/Product/amd64/debug$ uname -a
Linux vagrant-ubuntu-trusty-64 3.13.0-45-generic #74-Ubuntu SMP Tue Jan 13 19:36:28 UTC 2015 x86_64 x86_64 x86_64 GNU/Linux
vagrant@vagrant-ubuntu-trusty-64:/vagrant/binaries/Product/amd64/debug$ ./corerun -c . unw.exe 
About to unwind
   at System.Environment.GetStackTrace(Exception e, Boolean needFileInfo)
   at System.Environment.get_StackTrace()
   at Program.D()
   at Program.Main(String[] args)
Unwound

Mac:

basalt:debug plasma$ uname -a
Darwin basalt.local 14.1.0 Darwin Kernel Version 14.1.0: Mon Dec 22 23:10:38 PST 2014; root:xnu-2782.10.72~2/RELEASE_X86_64 x86_64
basalt:debug plasma$ ./corerun -c . unw.exe 
About to unwind

Unwound
poizan42 and others added 30 commits Feb 12, 2015
In the same vein as "COR" and "URT", the terms "COM+"/"COMPLUS" appear frequently throughout the codebase, most notably in config settings (e.g. "COMPLUS_DefaultVersion"). I couldn't find any other explanation for what this means, and since I remember learning about it myself by asking older members of the team, I thought it would be helpful to have this documented for newcomers to the codebase.
Add definition of "COMPLUS" to the glossary
Correctly locate Clang [Fixes 250]
Fix wrong flag to rm when deleting intermediates folder.
Update the CoreCLR nuspec to deploy the files under the aspnetcore5 folder
Any managed code that touches the ThreadPool is currently crashing when the ThreadPool tries to initialize. This is due to it creating a semaphore via CreateSemaphoreExA/W in the pal, and those functions not being implemented.

This commit just implements those functions by delegating to their non-Ex counterparts. With this change, ThreadPool.QueueUserWorkItem, Task.Run, etc. are able to successfully schedule work and have it executed.
The current PAL has binary check-ins of the config.h for Linux and MAC, long
term this is not sustainable.  This is a first pass at cleaning this up.
Please note, there are some static platform independent defines as I tried to
keep consistency with the current outputs of the not-checked-in configure.in.
As we clean up the PAL the CMakeLists should be revisited and redundant pieces
removed.
Implement CreateSemaphoreExA/W via CreateSemaphoreA/W
Simplify platform ifdefs like #if defined(_WIN64) || defined(_TARGET_ARM_) to #if !defined(_TARGET_X86_) based on @BruceForstall suggestion
When building debug, author the development package with an appropriate name
Refactor the PAL build configuration to detect at configure time
[tfs-changeset: 1415626]
di can be built, but still cannot be linked.
This commit removes a bunch of dead checks in the configure stage, as well
as fixing inconsistencies in HAVE_FTRUNCATE_LENGTH_ISSUE/HAVE_FTRUNCATE_LENGTH_BUG
off_t in file.cpp was declared after the goto, making it a protected scope, so it
needed to be moved up.
@kangaroo kangaroo force-pushed the kangaroo:unix_issue177 branch from 761d9f5 to 1fa9f00 Feb 17, 2015
@kangaroo
Copy link
Author

@kangaroo kangaroo commented Feb 17, 2015

@janvorli We've had some back and forth on this. Want me to rebase and squash it down?

@janvorli
Copy link
Member

@janvorli janvorli commented Feb 17, 2015

@kangaroo Yes please, that would be nice.

libunwind on linux exposes the unw_context_t directly as a
ucontext_t, but on OSX its an opaque data structure.  Additionally
UNW_REG_SP is read/write on OSX, but read-only on Linux.  As such
we need to diverge the libunwind code a bit depending on what flavor
we are running on.  There is one OSXTODO around the context pointers,
since we do not have unw_get_save_loc on OSX.
@kangaroo kangaroo force-pushed the kangaroo:unix_issue177 branch from 115dd5e to 551c976 Feb 17, 2015
@kangaroo
Copy link
Author

@kangaroo kangaroo commented Feb 17, 2015

Rebased into 551c976

@janvorli
Copy link
Member

@janvorli janvorli commented Feb 17, 2015

LGTM

@janvorli
Copy link
Member

@janvorli janvorli commented Feb 17, 2015

@kangaroo Thank you very much for making it work on OSX!

@kangaroo
Copy link
Author

@kangaroo kangaroo commented Feb 17, 2015

@janvorli No problem. Do we need 2 core members to sign this PR? Since its not targeting master (I submitted this against your branch), should we merge into there and continue review there?

Conflicts:
	src/pal/src/config.h.linux
	src/pal/src/config.h.osx
	src/pal/src/exception/seh-unwind.cpp
@janvorli
Copy link
Member

@janvorli janvorli commented Feb 17, 2015

I think we should merge right away and then I should reflect the feedback I got on my changes from @jkotas. But we'll have a cross platform weekly meeting in a minute, so I'll double check thatin there and will merge right after.

@kangaroo
Copy link
Author

@kangaroo kangaroo commented Feb 17, 2015

Alright, I'm rebasing the branch onto master currently anyways, so if that passes tests I'll push that up to this branch as a separate commit as well.

@kangaroo
Copy link
Author

@kangaroo kangaroo commented Feb 17, 2015

Build failure seems unrelated, something is flapping on windows.

@kangaroo
Copy link
Author

@kangaroo kangaroo commented Feb 17, 2015

@dotnet-bot test this please

@janvorli
Copy link
Member

@janvorli janvorli commented Feb 17, 2015

@mmitche There is something strange going on with the windows build. It fails with
C:\Program Files (x86)\MSBuild\Microsoft.Cpp\v4.0\V120\BuildCustomizations\masm.targets(50,5): error MSB4175: The task factory "XamlTaskFactory" could not be loaded from the assembly "Microsoft.Build.Tasks.v4.0, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a". Could not find file 'C:\Windows\TEMP\qhmbsmpu.dll'
Which I don't see possibly related to @kangaroo's change.
Do you have any idea what could be causing that?

@mmitche
Copy link
Member

@mmitche mmitche commented Feb 17, 2015

taking a look now

@mmitche
Copy link
Member

@mmitche mmitche commented Feb 17, 2015

@kangaroo The failure doesn't appear on master on one of those machines that failed, so I don't think it's a machine issue. I'm checking the feature branch now.

@mmitche
Copy link
Member

@mmitche mmitche commented Feb 18, 2015

@dotnet-bot test this please

I've been unable to reproduce the failure on that machine...attempting to get some more data now.

@kangaroo
Copy link
Author

@kangaroo kangaroo commented Feb 18, 2015

@mmitche It was failing on DEBUG before, now RELEASE, and my patch doesn't touch windows. It seems to be flapping for some reason.

@mmitche
Copy link
Member

@mmitche mmitche commented Feb 18, 2015

@kangaroo Your patch does touch a few of the build files, probably from merges, so it's a possibility that that's related somehow. But when I checked out your branch I got none of those failures..

@mmitche
Copy link
Member

@mmitche mmitche commented Feb 18, 2015

@dotnet-bot test this please

@kangaroo
Copy link
Author

@kangaroo kangaroo commented Feb 18, 2015

@mmitche yeah thats from the master rebase

@mmitche
Copy link
Member

@mmitche mmitche commented Feb 18, 2015

@kangaroo @janvorli I can continue investigating tomorrow, I have to head out. Although Jan may have a different opinion, since this isn't a merge into master I'm okay with this being merged for now if it's not going to break his workflow.

@janvorli
Copy link
Member

@janvorli janvorli commented Feb 18, 2015

@mmitche, @kangaroo I was just going to write the same as Matt. This is our feature branch, so let's just merge the change in.

@kangaroo
Copy link
Author

@kangaroo kangaroo commented Feb 18, 2015

And now it passed :)

janvorli added a commit that referenced this pull request Feb 18, 2015
Refactor libunwind to work on osx
@janvorli janvorli merged commit b2a0855 into dotnet:unix_issue177 Feb 18, 2015
1 check passed
1 check passed
@dotnet-bot
default Merged build finished.
Details
@kangaroo kangaroo deleted the kangaroo:unix_issue177 branch Feb 18, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet