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.

Sign up for GitHub

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

[NativeAOT] Initial osx-arm64 bring up #75264

Merged
merged 19 commits into from
Sep 11, 2022
Merged

[NativeAOT] Initial osx-arm64 bring up #75264

Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
ca95d4b
Enable NativeAOT osx-arm64 builds
filipnavara Aug 31, 2022
35a40f0
Restrict alignment in VirtualReserveInner
filipnavara Aug 31, 2022
387b31b
Fix generation and handling of compact unwinding info on osx-arm64
filipnavara Aug 31, 2022
50eb6f6
Fix PC check in findFDE in DWARF CFI unwinding
filipnavara Sep 6, 2022
cb81686
Handle MAP_JIT for P/Invoke on osx-arm64
filipnavara Sep 7, 2022
69fd921
Handle P/Invoke with MAP_JIT on osx-arm64
filipnavara Sep 7, 2022
b05ba08
Fix incorrect OS_PAGE_SIZE definition
filipnavara Sep 7, 2022
8742268
Fix TLS register trashing
filipnavara Sep 8, 2022
92a7164
Fix memory trashing caused by incorrect PREPARE_EXTERNAL_VAR_INDIRECT…
filipnavara Sep 8, 2022
6325ba7
Ignore ESRCH in PalHijack (thread is already gone)
filipnavara Sep 8, 2022
125bed7
Make CompactUnwinder_* parametrized with registry class
filipnavara Sep 8, 2022
817b10c
Remove custom CompactUnwinder
filipnavara Sep 8, 2022
f44843f
Update src/coreclr/nativeaot/Runtime/CommonMacros.h
filipnavara Sep 8, 2022
3308493
Update llvm-libunwind-version.txt
filipnavara Sep 8, 2022
8b9d0dc
Fix initial alignment for __module_initializer. ARM64 requires absolu…
filipnavara Sep 9, 2022
9d73bfa
Allow publishing with the osx-arm64 RID
filipnavara Sep 9, 2022
5244049
Use pointer sized alignment for fat function pointers since they cont…
filipnavara Sep 9, 2022
59b79b3
Remove __builtin___clear_cache, it was already fixed
filipnavara Sep 11, 2022
d8c7ee0
Update src/coreclr/nativeaot/Runtime/unix/PalRedhawkUnix.cpp
jkotas Sep 11, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion eng/Subsets.props
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@

<PropertyGroup>
<!-- CLR NativeAot only builds in a subset of the matrix -->
<NativeAotSupported Condition="('$(TargetOS)' == 'windows' or '$(TargetOS)' == 'linux' or '$(TargetOS)' == 'OSX') and ('$(TargetArchitecture)' == 'x64' or '$(TargetArchitecture)' == 'arm64') and ('$(TargetOS)' != 'OSX' or '$(TargetArchitecture)' != 'arm64')">true</NativeAotSupported>
<NativeAotSupported Condition="('$(TargetOS)' == 'windows' or '$(TargetOS)' == 'linux' or '$(TargetOS)' == 'OSX') and ('$(TargetArchitecture)' == 'x64' or '$(TargetArchitecture)' == 'arm64')">true</NativeAotSupported>

<!-- If we're building clr.nativeaotlibs and not building the CLR runtime, compile libraries against NativeAOT CoreLib -->
<UseNativeAotCoreLib Condition="$(_subset.Contains('+clr.nativeaotlibs+')) and !$(_subset.Contains('+clr.native+')) and !$(_subset.Contains('+clr.runtime+'))">true</UseNativeAotCoreLib>
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ add_subdirectory(tools/aot/jitinterface)

