Port CoreCLR to SunOS #35173

am11 · 2020-04-19T03:44:36Z

With these changes, CoreCLR compiles on SmartOS 20190829T000927Z x86_64, Global Zone (GZ).

Libunwid changes are left out from this PR branch, as it is awaiting @jasonbking's PR merge upstream with Illumos fixes: libunwind/libunwind#171. I have a separate branch that has latest libunwind with Jason's PR changes, and CoreCLR CMake config updates: am11@3180b6c.

Here are the steps to build with libunwind changes branch:

# new machine, after bootstrapping pkgsrc

# dependencies
sudo pkgin -y update
sudo pkgin -y install git gcc7 mozilla-rootcerts cmake icu py37-expat gmake

git clone https://github.com/am11/runtime -b feature/solaris/coreclr-port --depth 1 --single-branch
cd runtime
./src/coreclr/build-runtime.sh -gcc

cc @jkotas, @janvorli

Contributes to #34944

ghost · 2020-04-19T03:44:43Z

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

am11 · 2020-04-19T03:47:50Z

src/coreclr/src/inc/crosscomp.h

 #else
 #warning
-#error  DAC_CS_NATIVE_DATA_SIZE is not defined for this architecture
+#error  DAC_CS_NATIVE_DATA_SIZE is not defined for this architecture. This should be same value as PAL_CS_NATIVE_DATA_SIZE (aka sizeof(PAL_CS_NATIVE_DATA)).


@smadala, I have appended the error message.

I guess you wanted to notify @sdmaclea instead

am11 · 2020-04-19T03:54:44Z

src/coreclr/src/pal/inc/pal.h

+inline WCHAR *PAL_wcspbrk(WCHAR* S, const WCHAR* P)
+        {return ((WCHAR *)PAL_wcspbrk((const WCHAR *)S, P)); }
+inline WCHAR *PAL_wcsstr(WCHAR* S, const WCHAR* P)
+        {return ((WCHAR *)PAL_wcsstr((const WCHAR *)S, P)); }


_S etc. are defined by some system headers, so renamed these without underscore, rather than undef'ing.

am11 · 2020-04-19T03:55:12Z

src/coreclr/src/pal/inc/pal.h

+PALIMPORT float __cdecl atanf(float)
+#ifndef __sun
+THROW_DECL
+#endif


Some of these do not have throw() semantics on Illumos.

am11 · 2020-04-19T09:37:58Z

src/coreclr/src/pal/src/misc/sysinfo.cpp

@@ -221,6 +225,8 @@ GetSystemInfo(
    lpSystemInfo->lpMaximumApplicationAddress = (PVOID) VM_MAXUSER_ADDRESS;
 #elif defined(__linux__)
    lpSystemInfo->lpMaximumApplicationAddress = (PVOID) (1ull << 47);
+#elif defined(__sun)
+    lpSystemInfo->lpMaximumApplicationAddress = (PVOID) 0xfffffd7fffe00000ul;


This value was copied from https://github.com/illumos/illumos-gate/blob/4e0c5eff9af325c80994e9527b7cb8b3a1ffd1d4/usr/src/uts/i86pc/sys/machparam.h#L235, unfortunately that header is not included in the base box. After some discussion in #illumos channel over at freenode IRC, we may get a helper function like getusrstack() (similar to getpagesize()) in the future, rather than a hardcoded value macro.

So on SunOS, the user code can live even in such a high address range?

Hi @janvorli, that's correct (at least as far as illumos is concerned, I work on that but cannot speak for Oracle Solaris). Though this value can shift a bit. For example, on my system it's currently 0xfffffc7fffe00000 and that's because @am11 used a slightly older base of illumos where we had to shift that due to part of our KPTI mitigations. However, it is different in different situations. For example if running inside of Xen pv (though that's rather uncommon) that today is 0x00007fffffe00000, e.g. below the VA hole. Those are values on x86. On SPARC right now the user limit is 0xFFFFFFFF80000000.

