From de76808d5ae3f5bd45dec7d946abecf8f87859d8 Mon Sep 17 00:00:00 2001 From: James Moschou Date: Sun, 23 Aug 2026 13:24:30 +0200 Subject: [PATCH 1/5] Fix comparison of heap objects --- Sources/ComputeCxx/Comparison/Builder.h | 2 +- Sources/ComputeCxx/Comparison/Compare.cpp | 18 ++++++++---- .../Comparison/LayoutDescriptor.cpp | 29 ++++++++++--------- .../ComputeCxx/Comparison/LayoutDescriptor.h | 3 +- Sources/ComputeCxx/Comparison/ValueLayout.h | 2 +- 5 files changed, 32 insertions(+), 22 deletions(-) diff --git a/Sources/ComputeCxx/Comparison/Builder.h b/Sources/ComputeCxx/Comparison/Builder.h index e9e3c55..7829078 100644 --- a/Sources/ComputeCxx/Comparison/Builder.h +++ b/Sources/ComputeCxx/Comparison/Builder.h @@ -31,7 +31,7 @@ class Builder : public swift::metadata_visitor { const swift::existential_type_metadata *type; }; struct HeapRefItem : RangeItem { - bool is_function; + bool is_capture_ref; }; struct NestedItem : RangeItem { ValueLayout layout; diff --git a/Sources/ComputeCxx/Comparison/Compare.cpp b/Sources/ComputeCxx/Comparison/Compare.cpp index 5027d06..84d20cc 100644 --- a/Sources/ComputeCxx/Comparison/Compare.cpp +++ b/Sources/ComputeCxx/Comparison/Compare.cpp @@ -1,5 +1,7 @@ #include "Compare.h" +#include + #include "Graph/Graph.h" #include "Swift/Metadata.h" #include "Swift/SwiftShims.h" @@ -158,14 +160,20 @@ bool Compare::operator()(ValueLayout layout, const unsigned char *lhs, const uns continue; } case ValueLayoutEntryKind::HeapRef: - case ValueLayoutEntryKind::Function: { - bool is_function = kind == ValueLayoutEntryKind::Function; + case ValueLayoutEntryKind::CaptureRef: { + bool is_capture_ref = kind == ValueLayoutEntryKind::CaptureRef; size_t item_end = offset + 8; - if (lhs + offset != rhs + offset) { - if (!compare_heap_objects(lhs + offset, rhs + offset, options & ~IAGComparisonOptionsTraceCompareFailed, - is_function)) { + // The buffer at offset stores a pointer to the heap object. + const void *lhs_object; + const void *rhs_object; + std::memcpy(&lhs_object, lhs + offset, sizeof(lhs_object)); + std::memcpy(&rhs_object, rhs + offset, sizeof(rhs_object)); + + if (lhs_object != rhs_object) { + auto heap_object_options = options & ~IAGComparisonOptionsTraceCompareFailed; + if (!compare_heap_objects(lhs_object, rhs_object, heap_object_options, is_capture_ref)) { failed(options, lhs, rhs, offset, 8, nullptr); return false; } diff --git a/Sources/ComputeCxx/Comparison/LayoutDescriptor.cpp b/Sources/ComputeCxx/Comparison/LayoutDescriptor.cpp index 3353d96..bc553c8 100644 --- a/Sources/ComputeCxx/Comparison/LayoutDescriptor.cpp +++ b/Sources/ComputeCxx/Comparison/LayoutDescriptor.cpp @@ -365,7 +365,7 @@ size_t length(ValueLayout layout) { reader.skip(sizeof(void *)); continue; case ValueLayoutEntryKind::HeapRef: - case ValueLayoutEntryKind::Function: + case ValueLayoutEntryKind::CaptureRef: continue; case ValueLayoutEntryKind::Nested: reader.skip(sizeof(void *)); @@ -482,8 +482,7 @@ bool compare_bytes(const unsigned char *lhs, const unsigned char *rhs, size_t si return true; } -bool compare_heap_objects(const unsigned char *lhs, const unsigned char *rhs, IAGComparisonOptions options, - bool is_function) { +bool compare_heap_objects(const void *lhs, const void *rhs, IAGComparisonOptions options, bool is_capture_ref) { if (lhs == rhs) { return true; } @@ -491,19 +490,23 @@ bool compare_heap_objects(const unsigned char *lhs, const unsigned char *rhs, IA return false; } - auto lhs_type = (const swift::metadata *)lhs; - auto rhs_type = (const swift::metadata *)rhs; + // The first field of a Swift HeapObject is a pointer to its metadata. + const swift::metadata *lhs_type; + const swift::metadata *rhs_type; + std::memcpy(&lhs_type, lhs, sizeof(lhs_type)); + std::memcpy(&rhs_type, rhs, sizeof(rhs_type)); if (lhs_type != rhs_type) { return false; } - HeapMode heap_mode = is_function ? HeapMode::Locals : HeapMode::Class; - IAGComparisonOptions fetch_options = options & IAGComparisonOptionsComparisonModeMask; // this has the effect of - // allowing async fetch + HeapMode heap_mode = is_capture_ref ? HeapMode::Locals : HeapMode::Class; + // this has the effect of allowing async fetch + IAGComparisonOptions fetch_options = options & IAGComparisonOptionsComparisonModeMask; ValueLayout layout = TypeDescriptorCache::shared_cache().fetch(*lhs_type, fetch_options, heap_mode, 1); if (layout > ValueLayoutTrivial) { - return compare(layout, lhs, rhs, -1, options & ~IAGComparisonOptionsCopyOnWrite); + return compare(layout, (const unsigned char *)lhs, (const unsigned char *)rhs, -1, + options & ~IAGComparisonOptionsCopyOnWrite); } return false; @@ -681,7 +684,7 @@ Partial find_partial(ValueLayout layout, size_t range_location, size_t range_siz continue; } case ValueLayoutEntryKind::HeapRef: - case ValueLayoutEntryKind::Function: { + case ValueLayoutEntryKind::CaptureRef: { accumulated_size += sizeof(void *); continue; } @@ -840,7 +843,7 @@ void print(std::string &output, ValueLayout layout) { continue; } case ValueLayoutEntryKind::HeapRef: - case ValueLayoutEntryKind::Function: { + case ValueLayoutEntryKind::CaptureRef: { bool is_heap_ref = kind == ValueLayoutEntryKind::HeapRef; output.push_back('\n'); @@ -1287,8 +1290,8 @@ void Builder::Emitter>::operator()(const Ex void Builder::Emitter>::operator()(const HeapRefItem &item) { enter(item); - _data->push_back(item.is_function ? (unsigned char)ValueLayoutEntryKind::Function - : (unsigned char)ValueLayoutEntryKind::HeapRef); + _data->push_back(item.is_capture_ref ? (unsigned char)ValueLayoutEntryKind::CaptureRef + : (unsigned char)ValueLayoutEntryKind::HeapRef); _emitted_size += item.size; } diff --git a/Sources/ComputeCxx/Comparison/LayoutDescriptor.h b/Sources/ComputeCxx/Comparison/LayoutDescriptor.h index e17dce7..96bed2d 100644 --- a/Sources/ComputeCxx/Comparison/LayoutDescriptor.h +++ b/Sources/ComputeCxx/Comparison/LayoutDescriptor.h @@ -58,8 +58,7 @@ bool compare(ValueLayout layout, const unsigned char *lhs, const unsigned char * bool compare_bytes_top_level(const unsigned char *lhs, const unsigned char *rhs, size_t size, IAGComparisonOptions options); bool compare_bytes(char unsigned const *lhs, char unsigned const *rhs, size_t size, size_t *_Nullable failure_location); -bool compare_heap_objects(char unsigned const *lhs, char unsigned const *rhs, IAGComparisonOptions options, - bool is_function); +bool compare_heap_objects(const void *lhs, const void *rhs, IAGComparisonOptions options, bool is_capture_ref); bool compare_indirect(ValueLayout _Nullable *_Nullable layout_ref, const swift::metadata &lhs_type, const swift::metadata &rhs_type, IAGComparisonOptions options, const unsigned char *lhs, const unsigned char *rhs); diff --git a/Sources/ComputeCxx/Comparison/ValueLayout.h b/Sources/ComputeCxx/Comparison/ValueLayout.h index 63a4989..f48f2b6 100644 --- a/Sources/ComputeCxx/Comparison/ValueLayout.h +++ b/Sources/ComputeCxx/Comparison/ValueLayout.h @@ -11,7 +11,7 @@ enum class ValueLayoutEntryKind : uint8_t { Indirect = '\x02', Existential = '\x03', HeapRef = '\x04', - Function = '\x05', + CaptureRef = '\x05', Nested = '\x06', CompactNested = '\x07', From cede134dba4011c789e1b174f637986771967856 Mon Sep 17 00:00:00 2001 From: James Moschou Date: Sun, 23 Aug 2026 13:35:14 +0200 Subject: [PATCH 2/5] Ensure lazily fetched indirect layouts are cached --- Sources/ComputeCxx/Comparison/Compare.cpp | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/Sources/ComputeCxx/Comparison/Compare.cpp b/Sources/ComputeCxx/Comparison/Compare.cpp index 84d20cc..c7d340b 100644 --- a/Sources/ComputeCxx/Comparison/Compare.cpp +++ b/Sources/ComputeCxx/Comparison/Compare.cpp @@ -129,13 +129,24 @@ bool Compare::operator()(ValueLayout layout, const unsigned char *lhs, const uns } case ValueLayoutEntryKind::Indirect: { auto type = reader.read_bytes(); - auto indirect_layout = reader.read_bytes(); + + unsigned char *indirect_layout_ptr = const_cast(reader.layout); + ValueLayout indirect_layout; + std::memcpy(&indirect_layout, indirect_layout_ptr, sizeof(indirect_layout)); + reader.skip(sizeof(indirect_layout)); size_t item_size = type->vw_size(); size_t item_end = offset + item_size; - if (!compare_indirect(&indirect_layout, *_enums.back().type, *type, - options & ~IAGComparisonOptionsTraceCompareFailed, lhs + offset, rhs + offset)) { + bool equal = + compare_indirect(&indirect_layout, *_enums.back().type, *type, + options & ~IAGComparisonOptionsTraceCompareFailed, lhs + offset, rhs + offset); + + // Persist the (possibly newly-fetched) indirect layout + // Layout fetch is idempotent for a given type and options + std::memcpy(indirect_layout_ptr, &indirect_layout, sizeof(indirect_layout)); + + if (!equal) { failed(options, lhs, rhs, offset, item_size, type); return false; } From e2548c9010995c389009c109d7d38b43fffbc3e3 Mon Sep 17 00:00:00 2001 From: James Moschou Date: Sun, 23 Aug 2026 13:36:09 +0200 Subject: [PATCH 3/5] Fix only first byte of indirect value pointers being compared --- Sources/ComputeCxx/Comparison/LayoutDescriptor.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/ComputeCxx/Comparison/LayoutDescriptor.cpp b/Sources/ComputeCxx/Comparison/LayoutDescriptor.cpp index bc553c8..066c608 100644 --- a/Sources/ComputeCxx/Comparison/LayoutDescriptor.cpp +++ b/Sources/ComputeCxx/Comparison/LayoutDescriptor.cpp @@ -552,7 +552,7 @@ bool compare_indirect(ValueLayout *layout_ref, const swift::metadata &enum_type, // compare as heap objects bool result; - if (*lhs_copy == *rhs_copy) { + if (std::memcmp(lhs_copy, rhs_copy, sizeof(void *)) == 0) { // projected data are referentially equal result = true; } else { From 60feaf9e8f6754ef00639d50bfe00ed7e5598752 Mon Sep 17 00:00:00 2001 From: James Moschou Date: Sun, 23 Aug 2026 13:43:31 +0200 Subject: [PATCH 4/5] Avoid strict-aliasing undefined behavior in compare_bytes --- Sources/ComputeCxx/Comparison/LayoutDescriptor.cpp | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/Sources/ComputeCxx/Comparison/LayoutDescriptor.cpp b/Sources/ComputeCxx/Comparison/LayoutDescriptor.cpp index 066c608..5710b86 100644 --- a/Sources/ComputeCxx/Comparison/LayoutDescriptor.cpp +++ b/Sources/ComputeCxx/Comparison/LayoutDescriptor.cpp @@ -452,7 +452,11 @@ bool compare_bytes(const unsigned char *lhs, const unsigned char *rhs, size_t si // If both aligned to 8 bytes, compare 8 bytes at a time if ((((uintptr_t)lhs | (uintptr_t)rhs) & 7) == 0) { while (remaining_size >= 8) { - if (*(uint64_t *)lhs != *(uint64_t *)rhs) { + uint64_t lhs_word; + uint64_t rhs_word; + std::memcpy(&lhs_word, lhs, sizeof(lhs_word)); + std::memcpy(&rhs_word, rhs, sizeof(rhs_word)); + if (lhs_word != rhs_word) { if (failure_location) { *failure_location = location; } @@ -467,7 +471,7 @@ bool compare_bytes(const unsigned char *lhs, const unsigned char *rhs, size_t si // Compare one byte at a time while (remaining_size > 0) { - if (*(uint8_t *)lhs != *(uint8_t *)rhs) { + if (*lhs != *rhs) { if (failure_location) { *failure_location = location; } From b66dc81e2d16687b83769c492cd22ab699f28c0b Mon Sep 17 00:00:00 2001 From: James Moschou Date: Sun, 23 Aug 2026 14:03:07 +0200 Subject: [PATCH 5/5] Update bundle version to 0.5.1 --- Xcode/Configs/Compute.xcconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Xcode/Configs/Compute.xcconfig b/Xcode/Configs/Compute.xcconfig index 20e5df5..3139687 100644 --- a/Xcode/Configs/Compute.xcconfig +++ b/Xcode/Configs/Compute.xcconfig @@ -34,7 +34,7 @@ USE_HEADERMAP = NO CODE_SIGN_STYLE = Automatic CURRENT_PROJECT_VERSION = 1 -MARKETING_VERSION = 0.5.0 +MARKETING_VERSION = 0.5.1 ENABLE_MODULE_VERIFIER = YES MODULE_VERIFIER_SUPPORTED_LANGUAGES = objective-c objective-c++