if(NOT CLR_CROSS_COMPONENTS_BUILD)
# NativeAOT only buildable for a subset of CoreCLR-supported configurations
if((CLR_CMAKE_HOST_LINUX OR CLR_CMAKE_HOST_OSX OR CLR_CMAKE_HOST_WIN32) AND (CLR_CMAKE_HOST_ARCH_ARM64 OR CLR_CMAKE_HOST_ARCH_AMD64) AND NOT (CLR_CMAKE_HOST_OSX AND CLR_CMAKE_HOST_ARCH_ARM64))
if((CLR_CMAKE_HOST_LINUX OR CLR_CMAKE_HOST_OSX OR CLR_CMAKE_HOST_WIN32) AND (CLR_CMAKE_HOST_ARCH_ARM64 OR CLR_CMAKE_HOST_ARCH_AMD64))
add_subdirectory(nativeaot)
endif()
endif(NOT CLR_CROSS_COMPONENTS_BUILD)
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/gc/unix/gcenv.unix.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -633,7 +633,7 @@ void GCToOSInterface::YieldThread(uint32_t switchCount)
static void* VirtualReserveInner(size_t size, size_t alignment, uint32_t flags, uint32_t hugePagesFlag = 0)
{
assert(!(flags & VirtualReserveFlags::WriteWatch) && "WriteWatch not supported on Unix");
if (alignment == 0)
if (alignment < OS_PAGE_SIZE)
Copy link
Member

Choose a reason for hiding this comment

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

Wrap under #if defined(HOST_ARM64) && defined(TARGET_OSX) && defined(FEATURE_NATIVEAOT) to limit the scope of this change?

Copy link
Member Author

Choose a reason for hiding this comment

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

I would still like to know why CoreCLR doesn't hit this but I assume that it's because of some GC feature switch (such as FEATURE_MANUALLY_MANAGED_CARD_BUNDLES).

While I can add the #if guards the code would still misbehave fatally when alignment < OS_PAGE_SIZE so I think the change is quite safe.

Copy link
Member

Choose a reason for hiding this comment

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

I would still like to know why CoreCLR doesn't hit this

AFAIK gcenv.unix.cpp is only used by CoreCLR when standalone GC is used (i.e. COMPlus_GCName is set to point to a standalone GC instead of the one in coreclr.dll/.so). @mangod9 @dotnet/gc can you have a look at this change?

Copy link
Member

@am11 am11 Sep 9, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Yes, the regular CI pipelines do not use this code. We should trigger gc-standalone pipeline https://dev.azure.com/dnceng/public/_build?definitionId=1017 against this PR to get realistic results (windows arm64 checked failure in that pipeline is preexisting, the rest should be green). AFAIK, there is no azp run trigger for that pipeline, someone with access will need to trigger it manually.

Copy link
Member

@MichalStrehovsky MichalStrehovsky Sep 9, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

AFAIK, there is no azp run trigger for that pipeline, someone with access will need to trigger it manually.

I tried to trigger it but it's complaining about the pool. It doesn't look like the gc-standalone pipeline ran for a week or so. It coincides with the pool changes. Cc @dotnet/gc.

The interesting thing about the gc-standalone pipeline is that it doesn't test mac at all. So if there was a arm64 macOS issue with standalone GC, we wouldn't even know it.

Copy link
Member

Choose a reason for hiding this comment

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

if there was a arm64 macOS issue with standalone GC, we wouldn't even know it.

This change as written is not specific to arm64 macOS, other platforms can break. My original commit was am11@45f3a48, but it was recreated without the guard which has potential to break existing code.

Copy link
Member Author

Choose a reason for hiding this comment

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

The specific call with the "invalid" alignment parameter is macOS arm64 specific though. It happens because of 16Kb page sizes and GC_PAGE_SIZE != OS_PAGE_SIZE.

{
alignment = OS_PAGE_SIZE;
}
Expand Down
3 changes: 1 addition & 2 deletions src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.Publish.targets
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,7 @@
<Error Condition="'$(DisableUnsupportedError)' != 'true' and '$(OS)' != 'Windows_NT' and $(RuntimeIdentifier.StartsWith('win'))"
Text="Cross-OS native compilation is not supported." />

<Error Condition="'$(DisableUnsupportedError)' != 'true' and !($(RuntimeIdentifier.EndsWith('x64')) or
($(RuntimeIdentifier.EndsWith('arm64')) and !$(RuntimeIdentifier.StartsWith('osx'))))"
<Error Condition="'$(DisableUnsupportedError)' != 'true' and !($(RuntimeIdentifier.EndsWith('x64')) or $(RuntimeIdentifier.EndsWith('arm64')))"
Text="Native compilation does not support targeting $(RuntimeIdentifier) yet." />