I'm looking at adding a sysconf style interface to get the maximum mapable address. I suspect on other systems with the advent of 5-level paging the maximum address may shift, though that may not matter for most consumers of the CLR. I would suggest limiting it at the VA hole on amd64 until such time as we have that available and falling back to the VA hole when not available.

am11 · 2020-04-19T10:03:23Z

CI failures are unrealted:

No space left on device on FreeBSD leg; failed on AzDO but showing in progress on GH.
opened issue for Libraries Test Run release mono Windows_NT x64 Debug: System.Xml.Xsl.XslTransformApi.Tests failed on netcoreapp5.0-Windows_NT-Debug-x64-Mono_release-Windows.7.Amd64.Open #35177 (dotnet(1) crashed running XML tests, recurring failure since Feb, but i could not find any existing reports)

src/coreclr/src/dlls/dbgshim/CMakeLists.txt

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

src/coreclr/src/pal/src/thread/context.cpp

janvorli

LGTM, thank you!

…wind-changes

rmustacc · 2020-04-21T15:23:45Z

Hi @janvorli, @am11, it looks like this went back (or at least still has) with the wrong value for the maximum usable address. I would still recommend tracking the VA hole here, but if you want to use the maximum value that can change, I'd strongly recommend at least using the current max on x86, instead of one from a few years ago (before KPTI).

am11 · 2020-04-21T15:37:03Z

@rmustacc, I took the lpMaximumApplicationAddress value from latest master of Illumos-gate USERLIMIT, is it incorrect?

rmustacc · 2020-04-21T15:50:58Z

@rmustacc, I took the lpMaximumApplicationAddress value from latest master of Illumos-gate USERLIMIT, is it incorrect?

So at least on systems I was looking at the, the actual value that was being applied was the lower one I mentioned. The fundamental problem is that this isn't a committed value. If things change, the maximum address will be pushed down and if a hardcoded value is used, things may change. I guess the problem with using the hole address is that it may mean the initial stack is outside of this.

am11 · 2020-04-21T22:22:14Z

@rmustacc, thanks. I have restricted this value to USERLIMIT32, the lowest value on x64.

I'm looking at adding a sysconf style interface to get the maximum mapable address.

Once we have such interface available, we can replace this hard-coded value with the function call. I have added a comment to that effect.

rmustacc · 2020-04-21T22:27:45Z

On 4/21/20 3:22 PM, Adeel Mujahid wrote: @rmustacc, thanks. I have restricted this value to `USERLIMIT32`, the lowest value on x64. > I'm looking at adding a sysconf style interface to get the maximum mapable address. Once we have such interface available, we can replace this hard-coded value with the function call. I have added a comment to that effect.

I'm not sure that limiting it to a 32-bit address space on a 64-bit process makes sense, especially when ASLR and other factors are in place. If this was targeting a 32-bit target, sure, but for a 64-bit target, that seems overly restrictive. When the .net runtime loads libraries and other things it won't be limited to 32-bits unless special flags are passed to mmap, etc. Does the allocator and other things which are trying to understand the address space handle the fact that the system will map things outside of it? I think you really want to understand that behavior. If the answer is that it's OK with the fact that things will be loaded outside of it, then I'd use the low side of the VA hole as a maximum value. If it won't, then it probably would be better to fall back into using a higher maximum value and hoping that the system doesn't allocate there on the time when you end up on a system with a slightly lower max.

janvorli · 2020-04-21T23:01:37Z

I'd use the low side of the VA hole as a maximum value

I agree, we should not limit it to 32 bits. Basically, this needs to be the maximum address any memory mapped into the user process can have. It is currently used at three places. At ClrVirtualAllocWithinRange, ClrVirtualAlloc and isRetAddr. The first one tries to allocate virtual memory from a specified range clipped by the memory top and bottom. This is not critical, if it fails to get memory allocated from the range, it will use whatever mmap returns. The second is just for debugging purposes. The third one needs a real top limit. It is used to check that an address in code is below the maximum address.

