From 9a2234461a1bf9994f7dd30ae3d828510cf3d23f Mon Sep 17 00:00:00 2001 From: Tess Strickland Date: Fri, 23 May 2025 06:19:16 -0700 Subject: [PATCH] [vm] Use a non-native definition of the runtime function structure. Attempting to use the native definition when available causes problems when cross-compiling, and the original attempts to avoid this broke in subtle ways. Instead, just make a non-native definition that mimics the native definition and use that with reinterpret_casts when sending uses of the non-native definition to Windows API calls that expect a value of the native definition. TEST=windows ci Fixes: https://github.com/dart-lang/sdk/issues/60771 Change-Id: I347fcaf8cccd809a3d8e6f041cc7e360b0b8226e Cq-Include-Trybots: luci.dart.try:vm-aot-win-release-arm64-try,vm-aot-win-release-x64-try,vm-win-release-arm64-try,vm-win-release-x64-try,vm-aot-win-debug-arm64-try,vm-aot-win-debug-x64-try,pkg-win-release-try,pkg-win-release-arm64-try,dart-sdk-win-try,dart-sdk-win-arm64-try Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/430400 Reviewed-by: Slava Egorov --- runtime/platform/unwinding_records.cc | 12 ++-- runtime/platform/unwinding_records.h | 75 +++++++++++++---------- runtime/platform/unwinding_records_win.cc | 13 ++-- runtime/vm/unwinding_records.cc | 15 +++-- runtime/vm/unwinding_records.h | 1 + runtime/vm/unwinding_records_win.cc | 39 ++++-------- 6 files changed, 75 insertions(+), 80 deletions(-) diff --git a/runtime/platform/unwinding_records.cc b/runtime/platform/unwinding_records.cc index 835464e5b2b..3aae279f6fc 100644 --- a/runtime/platform/unwinding_records.cc +++ b/runtime/platform/unwinding_records.cc @@ -3,30 +3,30 @@ // BSD-style license that can be found in the LICENSE file. #include "platform/unwinding_records.h" + #include "platform/globals.h" namespace dart { -#if !(defined(DART_TARGET_OS_WINDOWS) && defined(TARGET_ARCH_IS_64_BIT) || \ - defined(DART_HOST_OS_WINDOWS) && defined(ARCH_IS_64_BIT)) +#if !defined(NEED_WINDOWS_UNWINDING_RECORDS) intptr_t UnwindingRecordsPlatform::SizeInBytes() { return 0; } -#endif // !defined(DART_TARGET_OS_WINDOWS) && !defined(DART_HOST_OS_WINDOWS) +#endif // !defined(NEED_WINDOWS_UNWINDING_RECORDS) // Also use empty definitions when running gen_snapshot on 64-bit Windows, as it // does not use the ELF loader, which is the client of these methods. -#if !defined(DART_HOST_OS_WINDOWS) || !defined(ARCH_IS_64_BIT) || \ - (defined(DART_PRECOMPILER) && !defined(TESTING)) +#if !defined(UNWINDING_RECORDS_WINDOWS_HOST) void UnwindingRecordsPlatform::RegisterExecutableMemory( void* start, intptr_t size, void** pp_dynamic_table) {} + void UnwindingRecordsPlatform::UnregisterDynamicTable(void* p_dynamic_table) {} -#endif // !defined(DART_HOST_OS_WINDOWS) || ... +#endif // !defined(UNWINDING_RECORDS_WINDOWS_HOST) } // namespace dart diff --git a/runtime/platform/unwinding_records.h b/runtime/platform/unwinding_records.h index 0da447b1919..7895b9ecf75 100644 --- a/runtime/platform/unwinding_records.h +++ b/runtime/platform/unwinding_records.h @@ -20,16 +20,20 @@ class UnwindingRecordsPlatform : public AllStatic { static void UnregisterDynamicTable(void* p_dynamic_table); }; -// These definitions are only needed if targeting 64-bit Windows, or if the -// ELF loader may be used on 64-bit Windows. -// -// More specifically, a 64-bit Windows gen_snapshot that does not target -// 64-bit Windows does not need these definitions, as the generated snapshot -// should not contain Windows-specific unwinding records and gen_snapshot -// does not uses the ELF loader. -#if (defined(DART_TARGET_OS_WINDOWS) && defined(TARGET_ARCH_IS_64_BIT)) || \ - (defined(DART_HOST_OS_WINDOWS) && defined(ARCH_IS_64_BIT) && \ - (!defined(DART_PRECOMPILER) || defined(TESTING))) +#if defined(DART_HOST_OS_WINDOWS) && defined(ARCH_IS_64_BIT) && \ + (!defined(DART_PRECOMPILER) || defined(TESTING)) +#define NEED_WINDOWS_UNWINDING_RECORDS 1 +// Guard for code and definitions that are used when the 64-bit Windows runtime +// may make calls to the Windows APIs using the unwinding records information. +#define UNWINDING_RECORDS_WINDOWS_HOST 1 +#elif defined(DART_TARGET_OS_WINDOWS) && defined(TARGET_ARCH_IS_64_BIT) +#define NEED_WINDOWS_UNWINDING_RECORDS 1 +// Guard for code and definitions that are used when precompiling for +// a 64-bit Windows target without any runtime use (and thus, no API calls). +#define UNWINDING_RECORDS_WINDOWS_PRECOMPILER 1 +#endif + +#if defined(NEED_WINDOWS_UNWINDING_RECORDS) #pragma pack(push, 1) @@ -63,14 +67,6 @@ typedef struct _UNWIND_INFO { UNWIND_CODE UnwindCode[2]; } UNWIND_INFO, *PUNWIND_INFO; -#if !defined(DART_HOST_OS_WINDOWS) -typedef struct _RUNTIME_FUNCTION { - ULONG BeginAddress; - ULONG EndAddress; - ULONG UnwindData; -} RUNTIME_FUNCTION, *PRUNTIME_FUNCTION; -#endif - static constexpr int kPushRbpInstructionLength = 1; static const int kMovRbpRspInstructionLength = 3; static constexpr int kRbpPrefixLength = @@ -102,13 +98,20 @@ struct GeneratedCodeUnwindInfo { static constexpr uint32_t kUnwindingRecordMagic = 0xAABBCCDD; +struct TargetRuntimeFunction { + ULONG BeginAddress; + ULONG EndAddress; + ULONG UnwindData; +}; + struct CodeRangeUnwindingRecord { void* dynamic_table; uint32_t magic; uint32_t runtime_function_count; GeneratedCodeUnwindInfo unwind_info; intptr_t exception_handler; - RUNTIME_FUNCTION runtime_function[1]; + // Must be cast to a PRUNTIME_FUNCTION when passed to Windows APIs. + TargetRuntimeFunction runtime_function[1]; }; #elif defined(TARGET_ARCH_ARM64) @@ -148,13 +151,6 @@ struct UNWIND_INFO { uint32_t CodeWords : 5; }; -#if !defined(DART_HOST_OS_WINDOWS) || !defined(HOST_ARCH_ARM64) -typedef struct _RUNTIME_FUNCTION { - ULONG BeginAddress; - ULONG UnwindData; -} RUNTIME_FUNCTION, *PRUNTIME_FUNCTION; -#endif - /** * Base on below doc, unwind record has 18 bits (unsigned) to encode function * length, besides 2 LSB which are always 0. @@ -218,6 +214,11 @@ struct UnwindData { static const uint32_t kDefaultRuntimeFunctionCount = 1; static constexpr uint32_t kUnwindingRecordMagic = 0xAABBCCEE; +struct TargetRuntimeFunction { + ULONG BeginAddress; + ULONG UnwindData; +}; + struct CodeRangeUnwindingRecord { void* dynamic_table; uint32_t magic; @@ -231,19 +232,29 @@ struct CodeRangeUnwindingRecord { // than full size. UnwindData<> unwind_info1; - // More RUNTIME_FUNCTION structs could follow below array because the number - // of RUNTIME_FUNCTION needed to cover given code range is computed at - // runtime. - RUNTIME_FUNCTION runtime_function[kDefaultRuntimeFunctionCount]; + // An arbitrary number of runtime function structs follow the initial header + // as the number needed to cover the given code range is computed at runtime. + // Must be cast to a PRUNTIME_FUNCTION when passed to Windows APIs. + TargetRuntimeFunction runtime_function[kDefaultRuntimeFunctionCount]; }; - #else #error Unhandled Windows architecture. #endif +// Since the definition of the RUNTIME_FUNCTION struct differs on X64 +// and ARM64 Windows and the precompiler may be cross compiling between +// the two, TargetRuntimeFunction is defined above, which mimics the +// native RUNTIME_FUNCTION struct of the target. +// +// Make sure that the sizes match, so that TargetRuntimeFunction values +// can be used as RUNTIME_FUNCTION values and vice versa in the runtime. +#if defined(UNWINDING_RECORDS_WINDOWS_HOST) +static_assert(sizeof(TargetRuntimeFunction) == sizeof(RUNTIME_FUNCTION)); +#endif + #pragma pack(pop) -#endif // (defined(DART_TARGET_OS_WINDOWS) || ... +#endif // defined(NEED_WINDOWS_UNWINDING_RECORDS) } // namespace dart diff --git a/runtime/platform/unwinding_records_win.cc b/runtime/platform/unwinding_records_win.cc index 6f9c6dc4442..3ddde0993a5 100644 --- a/runtime/platform/unwinding_records_win.cc +++ b/runtime/platform/unwinding_records_win.cc @@ -9,8 +9,7 @@ namespace dart { -#if (defined(DART_TARGET_OS_WINDOWS) && defined(TARGET_ARCH_IS_64_BIT)) || \ - (defined(DART_HOST_OS_WINDOWS) && defined(ARCH_IS_64_BIT)) +#if defined(NEED_WINDOWS_UNWINDING_RECORDS) #if defined(TARGET_ARCH_X64) const intptr_t kReservedUnwindingRecordsSizeBytes = 64; @@ -24,12 +23,11 @@ intptr_t UnwindingRecordsPlatform::SizeInBytes() { return kReservedUnwindingRecordsSizeBytes; } -#endif // defined(DART_TARGET_OS_WINDOWS) ... +#endif // defined(NEED_WINDOWS_UNWINDING_RECORDS) // Only use these definitions when the ELF loader may be used on 64-bit Windows, // as it is the only client of these methods (e.g., _not_ in gen_snapshot). -#if defined(DART_HOST_OS_WINDOWS) && defined(ARCH_IS_64_BIT) && \ - (!defined(DART_PRECOMPILER) || defined(TESTING)) +#if defined(UNWINDING_RECORDS_WINDOWS_HOST) void UnwindingRecordsPlatform::RegisterExecutableMemory( void* start, @@ -44,7 +42,8 @@ void UnwindingRecordsPlatform::RegisterExecutableMemory( uword end_num = start_num + size; DWORD status = RtlAddGrowableFunctionTable( pp_dynamic_table, - /*FunctionTable=*/record->runtime_function, + /*FunctionTable=*/ + reinterpret_cast(record->runtime_function), /*EntryCount=*/record->runtime_function_count, /*MaximumEntryCount=*/record->runtime_function_count, /*RangeBase=*/start_num, @@ -58,6 +57,6 @@ void UnwindingRecordsPlatform::UnregisterDynamicTable(void* p_dynamic_table) { RtlDeleteGrowableFunctionTable(p_dynamic_table); } -#endif // defined(DART_HOST_OS_WINDOWS) ... +#endif // defined(UNWINDING_RECORDS_WINDOWS_HOST) } // namespace dart diff --git a/runtime/vm/unwinding_records.cc b/runtime/vm/unwinding_records.cc index e442118ab7e..a284299ed9a 100644 --- a/runtime/vm/unwinding_records.cc +++ b/runtime/vm/unwinding_records.cc @@ -3,27 +3,26 @@ // BSD-style license that can be found in the LICENSE file. #include "vm/unwinding_records.h" -#include "vm/globals.h" namespace dart { -#if !defined(DART_TARGET_OS_WINDOWS) || !defined(TARGET_ARCH_IS_64_BIT) +// The default definition when not precompiling for a Windows target. +#if !defined(UNWINDING_RECORDS_WINDOWS_PRECOMPILER) const void* UnwindingRecords::GenerateRecordsInto(intptr_t offset, uint8_t* target_buffer) { return nullptr; } -#endif // !defined(DART_TARGET_OS_WINDOWS) +#endif // !defined(UNWINDING_RECORDS_WINDOWS_PRECOMPILER) -// Also use empty definitions when running gen_snapshot on 64-bit Windows, as -// it does not use the ELF loader, which is the client of these methods. -#if !defined(DART_HOST_OS_WINDOWS) || !defined(ARCH_IS_64_BIT) || \ - (defined(DART_PRECOMPILER) && !defined(TESTING)) +// The default definition when the ElfLoader is not used or the VM is not +// running on a Windows system. +#if !defined(UNWINDING_RECORDS_WINDOWS_HOST) void UnwindingRecords::RegisterExecutablePage(Page* page) {} void UnwindingRecords::UnregisterExecutablePage(Page* page) {} -#endif // !defined(DART_HOST_OS_WINDOWS) +#endif // !defined(UNWINDING_RECORDS_WINDOWS_HOST) } // namespace dart diff --git a/runtime/vm/unwinding_records.h b/runtime/vm/unwinding_records.h index 0a23a545c2e..b5348fdfafd 100644 --- a/runtime/vm/unwinding_records.h +++ b/runtime/vm/unwinding_records.h @@ -6,6 +6,7 @@ #ifndef RUNTIME_VM_UNWINDING_RECORDS_H_ #define RUNTIME_VM_UNWINDING_RECORDS_H_ +#include "platform/unwinding_records.h" #include "vm/allocation.h" #include "vm/heap/page.h" diff --git a/runtime/vm/unwinding_records_win.cc b/runtime/vm/unwinding_records_win.cc index 01e5410b9bd..2ab9c4d27e1 100644 --- a/runtime/vm/unwinding_records_win.cc +++ b/runtime/vm/unwinding_records_win.cc @@ -4,18 +4,11 @@ #include "vm/unwinding_records.h" -#include "vm/globals.h" - -#include "platform/unwinding_records.h" +#include "platform/globals.h" namespace dart { -// When the host is 64-bit Windows but the target is not, only define this -// when the ELF loader may be used, so _not_ in gen_snapshot. -#if (defined(DART_TARGET_OS_WINDOWS) && defined(TARGET_ARCH_IS_64_BIT)) || \ - (defined(DART_HOST_OS_WINDOWS) && defined(ARCH_IS_64_BIT) && \ - (!defined(DART_PRECOMPILER) || defined(TESTING))) - +#if defined(NEED_WINDOWS_UNWINDING_RECORDS) static void InitUnwindingRecord(intptr_t offset, CodeRangeUnwindingRecord* record, size_t code_size_in_bytes) { @@ -34,13 +27,13 @@ static void InitUnwindingRecord(intptr_t offset, // committed and reserved to contain multiple PDATA/XDATA to cover the whole // range. All addresses are 32bit relative offsets to start. - // Maximum RUNTIME_FUNCTION count available in reserved memory, this includes + // Maximum runtime function count available in reserved memory, this includes // static part in Record as kDefaultRuntimeFunctionCount plus dynamic part in // the remaining reserved memory. const uint32_t max_runtime_function_count = static_cast((UnwindingRecordsPlatform::SizeInBytes() - sizeof(CodeRangeUnwindingRecord)) / - sizeof(RUNTIME_FUNCTION) + + sizeof(TargetRuntimeFunction) + kDefaultRuntimeFunctionCount); uint32_t runtime_function_index = 0; @@ -48,7 +41,7 @@ static void InitUnwindingRecord(intptr_t offset, int64_t remaining_size_in_bytes = static_cast(code_size_in_bytes); // Divide the code range into chunks in size kMaxFunctionLength and create a - // RUNTIME_FUNCTION for each of them. All the chunks in the same size can + // runtime function for each of them. All the chunks in the same size can // share 1 unwind_info struct, but a separate unwind_info is needed for the // last chunk if it is smaller than kMaxFunctionLength, because unlike X64, // unwind_info encodes the function/chunk length. @@ -95,11 +88,9 @@ static void InitUnwindingRecord(intptr_t offset, #endif record->magic = kUnwindingRecordMagic; } +#endif // defined(NEED_WINDOWS_UNWINDING_RECORDS) -#endif // defined(DART_TARGET_OS_WINDOWS) || ... - -#if defined(DART_TARGET_OS_WINDOWS) && defined(TARGET_ARCH_IS_64_BIT) - +#if defined(UNWINDING_RECORDS_WINDOWS_PRECOMPILER) const void* UnwindingRecords::GenerateRecordsInto(intptr_t offset, uint8_t* target_buffer) { CodeRangeUnwindingRecord* record = @@ -107,15 +98,9 @@ const void* UnwindingRecords::GenerateRecordsInto(intptr_t offset, InitUnwindingRecord(offset, record, offset); return target_buffer; } +#endif // defined(UNWINDING_RECORDS_WINDOWS_PRECOMPILER) -#endif // defined(DART_TARGET_OS_WINDOWS) && defined(TARGET_ARCH_IS_64_BIT) - -// Only use these definitions when the ELF loader may be used on 64-bit -// Windows, as it is the only client of these methods (e.g., _not_ in -// gen_snapshot). -#if defined(DART_HOST_OS_WINDOWS) && defined(ARCH_IS_64_BIT) && \ - (!defined(DART_PRECOMPILER) || defined(TESTING)) - +#if defined(UNWINDING_RECORDS_WINDOWS_HOST) // Special exception-unwinding records are put at the end of executable // page on Windows for 64-bit applications. void UnwindingRecords::RegisterExecutablePage(Page* page) { @@ -132,7 +117,8 @@ void UnwindingRecords::RegisterExecutablePage(Page* page) { RELEASE_ASSERT(record->magic == kUnwindingRecordMagic); DWORD status = RtlAddGrowableFunctionTable( /*DynamicTable=*/&record->dynamic_table, - /*FunctionTable=*/record->runtime_function, + /*FunctionTable=*/ + reinterpret_cast(record->runtime_function), /*EntryCount=*/record->runtime_function_count, /*MaximumEntryCount=*/record->runtime_function_count, /*RangeBase=*/page->memory_->start(), @@ -153,7 +139,6 @@ void UnwindingRecords::UnregisterExecutablePage(Page* page) { RELEASE_ASSERT(record->magic == kUnwindingRecordMagic); RtlDeleteGrowableFunctionTable(record->dynamic_table); } - -#endif // defined(DART_HOST_OS_WINDOWS) ... +#endif // defined(UNWINDING_RECORDS_WINDOWS_HOST) } // namespace dart