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

Add support for Alpine Linux #7440

Merged
merged 1 commit into from Oct 4, 2016

Conversation

Projects
None yet
7 participants
@janvorli
Copy link
Member

commented Oct 1, 2016

This change enables build of CoreCLR on Alpine Linux. Here is the list
of changes:

  • Disable asserts checking RSP in arbitrary threads against cached stack limit
    for the respective thread. The stack on Alpine obviously grows over the limit
    reported by the pthread functions.
  • Disable using XSTATE on Alpine. This should be re-enabled after MUSL gets the _xstate,
    _fpx_sw_bytes and related data structures added to the signal.h header.
  • Disable setting rlimit of RLIMIT_NOFILE to the max value, since it breaks
    debugging for some reason.
  • Add skipping over the hardware signal trampoline in the PAL_VirtualUnwind.
    While we were not trying to walk over it in a simple case, in a case where
    an exception was thrown from a catch handler of a hardware exception, we
    still attempted to walk over it and it fails on Alpine.
  • Fix detection of Alpine Linux in the PAL's CMakeLists.txt so that it works
    in Docker containers too.
  • Modified PAL_VirtualUnwind to make the check for unwinding past the bottom
    of the stack unconditional. We had a long list of platforms where we were
    doing this check and it doesn't hurt to do it on platforms where it is not
    needed. I have done that rather than adding a check for Alpine Linux as
    another platform that needs it.
@janvorli

This comment has been minimized.

Copy link
Member Author

commented Oct 1, 2016

@adityamandaleeka can you take a look please?
cc: @gkhanna79, @Petermarcu

@janvorli janvorli force-pushed the janvorli:enable-alpine-linux-2 branch 2 times, most recently from e91748e to 5b86f23 Oct 3, 2016

@@ -61,6 +61,12 @@ if (CLR_CMAKE_PLATFORM_UNIX)

endif(CLR_CMAKE_PLATFORM_UNIX)

if(CLR_CMAKE_PLATFORM_ALPINE_LINUX)
# Alpine Linux doesn't have fixed stack limit, this define disables some stack pointer

This comment has been minimized.

Copy link
@gkhanna79

gkhanna79 Oct 3, 2016

Member

Is this confirmed to be the OS behavior?

This comment has been minimized.

Copy link
@janvorli

janvorli Oct 3, 2016

Author Member

I have written a simple test that basically gets the stack limit and size, then calls alloca to allocate as much as the stack size returned and then writes to the lowest byte of the allocated zone. On Ubuntu, it crashes with SIGSEGV, on Alpine, it passes and I can see in GDB (info proc map) that the stack got extended.

@@ -70,6 +63,16 @@ elseif(PAL_CMAKE_PLATFORM_ARCH_ARM64)
add_definitions(-D_WIN64=1)
endif()

if(CMAKE_SYSTEM_NAME STREQUAL Linux AND NOT CLR_CMAKE_PLATFORM_ALPINE_LINUX)

This comment has been minimized.

Copy link
@gkhanna79

gkhanna79 Oct 3, 2016

Member

Do we need the Linux check?

This comment has been minimized.

Copy link
@janvorli

janvorli Oct 3, 2016

Author Member

Yes, we don't support that on other OS-es like FreeBSD, NetBSD or OSX. They implement this differently.

{
ULONG contextFlags = CONTEXT_CONTROL | CONTEXT_INTEGER | CONTEXT_FLOATING_POINT | CONTEXT_EXCEPTION_ACTIVE;

#if defined(_AMD64_)

This comment has been minimized.

Copy link
@gkhanna79

gkhanna79 Oct 3, 2016

Member

Should this be under XSTATE_SUPPORTED?

This comment has been minimized.

Copy link
@janvorli

janvorli Oct 3, 2016

Author Member

It doesn't need to. The ContextFromNativeContext clears the flag when it is not supported.

@@ -132,6 +132,13 @@ if(CMAKE_SYSTEM_NAME STREQUAL Linux)
clr_unknown_arch()
endif()
set(CLR_CMAKE_PLATFORM_LINUX 1)

# Detect Alpine Linux
file(READ "/etc/os-release" OS_RELEASE)

This comment has been minimized.

Copy link
@adityamandaleeka

adityamandaleeka Oct 3, 2016

Member

Does this gracefully handle the case where this file doesn't exist (I don't know if there are any distros where that's the case)?

This comment has been minimized.

Copy link
@janvorli

janvorli Oct 3, 2016

Author Member

Good point. I have assumed file read just returns an empty variable, but it actually fails instead. So I am adding a check for the file existence.

endif(CMAKE_SYSTEM_NAME STREQUAL Linux AND NOT CLR_CMAKE_PLATFORM_ALPINE_LINUX)

if(CLR_CMAKE_PLATFORM_ALPINE_LINUX)
# Setting RLIMIT_NOFILE breaks debugging of coreclr on Alpine Linux for some reason

This comment has been minimized.

Copy link
@adityamandaleeka

adityamandaleeka Oct 3, 2016

Member

for some reason

Do we understand the reason? I assume we're not creating more file descriptors than we're setting the limit to?

This comment has been minimized.

Copy link
@janvorli

janvorli Oct 3, 2016

Author Member

So, this is what happens:

999         result = getrlimit(RLIMIT_NOFILE, &rlp);
(gdb) n
1000        if (result != 0)
(gdb)
1006        rlp.rlim_cur = rlp.rlim_max;
(gdb) p/x rlp
$1 = {rlim_cur = 0x80000, rlim_max = 0x100000}
(gdb) n
1007        result = setrlimit(RLIMIT_NOFILE, &rlp);
(gdb)

Thread 3 "corerun" received signal ?, Unknown signal.
[Switching to LWP 28373]
0x00007ffff7da8087 in syscall () from /lib/ld-musl-x86_64.so.1

See - I've just stepped over the setrlimit. From that time on, I cannot do anything in GDB - the "c" doesn't work etc. Btw, the same thing happens even if I don't step through the code and just run it under GDB.
If I run it out of the GDB, I haven't noticed any problem.
I've even tried to lower the rlim_max by 1 before assigning it to the rlim_cur, but it didn't help.
It is interesting that LLDB doesn't have this problem.
Anyways, it seems that the default 524288 file descriptors are enough.

This comment has been minimized.

Copy link
@sergiy-k

sergiy-k Oct 4, 2016

Contributor

We should at least open an issue to investigate and understand this problem later.

This comment has been minimized.

Copy link
@janvorli

janvorli Oct 4, 2016

Author Member

@sergiy-k it makes sense. One interesting thing that I've just tried is that if I have a simple testing app that just does the getrlimit / setrlimit like we do here, it doesn't have this effect on GDB. So maybe it is because we are calling it from a shared library or something else.
Btw, On Ubuntu, the rlim_cur is 0x400 and rlim_max is 0x1000. So on Alpine, even the rlim_cur is 128 times higher than the rlim_max on Ubuntu. So we really don't need to fiddle with it on Alpine.

This comment has been minimized.

Copy link
@janvorli

janvorli Oct 4, 2016

Author Member

I've created issue #7487 to track it.

This comment has been minimized.

Copy link
@sergiy-k

sergiy-k Oct 4, 2016

Contributor

@janvorli, thanks a lot! The main reason I asked is just to make sure there is no other problem hidden behind this.

This comment has been minimized.

Copy link
@sergiy-k

sergiy-k Oct 4, 2016

Contributor

@janvorli, thanks a lot! The main reason I asked is just to make sure there is no other problem hidden behind this.

@@ -468,6 +472,19 @@ inline static void CONTEXTSetPC(LPCONTEXT pContext, DWORD64 pc)
#endif
}

