Fix serialization bug

rtfeldman · rtfeldman · commit dc4673291f2f · 2025-11-24T00:00:15.000-05:00
diff --git a/src/base/CommonEnv.zig b/src/base/CommonEnv.zig
@@ -117,18 +117,26 @@ pub const Serialized = extern struct {
         offset: i64,
         source: []const u8,
     ) *CommonEnv {
-        // Note: Serialized may be smaller than the runtime struct because:
-        // - Uses i64 offsets instead of usize pointers (same size on 64-bit, but conceptually different)
-        // - May have different alignment/padding requirements
-        // We deserialize by overwriting the Serialized memory with the runtime struct.
+        // CRITICAL: We must deserialize ALL fields into local variables BEFORE writing to the
+        // output struct. This is because CommonEnv is a regular struct (not extern), so Zig may
+        // reorder its fields differently than Serialized (which is extern). If we read from self
+        // while writing to env (which aliases self), we may read corrupted data in Release mode
+        // when field orderings differ.
+
+        // Step 1: Deserialize all fields into local variables first
+        const deserialized_idents = self.idents.deserialize(offset).*;
+        const deserialized_strings = self.strings.deserialize(offset).*;
+        const deserialized_exposed_items = self.exposed_items.deserialize(offset).*;
+        const deserialized_line_starts = self.line_starts.deserialize(offset).*;
+
+        // Step 2: Overwrite ourself with the deserialized version
         const env = @as(*CommonEnv, @ptrFromInt(@intFromPtr(self)));
 
         env.* = CommonEnv{
-            .idents = self.idents.deserialize(offset).*,
-            // .ident_ids_for_slicing = self.ident_ids_for_slicing.deserialize(offset).*,
-            .strings = self.strings.deserialize(offset).*,
-            .exposed_items = self.exposed_items.deserialize(offset).*,
-            .line_starts = self.line_starts.deserialize(offset).*,
+            .idents = deserialized_idents,
+            .strings = deserialized_strings,
+            .exposed_items = deserialized_exposed_items,
+            .line_starts = deserialized_line_starts,
             .source = source,
         };
 
diff --git a/src/base/Ident.zig b/src/base/Ident.zig
@@ -120,15 +120,25 @@ pub const Store = struct {
         }
 
         /// Deserialize this Serialized struct into a Store
-        pub fn deserialize(self: *Serialized, offset: i64) *Store {
-            // Note: Serialized may be smaller than the runtime struct.
-            // We deserialize by overwriting the Serialized memory with the runtime struct.
+        pub noinline fn deserialize(self: *Serialized, offset: i64) *Store {
+            // CRITICAL: We must deserialize ALL fields into local variables BEFORE writing to the
+            // output struct. This is because Store is a regular struct (not extern), so Zig may
+            // reorder its fields differently than Serialized (which is extern). If we read from self
+            // while writing to store (which aliases self), we may read corrupted data in Release mode
+            // when field orderings differ.
+
+            // Step 1: Deserialize all fields into local variables first
+            const deserialized_interner = self.interner.deserialize(offset).*;
+            const deserialized_attributes = self.attributes.deserialize(offset).*;
+            const deserialized_next_unique_name = self.next_unique_name;
+
+            // Step 2: Overwrite ourself with the deserialized version
             const store = @as(*Store, @ptrFromInt(@intFromPtr(self)));
 
             store.* = Store{
-                .interner = self.interner.deserialize(offset).*,
-                .attributes = self.attributes.deserialize(offset).*,
-                .next_unique_name = self.next_unique_name,
+                .interner = deserialized_interner,
+                .attributes = deserialized_attributes,
+                .next_unique_name = deserialized_next_unique_name,
             };
 
             return store;
diff --git a/src/base/SmallStringInterner.zig b/src/base/SmallStringInterner.zig
@@ -226,14 +226,25 @@ pub const Serialized = extern struct {
     }
 
     /// Deserialize this Serialized struct into a SmallStringInterner
-    pub fn deserialize(self: *Serialized, offset: i64) *SmallStringInterner {
-        // Overwrite ourself with the deserialized version, and return our pointer after casting it to Self.
+    pub noinline fn deserialize(self: *Serialized, offset: i64) *SmallStringInterner {
+        // CRITICAL: We must deserialize ALL fields into local variables BEFORE writing to the
+        // output struct. This is because SmallStringInterner is a regular struct (not extern), so Zig may
+        // reorder its fields differently than Serialized (which is extern). If we read from self
+        // while writing to interner (which aliases self), we may read corrupted data in Release mode
+        // when field orderings differ.
+
+        // Step 1: Deserialize all fields into local variables first
+        const deserialized_bytes = self.bytes.deserialize(offset).*;
+        const deserialized_hash_table = self.hash_table.deserialize(offset).*;
+        const deserialized_entry_count = self.entry_count;
+
+        // Step 2: Overwrite ourself with the deserialized version
         const interner = @as(*SmallStringInterner, @ptrCast(self));
 
         interner.* = .{
-            .bytes = self.bytes.deserialize(offset).*,
-            .hash_table = self.hash_table.deserialize(offset).*,
-            .entry_count = self.entry_count,
+            .bytes = deserialized_bytes,
+            .hash_table = deserialized_hash_table,
+            .entry_count = deserialized_entry_count,
         };
 
         return interner;
diff --git a/src/base/StringLiteral.zig b/src/base/StringLiteral.zig
@@ -113,12 +113,18 @@ pub const Store = struct {
         }
 
         /// Deserialize this Serialized struct into a Store
-        pub fn deserialize(self: *Serialized, offset: i64) *Store {
-            // Overwrite ourself with the deserialized version, and return our pointer after casting it to Self.
+        pub noinline fn deserialize(self: *Serialized, offset: i64) *Store {
+            // CRITICAL: We must deserialize ALL fields into local variables BEFORE writing to the
+            // output struct to avoid aliasing issues in Release mode.
+
+            // Step 1: Deserialize all fields into local variables first
+            const deserialized_buffer = self.buffer.deserialize(offset).*;
+
+            // Step 2: Overwrite ourself with the deserialized version
             const store = @as(*Store, @ptrFromInt(@intFromPtr(self)));
 
             store.* = Store{
-                .buffer = self.buffer.deserialize(offset).*,
+                .buffer = deserialized_buffer,
             };
 
             return store;
diff --git a/src/canonicalize/CIR.zig b/src/canonicalize/CIR.zig
@@ -704,13 +704,17 @@ pub const Import = struct {
             }
 
             /// Deserialize this Serialized struct into a Store
-            pub fn deserialize(self: *Serialized, offset: i64, allocator: std.mem.Allocator) std.mem.Allocator.Error!*Store {
-                // Overwrite ourself with the deserialized version, and return our pointer after casting it to Store.
+            pub noinline fn deserialize(self: *Serialized, offset: i64, allocator: std.mem.Allocator) std.mem.Allocator.Error!*Store {
+                // CRITICAL: Deserialize nested struct BEFORE casting and writing to store
+                // to avoid aliasing issues in Release mode.
+                const deserialized_imports = self.imports.deserialize(offset).*;
+
+                // Now we can cast and write to the destination
                 const store = @as(*Store, @ptrFromInt(@intFromPtr(self)));
 
                 store.* = .{
                     .map = .{}, // Will be repopulated below
-                    .imports = self.imports.deserialize(offset).*,
+                    .imports = deserialized_imports,
                 };
 
                 // Pre-allocate the exact capacity needed for the map
diff --git a/src/canonicalize/ModuleEnv.zig b/src/canonicalize/ModuleEnv.zig
@@ -1791,7 +1791,7 @@ pub const Serialized = extern struct {
     }
 
     /// Deserialize a ModuleEnv from the buffer, updating the ModuleEnv in place
-    pub fn deserialize(
+    pub noinline fn deserialize(
         self: *Serialized,
         offset: i64,
         gpa: std.mem.Allocator,
@@ -1803,47 +1803,66 @@ pub const Serialized = extern struct {
         // On 32-bit platforms, Serialized may be larger due to using fixed-size types for platform-independent serialization.
         comptime std.debug.assert(@sizeOf(@This()) >= @sizeOf(Self));
 
-        // Overwrite ourself with the deserialized version, and return our pointer after casting it to Self.
+        // CRITICAL: We must deserialize ALL fields into local variables BEFORE writing to the
+        // output struct. This is because ModuleEnv is a regular struct (not extern), so Zig may
+        // reorder its fields differently than Serialized (which is extern). If we read from self
+        // while writing to env (which aliases self), we may read corrupted data in Release mode
+        // when field orderings differ.
+        //
+        // Following the same pattern as NodeStore.deserialize to avoid aliasing issues.
+
+        // Step 1: Deserialize all complex fields into local variables first
+        const deserialized_common = self.common.deserialize(offset, source).*;
+        const deserialized_types = self.types.deserialize(offset, gpa).*;
+        const deserialized_module_kind = self.module_kind.decode();
+        const deserialized_all_defs = self.all_defs;
+        const deserialized_all_statements = self.all_statements;
+        const deserialized_exports = self.exports;
+        const deserialized_builtin_statements = self.builtin_statements;
+        const deserialized_external_decls = self.external_decls.deserialize(offset).*;
+        const deserialized_imports = (try self.imports.deserialize(offset, gpa)).*;
+        const deserialized_diagnostics = self.diagnostics;
+        const deserialized_store = self.store.deserialize(offset, gpa).*;
+        const deserialized_deferred_numeric_literals = self.deferred_numeric_literals.deserialize(offset).*;
+
+        // Step 2: Overwrite ourself with the deserialized version
         const env = @as(*Self, @ptrFromInt(@intFromPtr(self)));
 
-        // Deserialize common env first so we can look up identifiers
-        const common = self.common.deserialize(offset, source).*;
-
         env.* = Self{
             .gpa = gpa,
-            .common = common,
-            .types = self.types.deserialize(offset, gpa).*,
-            .module_kind = self.module_kind.decode(),
-            .all_defs = self.all_defs,
-            .all_statements = self.all_statements,
-            .exports = self.exports,
-            .builtin_statements = self.builtin_statements,
-            .external_decls = self.external_decls.deserialize(offset).*,
-            .imports = (try self.imports.deserialize(offset, gpa)).*,
+            .common = deserialized_common,
+            .types = deserialized_types,
+            .module_kind = deserialized_module_kind,
+            .all_defs = deserialized_all_defs,
+            .all_statements = deserialized_all_statements,
+            .exports = deserialized_exports,
+            .builtin_statements = deserialized_builtin_statements,
+            .external_decls = deserialized_external_decls,
+            .imports = deserialized_imports,
             .module_name = module_name,
             .module_name_idx = undefined, // Not used for deserialized modules (only needed during fresh canonicalization)
-            .diagnostics = self.diagnostics,
-            .store = self.store.deserialize(offset, gpa).*,
+            .diagnostics = deserialized_diagnostics,
+            .store = deserialized_store,
             .evaluation_order = null, // Not serialized, will be recomputed if needed
             // Well-known identifiers for type checking - look them up in the deserialized common env
-            .try_ident = common.findIdent("Try") orelse unreachable,
-            .out_of_range_ident = common.findIdent("OutOfRange") orelse unreachable,
-            .builtin_module_ident = common.findIdent("Builtin") orelse unreachable,
-            .plus_ident = common.findIdent(Ident.PLUS_METHOD_NAME) orelse unreachable,
-            .minus_ident = common.findIdent("minus") orelse unreachable,
-            .times_ident = common.findIdent("times") orelse unreachable,
-            .div_by_ident = common.findIdent("div_by") orelse unreachable,
-            .div_trunc_by_ident = common.findIdent("div_trunc_by") orelse unreachable,
-            .rem_by_ident = common.findIdent("rem_by") orelse unreachable,
-            .negate_ident = common.findIdent(Ident.NEGATE_METHOD_NAME) orelse unreachable,
-            .not_ident = common.findIdent("not") orelse unreachable,
-            .is_lt_ident = common.findIdent("is_lt") orelse unreachable,
-            .is_lte_ident = common.findIdent("is_lte") orelse unreachable,
-            .is_gt_ident = common.findIdent("is_gt") orelse unreachable,
-            .is_gte_ident = common.findIdent("is_gte") orelse unreachable,
-            .is_eq_ident = common.findIdent("is_eq") orelse unreachable,
-            .is_ne_ident = common.findIdent("is_ne") orelse unreachable,
-            .deferred_numeric_literals = self.deferred_numeric_literals.deserialize(offset).*,
+            .try_ident = deserialized_common.findIdent("Try") orelse unreachable,
+            .out_of_range_ident = deserialized_common.findIdent("OutOfRange") orelse unreachable,
+            .builtin_module_ident = deserialized_common.findIdent("Builtin") orelse unreachable,
+            .plus_ident = deserialized_common.findIdent(Ident.PLUS_METHOD_NAME) orelse unreachable,
+            .minus_ident = deserialized_common.findIdent("minus") orelse unreachable,
+            .times_ident = deserialized_common.findIdent("times") orelse unreachable,
+            .div_by_ident = deserialized_common.findIdent("div_by") orelse unreachable,
+            .div_trunc_by_ident = deserialized_common.findIdent("div_trunc_by") orelse unreachable,
+            .rem_by_ident = deserialized_common.findIdent("rem_by") orelse unreachable,
+            .negate_ident = deserialized_common.findIdent(Ident.NEGATE_METHOD_NAME) orelse unreachable,
+            .not_ident = deserialized_common.findIdent("not") orelse unreachable,
+            .is_lt_ident = deserialized_common.findIdent("is_lt") orelse unreachable,
+            .is_lte_ident = deserialized_common.findIdent("is_lte") orelse unreachable,
+            .is_gt_ident = deserialized_common.findIdent("is_gt") orelse unreachable,
+            .is_gte_ident = deserialized_common.findIdent("is_gte") orelse unreachable,
+            .is_eq_ident = deserialized_common.findIdent("is_eq") orelse unreachable,
+            .is_ne_ident = deserialized_common.findIdent("is_ne") orelse unreachable,
+            .deferred_numeric_literals = deserialized_deferred_numeric_literals,
         };
 
         return env;
diff --git a/src/canonicalize/NodeStore.zig b/src/canonicalize/NodeStore.zig
@@ -3522,7 +3522,7 @@ pub const Serialized = extern struct {
     }
 
     /// Deserialize this Serialized struct into a NodeStore
-    pub fn deserialize(self: *Serialized, offset: i64, gpa: Allocator) *NodeStore {
+    pub noinline fn deserialize(self: *Serialized, offset: i64, gpa: Allocator) *NodeStore {
         // Note: Serialized may be smaller than the runtime struct.
         // CRITICAL: On 32-bit platforms, deserializing nodes in-place corrupts the adjacent
         // regions and extra_data fields. We must deserialize in REVERSE order (last to first)
diff --git a/src/collections/ExposedItems.zig b/src/collections/ExposedItems.zig
@@ -121,13 +121,17 @@ pub const ExposedItems = struct {
         }
 
         /// Deserialize this Serialized struct into an ExposedItems
-        pub fn deserialize(self: *Serialized, offset: i64) *ExposedItems {
-            // Note: Serialized may be smaller than the runtime struct.
-            // We deserialize by overwriting the Serialized memory with the runtime struct.
+        pub noinline fn deserialize(self: *Serialized, offset: i64) *ExposedItems {
+            // CRITICAL: Deserialize nested struct BEFORE casting and writing to exposed_items.
+            // Since exposed_items aliases self (they point to the same memory), we must complete
+            // all reads before any writes to avoid corruption in Release mode.
+            const deserialized_items = self.items.deserialize(offset).*;
+
+            // Now we can cast and write to the destination
             const exposed_items = @as(*ExposedItems, @ptrFromInt(@intFromPtr(self)));
 
             exposed_items.* = ExposedItems{
-                .items = self.items.deserialize(offset).*,
+                .items = deserialized_items,
             };
 
             return exposed_items;
diff --git a/src/collections/SortedArrayBuilder.zig b/src/collections/SortedArrayBuilder.zig
@@ -299,33 +299,35 @@ pub fn SortedArrayBuilder(comptime K: type, comptime V: type) type {
             }
 
             /// Deserialize this Serialized struct into a SortedArrayBuilder
-            pub fn deserialize(self: *Serialized, offset: i64) *SortedArrayBuilder(K, V) {
-                // Note: Serialized may be smaller than the runtime struct because:
-                // - Uses i64 offsets instead of usize pointers
-                // - Omits runtime-only fields like allocators
-                // - May have different alignment/padding requirements
-                // We deserialize by overwriting the Serialized memory with the runtime struct.
-
-                // Overwrite ourself with the deserialized version, and return our pointer after casting it to Self.
+            pub noinline fn deserialize(self: *Serialized, offset: i64) *SortedArrayBuilder(K, V) {
+                // CRITICAL: Read ALL fields from self BEFORE casting and writing to builder.
+                // Since builder aliases self (they point to the same memory), we must complete
+                // all reads before any writes to avoid corruption in Release mode.
+                const serialized_entries_offset = self.entries_offset;
+                const serialized_entries_len = self.entries_len;
+                const serialized_entries_capacity = self.entries_capacity;
+                const serialized_sorted = self.sorted;
+
+                // Now we can cast and write to the destination
                 const builder = @as(*SortedArrayBuilder(K, V), @ptrFromInt(@intFromPtr(self)));
 
                 // Handle empty array case
-                if (self.entries_len == 0) {
+                if (serialized_entries_len == 0) {
                     builder.* = SortedArrayBuilder(K, V){
                         .entries = .{},
-                        .sorted = self.sorted,
+                        .sorted = serialized_sorted,
                     };
                 } else {
                     // Apply the offset to convert from serialized offset to actual pointer
-                    const entries_ptr_usize: usize = @intCast(self.entries_offset + offset);
+                    const entries_ptr_usize: usize = @intCast(serialized_entries_offset + offset);
                     const entries_ptr: [*]Entry = @ptrFromInt(entries_ptr_usize);
 
                     builder.* = SortedArrayBuilder(K, V){
                         .entries = .{
-                            .items = entries_ptr[0..@intCast(self.entries_len)],
-                            .capacity = @intCast(self.entries_capacity),
+                            .items = entries_ptr[0..@intCast(serialized_entries_len)],
+                            .capacity = @intCast(serialized_entries_capacity),
                         },
-                        .sorted = self.sorted,
+                        .sorted = serialized_sorted,
                     };
                 }
 
diff --git a/src/collections/safe_list.zig b/src/collections/safe_list.zig
diff --git a/src/compile/cache_module.zig b/src/compile/cache_module.zig
diff --git a/src/types/store.zig b/src/types/store.zig
diff --git a/test/serialization_size_check.zig b/test/serialization_size_check.zig

Original file line number	Diff line number	Diff line change
`@@ -3522,7 +3522,7 @@ pub const Serialized = extern struct {`
`3522`	`3522`	`}`
`3523`	`3523`
`3524`	`3524`	`/// Deserialize this Serialized struct into a NodeStore`
`3525`		`- pub fn deserialize(self: Serialized, offset: i64, gpa: Allocator) NodeStore {`
	`3525`	`+ pub noinline fn deserialize(self: Serialized, offset: i64, gpa: Allocator) NodeStore {`
`3526`	`3526`	`// Note: Serialized may be smaller than the runtime struct.`
`3527`	`3527`	`// CRITICAL: On 32-bit platforms, deserializing nodes in-place corrupts the adjacent`
`3528`	`3528`	`// regions and extra_data fields. We must deserialize in REVERSE order (last to first)`