<Error Condition="'$(DisableUnsupportedError)' != 'true' and !('$(IlcHostArch)' == 'x64' or '$(IlcHostArch)' == 'arm64')"
Expand Down
7 changes: 6 additions & 1 deletion src/coreclr/nativeaot/Runtime.Base/src/System/Runtime/ThunkPool.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,14 @@ namespace System.Runtime
{
internal static class Constants
{
#if TARGET_ARM64 && TARGET_OSX
public const uint PageSize = 0x4000; // 16k
public const nuint PageSizeMask = 0x3FFF;
#else
public const uint PageSize = 0x1000; // 4k
public const uint AllocationGranularity = 0x10000; // 64k
public const nuint PageSizeMask = 0xFFF;
#endif
public const uint AllocationGranularity = 0x10000; // 64k
public const nuint AllocationGranularityMask = 0xFFFF;

public static readonly int ThunkDataSize = 2 * IntPtr.Size;
Expand Down
4 changes: 4 additions & 0 deletions src/coreclr/nativeaot/Runtime/CommonMacros.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,8 +155,12 @@ inline bool IS_ALIGNED(T* val, uintptr_t alignment);

#define DATA_ALIGNMENT 8
#ifndef OS_PAGE_SIZE
#ifdef HOST_OSX
#define OS_PAGE_SIZE 0x4000
#else
#define OS_PAGE_SIZE 0x1000
#endif
#endif

#elif defined(HOST_WASM)

Expand Down
7 changes: 7 additions & 0 deletions src/coreclr/nativeaot/Runtime/ThunksMapping.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,9 @@ EXTERN_C NATIVEAOT_API void* __cdecl RhAllocateThunksMapping()
return NULL;
}
jkotas marked this conversation as resolved.
Show resolved Hide resolved

#if defined(HOST_OSX) && defined(HOST_ARM64)
pthread_jit_write_protect_np(0);
#endif
#endif

int numBlocksPerMap = RhpGetNumThunkBlocksPerMapping();
Expand Down Expand Up @@ -223,11 +226,15 @@ EXTERN_C NATIVEAOT_API void* __cdecl RhAllocateThunksMapping()
}
}

#if defined(HOST_OSX) && defined(HOST_ARM64)
pthread_jit_write_protect_np(1);
#else
if (!PalVirtualProtect(pThunksSection, THUNKS_MAP_SIZE, PAGE_EXECUTE_READ))
{
PalVirtualFree(pNewMapping, 0, MEM_RELEASE);
return NULL;
}
#endif

PalFlushInstructionCache(pThunksSection, THUNKS_MAP_SIZE);

Expand Down
11 changes: 9 additions & 2 deletions src/coreclr/nativeaot/Runtime/unix/PalRedhawkUnix.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -776,8 +776,15 @@ REDHAWK_PALEXPORT _Ret_maybenull_ _Post_writable_byte_size_(size) void* REDHAWK_
static const size_t Alignment = 64 * 1024;

size_t alignedSize = size + (Alignment - OS_PAGE_SIZE);
int flags = MAP_ANON | MAP_PRIVATE;

void * pRetVal = mmap(pAddress, alignedSize, unixProtect, MAP_ANON | MAP_PRIVATE, -1, 0);
#if defined(HOST_OSX) && defined(HOST_ARM64)
if (unixProtect & PROT_EXEC) {
jkotas marked this conversation as resolved.
Show resolved Hide resolved
flags |= MAP_JIT;
}
#endif

void * pRetVal = mmap(pAddress, alignedSize, unixProtect, flags, -1, 0);

