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

Fix runtime problems on MacOS using M1 chips #2579

Merged
merged 12 commits into from Mar 2, 2022
Merged

Conversation

WojciechMazur
Copy link
Contributor

@WojciechMazur WojciechMazur commented Feb 27, 2022

This PR is aimed to fix #2494 and other runtime issues when using macOS with M1 (arm64) chips. Because currently, we can't have any self-hosted Mac/M1 based machines that could be used in the CI all of the introduced changes are being tested only locally on the M1 host.

  • Fix CVarArgs - it should use the same logic as proposed by @shadaj in the 32bit branch, but with different padding
  • Fix ProcessMonitor deadlocks - currently used dispatch_semapthore was leading to deadlocks, we replace it with named semaphores that would be used in both Mac and Linux.
  • Use TMPDIR env to determinate java.io.tmpdir system property - on MacOs 12 wiht M1 files created in /tmp directory have wrong permissions. We should create them in the path described by TMPDIR
  • Adapt TimeTest - TimeZone is not being abbreviated on Arm64 - same problems as in the FreeBSD
  • Fix failing java.io.FilesTest - 2 up to 4 tests are frequently failing due to bad descriptor or bad permissions
  • Fix failing java.io.SocketTest - Connecting to non-existing socket with timeout is blocking (for 1 min) and return incorrect exception
  • Fix failing scalanative.unsafe.ExternTest - incorrect results, probably due to incorrect Scala definition of snprintf method taking varargs (...), where positional args are being passed.

@ekrich
Copy link
Member

ekrich commented Feb 28, 2022

Ok, I ran this and got the following. Went from 137 test failures to 7.
M1.txt

finally out.close()
try {
val copyResult = copy(in, out)
// Make sure that created file has correct permissions
Copy link
Contributor Author

Choose a reason for hiding this comment

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

On MacOS M1 the files created by copying were given empty permissions, this should match the permission of files created by the JVM

Comment on lines +18 to +24
// The +1 accounts for the null char at the end of the name
#ifdef __APPLE__
#include <sys/posix_sem.h>
#define SEM_MAX_LENGTH PSEMNAMLEN + 1
#else
#include <limits.h>
#define SEM_MAX_LENGTH _POSIX_PATH_MAX + 1
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is and creation of semaphore is copy-pasted from Phase.c in Commix impl. In both cases this is needed due to problems on MacOs - usage of sem_create would return -1 and errno would mention not implemented error.
Also because previous dispatch_semaphore_t was leading to deadlocks on Arm64 we do replace both linux and mac implementation with shared named semaphore

Comment on lines +221 to +242
private def toCVarArgList_Arm64_MacOS(
varargs: Seq[CVarArg]
)(implicit z: Zone) = {
val alignedArgs = varargs.map { arg =>
arg.value match {
case value: Byte =>
value.toLong: CVarArg
case value: Short =>
value.toLong: CVarArg
case value: Int =>
value.toLong: CVarArg
case value: UByte =>
value.toULong: CVarArg
case value: UShort =>
value.toULong: CVarArg
case value: UInt =>
value.toULong: CVarArg
case value: Float =>
value.toDouble: CVarArg
case o => arg
}
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This implementation is exactly the same as for the x86 (in 32bit support branch), the only difference comes from mapping to Int64 instead of Int32

@WojciechMazur WojciechMazur marked this pull request as ready for review February 28, 2022 23:20
@WojciechMazur WojciechMazur merged commit 490f891 into main Mar 2, 2022
@WojciechMazur WojciechMazur deleted the fix/macos-m1 branch March 2, 2022 14:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CVarArgList not working correctly on macOS M1
2 participants