-
Notifications
You must be signed in to change notification settings - Fork 5.2k
Mono musl support #76500
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
Mono musl support #76500
Conversation
|
Tagging subscribers to this area: @directhex Issue DetailsDescriptionsAttempt at musl support for mono as to allow s390x on Alpine Linux. There seems to be issues with mono on musl. Outside perspective would be much appreciated. Related issues:
|
4c044f3 to
b94845d
Compare
29d3104 to
6a37c1c
Compare
|
The following bugs exist for mono on musl:
A fix for one of them likely involves setting stack size properly due to musl's really low default of 128K rather than the usual 8M. |
6a37c1c to
993447a
Compare
993447a to
98054ea
Compare
|
With progress done on #76805, I've got a full build of s390x working on dotnet6. They are still some issues on dotnet7, which indeed only occurs on |
b007d39 to
a1e180a
Compare
7602749 to
cec3371
Compare
|
On |
Can you share the actual full linker command line in effect when building |
Can you share the actual full linker command line in effect when building |
|
|
Thanks everyone, here it is: Indeed, --no-as-needed is after |
e8b2364 to
c1ccb3d
Compare
|
Fixed the issue via setting |
c1ccb3d to
45f82f7
Compare
|
Found a proper fix where |
45f82f7 to
969eaf8
Compare
969eaf8 to
53fca63
Compare
|
@akoeplinger Outside of resolving conflicts, how does this PR look? |
akoeplinger
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM thank you!
| if (sourceLength != 0) | ||
| { | ||
| while ((ret = ioctl(outFd, FICLONE, inFd)) < 0 && errno == EINTR); | ||
| while ((ret = ioctl(outFd, (int)FICLONE, inFd)) < 0 && errno == EINTR); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ayakael
Why FICLONE casted to int? Second argument of ioctl function is unsigned long and not int.
This caused ppc64le build error. Can you please revert this change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was a proposal by @am11, as build failed without setting Wno-sign-conversion. See: #76500 (comment).
Here is the exact error:
/var/build/dotnet7/testing/dotnet7-stage0/src/dotnet-e6dd91c290b808f971a1ac69c2fb29395bbf1051/src/runtime/src/native/libs/System.Native/pal_io.c(1270,36): error GBBD8A67C: implicit conversion changes signedness: 'unsigned long' to 'int' [-Wsign-conversion] [/var/build/dotnet7/testing/dotnet7-stage0/src/dotnet-e6dd91c290b808f971a1ac69c2fb29395bbf1051/src/runtime/src/native/libs/build-native.proj]
/var/build/dotnet7/testing/dotnet7-stage0/src/dotnet-e6dd91c290b808f971a1ac69c2fb29395bbf1051/src/runtime/src/native/libs/System.Native/pal_io.c(1270,36): error GBBD8A67C: implicit conversion changes signedness: 'unsigned long' to 'int' [-Wsign-conversion] [/var/build/dotnet7/testing/dotnet7-stage0/src/dotnet-e6dd91c290b808f971a1ac69c2fb29395bbf1051/src/runtime/src/native/libs/build-native.proj]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you change last argument to unsigned long here:
| #define FICLONE _IOW(0x94, 9, int) |
(int)FICLONE cast, does it fix both linux-ppc64le and linux-musl-ppc64le builds?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That would be incorrect - the _IOW macro uses sizeof its last argument as a component of the resulting value, so changing that size from 4 to 8 will result in an incorrect value of FICLONE.
This issue seems to be caused by some difference in the definition of ioctl with musl. In glibc, we have this:
extern int ioctl (int __fd, unsigned long int __request, ...);
Is this declared differently in musl?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is a ppc64le specific issue, where musl and glibc have different definitions. Rest of the platform matrix (including linux-musl-x64/arm64/arm etc..) is fine either way.
That would be incorrect - the _IOW macro uses sizeof
I see, so the current definition is also incorrect; should be size_t instead of int, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is a ppc64le specific issue, where musl and glibc have different definitions. Rest of the platform matrix (including linux-musl-x64/arm64/arm etc..) is fine either way.
The only difference between ppc64le and other platforms is the numerical value of FICLONE here. On ppc64le, this value is 0x80049409 while on other platforms it is 0x40049409 due to a different definition of the _IOW macro. This means on other platforms the value is the same interpreted as int and unsigned long, so there's no compiler warning. On ppc64le, the value is negative interpreted as int and positive interpreted as unsigned long, that's why we get the warning. The type mismatch itself is currently present on all platforms.
That would be incorrect - the _IOW macro uses sizeof
I see, so the current definition is also incorrect; should be
size_tinstead ofint, right?
No. We must use sizeof (int), i.e. 4, here; this flows into the construction of that value above.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This issue seems to be caused by some difference in the definition of
ioctlwith musl. In glibc, we have this:extern int ioctl (int __fd, unsigned long int __request, ...);Is this declared differently in musl?
Indeed this seems to be problem. In the musl headers I see instead:
int ioctl (int, int, ...);
I guess as long as the two headers differ, there's probably no way to fix the source to work with both libcs, except for using some #ifdef ...
|
Thank you for the fix! Would it be worthwhile to backport this and #82173 to |
|
@ayakael, could you tell a bit why the s390x support in 7.0 and 6.0 is important? Normally folks tend to use "next version" for new (previously unsupported) platforms / architectures, e.g. libunwind 1.6 added riscv64 support, and Alpine package enabled it for that version: alpinelinux/aports@15c3641. They did not backport to the previous version. This way, it is easy to remember which version added support for which architecture/platform; it is also something we can announce and include in the release notes etc. Same in the runtime repo that historically, we don't backport new platform support. It makes sense because it gives us some time to test and fix bugs / improve implementation. e.g. so far, there is no support for linux-musl-s390x in our cross infra ( |
When it comes to which platforms we support for dotnet, we try to keep parity with Fedora. Thus, once s390x was supported on Thanks for mentioning the cross infra needs, it reminds me that I need to complete the work on dotnet/arcade#11608 |
Descriptions
Attempt at musl support for mono as to allow s390x on Alpine Linux.
There seems to be issues with mono on musl. Outside perspective would be much appreciated.
Related issues:
System.InsufficientExecutionStackExceptionwhen building many packages (dotnet/roslyn#64423)Fixes #76805