inline static DWORD64 CONTEXTGetFP(LPCONTEXT pContext)

This comment has been minimized.

Copy link
@adityamandaleeka

adityamandaleeka Oct 3, 2016

Member

Up to you, but I think 'CONTEXTGetFP' could be a little confusing since we use FP to refer to floating-point stuff in a lot of our context-handling code.

This comment has been minimized.

Copy link
@janvorli

janvorli Oct 3, 2016

Author Member

I understand, but we already have GetPC and GetSP, so naming it e.g. GetFramePointer would look a bit out of line.

This comment has been minimized.

Copy link
@adityamandaleeka

adityamandaleeka Oct 3, 2016

Member

Yea, I noticed that as well. I guess it's fine as is. If it's problematic we can rename it later.

Add support for Alpine Linux
This change enables build of CoreCLR on Alpine Linux. Here is the list
of changes:
- Disable asserts checking RSP in arbitrary threads against cached stack limit
  for the respective thread. The stack on Alpine obviously grows over the limit
  reported by the pthread functions.
- Disable using XSTATE. This should be re-enabled after MUSL gets the _xstate,
  _fpx_sw_bytes and related data structures added to the signal.h header.
- Disable setting rlimit of RLIMIT_NOFILE to the max value, since it breaks
  debugging for some reason.
- Add skipping over the hardware signal trampoline in the PAL_VirtualUnwind.
  While we were not trying to walk over it in a simple case, in a case where
  an exception was thrown from a catch handler of a hardware exception, we
  still attempted to walk over it and it fails on Alpine.
- Fix detection of Alpine Linux in the PAL's CMakeLists.txt so that it works
  in Docker containers too.
- Modified PAL_VirtualUnwind to make the check for unwinding past the bottom
  of the stack unconditional. We had a long list of platforms where we were
  doing this check and it doesn't hurt to do it on platforms where it is not
  needed. I have done that rather than adding a check for Alpine Linux as
  another platform that needs it.

@janvorli janvorli force-pushed the janvorli:enable-alpine-linux-2 branch from 5b86f23 to ddd6f49 Oct 3, 2016

@adityamandaleeka

This comment has been minimized.

Copy link
Member

commented Oct 3, 2016

LGTM

@janvorli janvorli merged commit 9c50604 into dotnet:master Oct 4, 2016

12 checks passed

CentOS7.1 x64 Debug Build and Test Build finished.
Details
FreeBSD x64 Checked Build Build finished.
Details
Linux ARM Emulator Cross Debug Build Build finished.
Details
Linux ARM Emulator Cross Release Build Build finished.
Details
OSX x64 Checked Build and Test Build finished.
Details
Ubuntu x64 Checked Build and Test Build finished.
Details
Ubuntu x64 Formatting Build finished.
Details
Windows_NT x64 Debug Build and Test Build finished.
Details
Windows_NT x64 Formatting Build finished.
Details
Windows_NT x64 Release Priority 1 Build and Test Build finished.
Details
Windows_NT x86 legacy_backend Checked Build and Test Build finished.
Details
Windows_NT x86 ryujit Checked Build and Test Build finished.
Details

@janvorli janvorli deleted the janvorli:enable-alpine-linux-2 branch Oct 4, 2016

@janvorli

This comment has been minimized.

Copy link
Member Author

commented Nov 2, 2016

@cydhaselton could you please share more detail on how it breaks the build?

@cydhaselton

This comment has been minimized.

Copy link
Contributor

commented Nov 3, 2016

@janvorli Unfortunately, since the time I posted that comment, I've into other build issues. I removed the sources and started over with an earlier revison.

Comment deleted. If I revisit this version and run into the error again I'll post more detail.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.