am11 · 2020-04-21T23:08:09Z

@janvorli, thanks. I have reverted the last commit so we use the current highest value defined in illumos-gate: https://github.com/illumos/illumos-gate/blob/4e0c5eff9af325c80994e9527b7cb8b3a1ffd1d4/usr/src/uts/i86pc/sys/machparam.h#L235.

Dotnet-GitSync-Bot added the area-Infrastructure label Apr 19, 2020

am11 commented Apr 19, 2020

View reviewed changes

Port CoreCLR to SunOS

5e68668

am11 marked this pull request as ready for review April 19, 2020 07:42

am11 commented Apr 19, 2020

View reviewed changes

am11 mentioned this pull request Apr 19, 2020

System.Xml.Xsl.XslTransformApi.Tests failed on netcoreapp5.0-Windows_NT-Debug-x64-Mono_release-Windows.7.Amd64.Open #35177

Closed

am11 mentioned this pull request Apr 20, 2020

Solaris ld raises reloc errors in JIT_WriteBarrier_Debug and ThePreStub #35154

Closed

janvorli reviewed Apr 20, 2020

View reviewed changes

Address PR feedback

d5ef2f9

am11 mentioned this pull request Apr 20, 2020

Fix PAL test build on non-Linux Unix platforms #35218

Merged

janvorli approved these changes Apr 21, 2020

View reviewed changes

Merge branch 'master' into feature/solaris/coreclr-port-without-libun…

c4bc754

…wind-changes

This was referenced Apr 21, 2020

Builds legs getting abandoned due to agent notification issues #35223

Closed

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

Closed

janvorli merged commit 84ec25f into dotnet:master Apr 22, 2020

am11 deleted the feature/solaris/coreclr-port-without-libunwind-changes branch April 22, 2020 11:49

am11 mentioned this pull request May 9, 2020

Tracking issue for illumos and Solaris x86-64 port work #34944

Open

19 tasks

janvorli mentioned this pull request Nov 11, 2020

how to build release version of coreclr for android? #33237

Open

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Port CoreCLR to SunOS #35173

Port CoreCLR to SunOS #35173

am11 commented Apr 19, 2020

ghost commented Apr 19, 2020

am11 Apr 19, 2020

janvorli Apr 20, 2020

am11 Apr 19, 2020

am11 Apr 19, 2020

am11 Apr 19, 2020

janvorli Apr 20, 2020

rmustacc Apr 20, 2020

am11 commented Apr 19, 2020

janvorli left a comment

rmustacc commented Apr 21, 2020 •

edited

Loading

am11 commented Apr 21, 2020

rmustacc commented Apr 21, 2020

am11 commented Apr 21, 2020

rmustacc commented Apr 21, 2020 via email

janvorli commented Apr 21, 2020

am11 commented Apr 21, 2020

Port CoreCLR to SunOS #35173

Port CoreCLR to SunOS #35173

Conversation

am11 commented Apr 19, 2020

ghost commented Apr 19, 2020

am11 Apr 19, 2020

Choose a reason for hiding this comment

janvorli Apr 20, 2020

Choose a reason for hiding this comment

am11 Apr 19, 2020

Choose a reason for hiding this comment

am11 Apr 19, 2020

Choose a reason for hiding this comment

am11 Apr 19, 2020

Choose a reason for hiding this comment

janvorli Apr 20, 2020

Choose a reason for hiding this comment

rmustacc Apr 20, 2020

Choose a reason for hiding this comment

am11 commented Apr 19, 2020

janvorli left a comment

Choose a reason for hiding this comment

rmustacc commented Apr 21, 2020 • edited Loading

am11 commented Apr 21, 2020

rmustacc commented Apr 21, 2020

am11 commented Apr 21, 2020

rmustacc commented Apr 21, 2020 via email

janvorli commented Apr 21, 2020

am11 commented Apr 21, 2020

rmustacc commented Apr 21, 2020 •

edited

Loading