if (pRetVal != NULL)
{
Expand Down Expand Up @@ -1057,7 +1064,7 @@ REDHAWK_PALEXPORT void REDHAWK_PALAPI PalHijack(HANDLE hThread, _In_opt_ void* p
}
#endif

if ((status != 0) && (status != EAGAIN))
if ((status != 0) && (status != EAGAIN) && (status != ESRCH))
filipnavara marked this conversation as resolved.
Show resolved Hide resolved
{
// Failure to send the signal is fatal. There are only two cases when sending
// the signal can fail. First, if the signal ID is invalid and second,
Expand Down
36 changes: 35 additions & 1 deletion src/coreclr/nativeaot/Runtime/unix/UnwindHelpers.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,12 @@

#if defined(TARGET_AMD64)
using libunwind::Registers_x86_64;
using libunwind::CompactUnwinder_x86_64;
#elif defined(TARGET_ARM)
using libunwind::Registers_arm;
#elif defined(TARGET_ARM64)
using libunwind::Registers_arm64;
using libunwind::CompactUnwinder_arm64;
#elif defined(TARGET_X86)
using libunwind::Registers_x86;
#else
Expand Down Expand Up @@ -501,6 +503,8 @@ struct Registers_REGDISPLAY : REGDISPLAY
uint64_t getIP() const { return IP;}
void setIP(uint64_t value, uint64_t location)
{ IP = value; pIP = (PTR_UIntNative)location; }
uint64_t getFP() const { return *pFP;}
void setFP(uint64_t value, uint64_t location) { pFP = (PTR_UIntNative)location;}
};

inline bool Registers_REGDISPLAY::validRegister(int num) const {
Expand Down Expand Up @@ -772,7 +776,37 @@ bool DoTheStep(uintptr_t pc, UnwindInfoSections uwInfoSections, REGDISPLAY *regs
#endif

#if _LIBUNWIND_SUPPORT_DWARF_UNWIND
bool retVal = uc.getInfoFromDwarfSection(pc, uwInfoSections, 0 /* fdeSectionOffsetHint */);
uint32_t dwarfOffsetHint = 0;

#if _LIBUNWIND_SUPPORT_COMPACT_UNWIND
// If there is a compact unwind encoding table, look there first.
if (uwInfoSections.compact_unwind_section != 0 && uc.getInfoFromCompactEncodingSection(pc, uwInfoSections)) {
unw_proc_info_t procInfo;
uc.getInfo(&procInfo);

#if defined(TARGET_ARM64)
if ((procInfo.format & UNWIND_ARM64_MODE_MASK) != UNWIND_ARM64_MODE_DWARF) {
CompactUnwinder_arm64<LocalAddressSpace, Registers_REGDISPLAY> compactInst;
int stepRet = compactInst.stepWithCompactEncoding(procInfo.format, pc, _addressSpace, *(Registers_REGDISPLAY*)regs);
return stepRet == UNW_STEP_SUCCESS;
} else {
dwarfOffsetHint = procInfo.format & UNWIND_ARM64_DWARF_SECTION_OFFSET;
}
#elif defined(TARGET_AMD64)
if ((procInfo.format & UNWIND_X86_64_MODE_MASK) != UNWIND_X86_64_MODE_DWARF) {
CompactUnwinder_x86_64<LocalAddressSpace, Registers_REGDISPLAY> compactInst;
int stepRet = compactInst.stepWithCompactEncoding(procInfo.format, pc, _addressSpace, *(Registers_REGDISPLAY*)regs);
return stepRet == UNW_STEP_SUCCESS;
} else {
dwarfOffsetHint = procInfo.format & UNWIND_X86_64_DWARF_SECTION_OFFSET;
}
#else
PORTABILITY_ASSERT("DoTheStep");
#endif
}
#endif

bool retVal = uc.getInfoFromDwarfSection(pc, uwInfoSections, dwarfOffsetHint);
if (!retVal)
{
return false;
Expand Down
11 changes: 10 additions & 1 deletion src/coreclr/nativeaot/Runtime/unix/unixasmmacrosarm64.inc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ C_FUNC(\Name):
#if defined(__APPLE__)
adrp \HelperReg, C_FUNC(\Name)@GOTPAGE
ldr \HelperReg, [\HelperReg, C_FUNC(\Name)@GOTPAGEOFF]
ldr \HelperReg, [\HelperReg]
filipnavara marked this conversation as resolved.
Show resolved Hide resolved
#else
adrp \HelperReg, C_FUNC(\Name)
ldr \HelperReg, [\HelperReg, :lo12:C_FUNC(\Name)]
Expand All @@ -82,7 +83,8 @@ C_FUNC(\Name):
.macro PREPARE_EXTERNAL_VAR_INDIRECT_W Name, HelperReg
#if defined(__APPLE__)
adrp x\HelperReg, C_FUNC(\Name)@GOTPAGE
ldr w\HelperReg, [x\HelperReg, C_FUNC(\Name)@GOTPAGEOFF]
ldr x\HelperReg, [x\HelperReg, C_FUNC(\Name)@GOTPAGEOFF]
ldr w\HelperReg, [x\HelperReg]
#else
adrp x\HelperReg, C_FUNC(\Name)
ldr w\HelperReg, [x\HelperReg, :lo12:C_FUNC(\Name)]
Expand Down Expand Up @@ -182,6 +184,10 @@ C_FUNC(\Name):
.endif

stp x0, lr, [sp,#-0x10]!
#if defined(__APPLE__)
// Apple's tls_get_var trashes xip0 (and possibly xip1)
stp xip0, xip1, [sp,#-0x10]!
#endif

// This sequence of instructions is recognized and potentially patched
// by the linker (GD->IE/LE relaxation).
Expand All @@ -206,6 +212,9 @@ C_FUNC(\Name):
add \target, \target, x0
#endif

#if defined(__APPLE__)
ldp xip0, xip1, [sp],#0x10
#endif
ldp x0, lr, [sp],#0x10
.endm

Expand Down
5 changes: 3 additions & 2 deletions ...reclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/FatFunctionPointerNode.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,9 @@ public override ObjectData GetData(NodeFactory factory, bool relocsOnly = false)
{
var builder = new ObjectDataBuilder(factory, relocsOnly);

// These need to be aligned the same as methods because they show up in same contexts
builder.RequireInitialAlignment(factory.Target.MinimumFunctionAlignment);
// These need to be aligned the same as method pointers because they show up in same contexts
// (macOS ARM64 has even stricter alignment requirement for the linker, so round up to pointer size)
builder.RequireInitialAlignment(factory.Target.PointerSize);
Copy link
Member Author

Choose a reason for hiding this comment

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

This one is actually hell to diagnose because it produces no error but causes corruption when linking. If absolute relocation of a symbols is 4-byte aligned and spans between two pages in the executable then only the lower part of the relocation is written correctly into the final executable.

This was the root cause of the intermittent problem I've hit earlier.


builder.AddSymbol(this);

Expand Down
1 change: 1 addition & 0 deletions ...lr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/ModuleInitializerListNode.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,7 @@ public override ObjectData GetData(NodeFactory factory, bool relocsOnly = false)
// the time of startup. (Linker likely can't do it, unfortunately.)

ObjectDataBuilder builder = new ObjectDataBuilder(factory, relocsOnly);
builder.RequireInitialAlignment(factory.Target.PointerSize);
builder.AddSymbol(this);
builder.AddSymbol(_endSymbol);

Expand Down
2 changes: 2 additions & 0 deletions src/native/external/llvm-libunwind-version.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,5 @@ https://github.com/llvm/llvm-project/releases/tag/llvmorg-14.0.6

Apply https://github.com/dotnet/runtime/commit/e57194552327bccaf2b45a9190f0e6e1289472e4
Apply https://github.com/dotnet/runtime/commit/db7ed089a9e20a391a53e0b547f95dc8bf916765
Apply https://github.com/dotnet/runtime/commit/828c8d0b2a8cdc2479524c5229939641d3610a1b
jkotas marked this conversation as resolved.
Show resolved Hide resolved
Apply https://github.com/dotnet/runtime/commit/92e7dffffbcd9725616741fbd73a7e7e1f5e97a4
Loading