From 4b776ae44139fb1a3601949b60ff580f3f16e73f Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Tue, 11 Jun 2024 15:15:21 -0700 Subject: [PATCH 1/2] std.Progress: fix race assertion failure A node may be freed during the execution of this loop, causing there to be a parent reference to a nonexistent node. Without this assignment, this would lead to the map entry containing stale data. By assigning none, the child node with the bad parent pointer will be harmlessly omitted from the tree. Closes #20262 --- lib/std/Progress.zig | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/lib/std/Progress.zig b/lib/std/Progress.zig index 3a7bc0d5af..c7dffe4c3f 100644 --- a/lib/std/Progress.zig +++ b/lib/std/Progress.zig @@ -734,7 +734,7 @@ const Serialized = struct { const Buffer = struct { parents: [node_storage_buffer_len]Node.Parent, storage: [node_storage_buffer_len]Node.Storage, - map: [node_storage_buffer_len]Node.Index, + map: [node_storage_buffer_len]Node.OptionalIndex, parents_copy: [node_storage_buffer_len]Node.Parent, storage_copy: [node_storage_buffer_len]Node.Storage, @@ -753,9 +753,11 @@ fn serialize(serialized_buffer: *Serialized.Buffer) Serialized { // Iterate all of the nodes and construct a serializable copy of the state that can be examined // without atomics. const end_index = @atomicLoad(u32, &global_progress.node_end_index, .monotonic); - const node_parents = global_progress.node_parents[0..end_index]; - const node_storage = global_progress.node_storage[0..end_index]; - for (node_parents, node_storage, 0..) |*parent_ptr, *storage_ptr, i| { + for ( + global_progress.node_parents[0..end_index], + global_progress.node_storage[0..end_index], + serialized_buffer.map[0..end_index], + ) |*parent_ptr, *storage_ptr, *map| { var begin_parent = @atomicLoad(Node.Parent, parent_ptr, .acquire); while (begin_parent != .unused) { const dest_storage = &serialized_buffer.storage[serialized_len]; @@ -766,12 +768,19 @@ fn serialize(serialized_buffer: *Serialized.Buffer) Serialized { if (begin_parent == end_parent) { any_ipc = any_ipc or (dest_storage.getIpcFd() != null); serialized_buffer.parents[serialized_len] = begin_parent; - serialized_buffer.map[i] = @enumFromInt(serialized_len); + map.* = @enumFromInt(serialized_len); serialized_len += 1; break; } begin_parent = end_parent; + } else { + // A node may be freed during the execution of this loop, causing + // there to be a parent reference to a nonexistent node. Without + // this assignment, this would lead to the map entry containing + // stale data. By assigning none, the child node with the bad + // parent pointer will be harmlessly omitted from the tree. + map.* = .none; } } From 727f1fa74316e412c32828689e569a3d1fd1981b Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Tue, 11 Jun 2024 15:16:04 -0700 Subject: [PATCH 2/2] update update_cpu_features tool to latest std.Progress API closes #20261 --- tools/update_cpu_features.zig | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/tools/update_cpu_features.zig b/tools/update_cpu_features.zig index 0690cc2e69..6d76fc0eb5 100644 --- a/tools/update_cpu_features.zig +++ b/tools/update_cpu_features.zig @@ -1038,8 +1038,7 @@ pub fn main() anyerror!void { var zig_src_dir = try fs.cwd().openDir(zig_src_root, .{}); defer zig_src_dir.close(); - var progress = std.Progress{}; - const root_progress = progress.start("", llvm_targets.len); + const root_progress = std.Progress.start(.{ .estimated_total_items = llvm_targets.len }); defer root_progress.end(); if (builtin.single_threaded) { @@ -1074,7 +1073,7 @@ const Job = struct { llvm_tblgen_exe: []const u8, llvm_src_root: []const u8, zig_src_dir: std.fs.Dir, - root_progress: *std.Progress.Node, + root_progress: std.Progress.Node, llvm_target: LlvmTarget, }; @@ -1085,12 +1084,10 @@ fn processOneTarget(job: Job) anyerror!void { defer arena_state.deinit(); const arena = arena_state.allocator(); - var progress_node = job.root_progress.start(llvm_target.zig_name, 3); - progress_node.activate(); + const progress_node = job.root_progress.start(llvm_target.zig_name, 3); defer progress_node.end(); - var tblgen_progress = progress_node.start("invoke llvm-tblgen", 0); - tblgen_progress.activate(); + const tblgen_progress = progress_node.start("invoke llvm-tblgen", 0); const child_args = [_][]const u8{ job.llvm_tblgen_exe, @@ -1127,16 +1124,14 @@ fn processOneTarget(job: Job) anyerror!void { }, }; - var json_parse_progress = progress_node.start("parse JSON", 0); - json_parse_progress.activate(); + const json_parse_progress = progress_node.start("parse JSON", 0); const parsed = try json.parseFromSlice(json.Value, arena, json_text, .{}); defer parsed.deinit(); const root_map = &parsed.value.object; json_parse_progress.end(); - var render_progress = progress_node.start("render zig code", 0); - render_progress.activate(); + const render_progress = progress_node.start("render zig code", 0); var features_table = std.StringHashMap(Feature).init(arena); var all_features = std.ArrayList(Feature).init(arena);