authorgravatar for andrew@ziglang.orgAndrew Kelley <andrew@ziglang.org> 2020-12-15 16:55:56-07:00
committergravatar for andrew@ziglang.orgAndrew Kelley <andrew@ziglang.org> 2020-12-20 15:08:59-07:00
log01d33855c736b04160d9616f138442aa4e41a738
tree16f5d96870a2a69de0786958576c07ce688b8a47
parent4964bb3282bf13de03a79fad1fb9bca104dc1930

stage2: protect mutable state from data races in updateCObject

1 files changed, 53 insertions(+), 28 deletions(-)

src/Compilation.zig+53-28
...@@ -41,7 +41,12 @@ link_error_flags: link.File.ErrorFlags = .{},...@@ -41,7 +41,12 @@ link_error_flags: link.File.ErrorFlags = .{},
4141
42work_queue: std.fifo.LinearFifo(Job, .Dynamic),42work_queue: std.fifo.LinearFifo(Job, .Dynamic),
4343
44/// These jobs are to invoke the Clang compiler to create an object file, which
45/// gets linked with the Compilation.
46c_object_work_queue: std.fifo.LinearFifo(*CObject, .Dynamic),
47
44/// The ErrorMsg memory is owned by the `CObject`, using Compilation's general purpose allocator.48/// The ErrorMsg memory is owned by the `CObject`, using Compilation's general purpose allocator.
49/// This data is accessed by multiple threads and is protected by `mutex`.
45failed_c_objects: std.AutoArrayHashMapUnmanaged(*CObject, *ErrorMsg) = .{},50failed_c_objects: std.AutoArrayHashMapUnmanaged(*CObject, *ErrorMsg) = .{},
4651
47keep_source_files_loaded: bool,52keep_source_files_loaded: bool,
...@@ -111,6 +116,9 @@ owned_link_dir: ?std.fs.Dir,...@@ -111,6 +116,9 @@ owned_link_dir: ?std.fs.Dir,
111/// Don't use this for anything other than stage1 compatibility.116/// Don't use this for anything other than stage1 compatibility.
112color: @import("main.zig").Color = .auto,117color: @import("main.zig").Color = .auto,
113118
119/// This mutex guards all `Compilation` mutable state.
120mutex: std.Mutex = .{},
121
114test_filter: ?[]const u8,122test_filter: ?[]const u8,
115test_name_prefix: ?[]const u8,123test_name_prefix: ?[]const u8,
116test_evented_io: bool,124test_evented_io: bool,
...@@ -150,9 +158,6 @@ const Job = union(enum) {...@@ -150,9 +158,6 @@ const Job = union(enum) {
150 /// The source file containing the Decl has been updated, and so the158 /// The source file containing the Decl has been updated, and so the
151 /// Decl may need its line number information updated in the debug info.159 /// Decl may need its line number information updated in the debug info.
152 update_line_number: *Module.Decl,160 update_line_number: *Module.Decl,
153 /// Invoke the Clang compiler to create an object file, which gets linked
154 /// with the Compilation.
155 c_object: *CObject,
156161
157 /// one of the glibc static objects162 /// one of the glibc static objects
158 glibc_crt_file: glibc.CRTFile,163 glibc_crt_file: glibc.CRTFile,
...@@ -971,6 +976,7 @@ pub fn create(gpa: *Allocator, options: InitOptions) !*Compilation {...@@ -971,6 +976,7 @@ pub fn create(gpa: *Allocator, options: InitOptions) !*Compilation {
971 .emit_analysis = options.emit_analysis,976 .emit_analysis = options.emit_analysis,
972 .emit_docs = options.emit_docs,977 .emit_docs = options.emit_docs,
973 .work_queue = std.fifo.LinearFifo(Job, .Dynamic).init(gpa),978 .work_queue = std.fifo.LinearFifo(Job, .Dynamic).init(gpa),
979 .c_object_work_queue = std.fifo.LinearFifo(*CObject, .Dynamic).init(gpa),
974 .keep_source_files_loaded = options.keep_source_files_loaded,980 .keep_source_files_loaded = options.keep_source_files_loaded,
975 .use_clang = use_clang,981 .use_clang = use_clang,
976 .clang_argv = options.clang_argv,982 .clang_argv = options.clang_argv,
...@@ -1190,11 +1196,13 @@ pub fn update(self: *Compilation) !void {...@@ -1190,11 +1196,13 @@ pub fn update(self: *Compilation) !void {
1190 const tracy = trace(@src());1196 const tracy = trace(@src());
1191 defer tracy.end();1197 defer tracy.end();
11921198
1199 self.c_object_cache_digest_set.clearRetainingCapacity();
1200
1193 // For compiling C objects, we rely on the cache hash system to avoid duplicating work.1201 // For compiling C objects, we rely on the cache hash system to avoid duplicating work.
1194 // Add a Job for each C object.1202 // Add a Job for each C object.
1195 try self.work_queue.ensureUnusedCapacity(self.c_object_table.items().len);1203 try self.c_object_work_queue.ensureUnusedCapacity(self.c_object_table.items().len);
1196 for (self.c_object_table.items()) |entry| {1204 for (self.c_object_table.items()) |entry| {
1197 self.work_queue.writeItemAssumeCapacity(.{ .c_object = entry.key });1205 self.c_object_work_queue.writeItemAssumeCapacity(entry.key);
1198 }1206 }
11991207
1200 const use_stage1 = build_options.is_stage1 and self.bin_file.options.use_llvm;1208 const use_stage1 = build_options.is_stage1 and self.bin_file.options.use_llvm;
...@@ -1372,6 +1380,26 @@ pub fn performAllTheWork(self: *Compilation) error{ TimerUnsupported, OutOfMemor...@@ -1372,6 +1380,26 @@ pub fn performAllTheWork(self: *Compilation) error{ TimerUnsupported, OutOfMemor
1372 var c_comp_progress_node = main_progress_node.start("Compile C Objects", self.c_source_files.len);1380 var c_comp_progress_node = main_progress_node.start("Compile C Objects", self.c_source_files.len);
1373 defer c_comp_progress_node.end();1381 defer c_comp_progress_node.end();
13741382
1383 while (self.c_object_work_queue.readItem()) |c_object| {
1384 self.updateCObject(c_object, &c_comp_progress_node) catch |err| switch (err) {
1385 error.AnalysisFail => continue,
1386 else => {
1387 {
1388 var lock = self.mutex.acquire();
1389 defer lock.release();
1390 try self.failed_c_objects.ensureCapacity(self.gpa, self.failed_c_objects.items().len + 1);
1391 self.failed_c_objects.putAssumeCapacityNoClobber(c_object, try ErrorMsg.create(
1392 self.gpa,
1393 0,
1394 "unable to build C object: {s}",
1395 .{@errorName(err)},
1396 ));
1397 }
1398 c_object.status = .{ .failure = {} };
1399 },
1400 };
1401 }
1402
1375 while (self.work_queue.readItem()) |work_item| switch (work_item) {1403 while (self.work_queue.readItem()) |work_item| switch (work_item) {
1376 .codegen_decl => |decl| switch (decl.analysis) {1404 .codegen_decl => |decl| switch (decl.analysis) {
1377 .unreferenced => unreachable,1405 .unreferenced => unreachable,
...@@ -1447,21 +1475,6 @@ pub fn performAllTheWork(self: *Compilation) error{ TimerUnsupported, OutOfMemor...@@ -1447,21 +1475,6 @@ pub fn performAllTheWork(self: *Compilation) error{ TimerUnsupported, OutOfMemor
1447 decl.analysis = .codegen_failure_retryable;1475 decl.analysis = .codegen_failure_retryable;
1448 };1476 };
1449 },1477 },
1450 .c_object => |c_object| {
1451 self.updateCObject(c_object, &c_comp_progress_node) catch |err| switch (err) {
1452 error.AnalysisFail => continue,
1453 else => {
1454 try self.failed_c_objects.ensureCapacity(self.gpa, self.failed_c_objects.items().len + 1);
1455 self.failed_c_objects.putAssumeCapacityNoClobber(c_object, try ErrorMsg.create(
1456 self.gpa,
1457 0,
1458 "unable to build C object: {s}",
1459 .{@errorName(err)},
1460 ));
1461 c_object.status = .{ .failure = {} };
1462 },
1463 };
1464 },
1465 .glibc_crt_file => |crt_file| {1478 .glibc_crt_file => |crt_file| {
1466 glibc.buildCRTFile(self, crt_file) catch |err| {1479 glibc.buildCRTFile(self, crt_file) catch |err| {
1467 // TODO Expose this as a normal compile error rather than crashing here.1480 // TODO Expose this as a normal compile error rather than crashing here.
...@@ -1553,7 +1566,7 @@ pub fn performAllTheWork(self: *Compilation) error{ TimerUnsupported, OutOfMemor...@@ -1553,7 +1566,7 @@ pub fn performAllTheWork(self: *Compilation) error{ TimerUnsupported, OutOfMemor
1553 };1566 };
1554}1567}
15551568
1556pub fn obtainCObjectCacheManifest(comp: *Compilation) Cache.Manifest {1569pub fn obtainCObjectCacheManifest(comp: *const Compilation) Cache.Manifest {
1557 var man = comp.cache_parent.obtain();1570 var man = comp.cache_parent.obtain();
15581571
1559 // Only things that need to be added on top of the base hash, and only things1572 // Only things that need to be added on top of the base hash, and only things
...@@ -1720,6 +1733,8 @@ fn updateCObject(comp: *Compilation, c_object: *CObject, c_comp_progress_node: *...@@ -1720,6 +1733,8 @@ fn updateCObject(comp: *Compilation, c_object: *CObject, c_comp_progress_node: *
17201733
1721 if (c_object.clearStatus(comp.gpa)) {1734 if (c_object.clearStatus(comp.gpa)) {
1722 // There was previous failure.1735 // There was previous failure.
1736 var lock = comp.mutex.acquire();
1737 defer lock.release();
1723 comp.failed_c_objects.removeAssertDiscard(c_object);1738 comp.failed_c_objects.removeAssertDiscard(c_object);
1724 }1739 }
17251740
...@@ -1747,8 +1762,14 @@ fn updateCObject(comp: *Compilation, c_object: *CObject, c_comp_progress_node: *...@@ -1747,8 +1762,14 @@ fn updateCObject(comp: *Compilation, c_object: *CObject, c_comp_progress_node: *
1747 }1762 }
17481763
1749 {1764 {
1750 const gop = try comp.c_object_cache_digest_set.getOrPut(comp.gpa, man.hash.peekBin());1765 const is_collision = blk: {
1751 if (gop.found_existing) {1766 var lock = comp.mutex.acquire();
1767 defer lock.release();
1768
1769 const gop = try comp.c_object_cache_digest_set.getOrPut(comp.gpa, man.hash.peekBin());
1770 break :blk gop.found_existing;
1771 };
1772 if (is_collision) {
1752 return comp.failCObj(1773 return comp.failCObj(
1753 c_object,1774 c_object,
1754 "the same source file was already added to the same compilation with the same flags",1775 "the same source file was already added to the same compilation with the same flags",
...@@ -1929,7 +1950,7 @@ pub fn addTranslateCCArgs(...@@ -1929,7 +1950,7 @@ pub fn addTranslateCCArgs(
19291950
1930/// Add common C compiler args between translate-c and C object compilation.1951/// Add common C compiler args between translate-c and C object compilation.
1931pub fn addCCArgs(1952pub fn addCCArgs(
1932 comp: *Compilation,1953 comp: *const Compilation,
1933 arena: *Allocator,1954 arena: *Allocator,
1934 argv: *std.ArrayList([]const u8),1955 argv: *std.ArrayList([]const u8),
1935 ext: FileExt,1956 ext: FileExt,
...@@ -2164,10 +2185,14 @@ fn failCObj(comp: *Compilation, c_object: *CObject, comptime format: []const u8,...@@ -2164,10 +2185,14 @@ fn failCObj(comp: *Compilation, c_object: *CObject, comptime format: []const u8,
21642185
2165fn failCObjWithOwnedErrorMsg(comp: *Compilation, c_object: *CObject, err_msg: *ErrorMsg) InnerError {2186fn failCObjWithOwnedErrorMsg(comp: *Compilation, c_object: *CObject, err_msg: *ErrorMsg) InnerError {
2166 {2187 {
2167 errdefer err_msg.destroy(comp.gpa);2188 var lock = comp.mutex.acquire();
2168 try comp.failed_c_objects.ensureCapacity(comp.gpa, comp.failed_c_objects.items().len + 1);2189 defer lock.release();
2190 {
2191 errdefer err_msg.destroy(comp.gpa);
2192 try comp.failed_c_objects.ensureCapacity(comp.gpa, comp.failed_c_objects.items().len + 1);
2193 }
2194 comp.failed_c_objects.putAssumeCapacityNoClobber(c_object, err_msg);
2169 }2195 }
2170 comp.failed_c_objects.putAssumeCapacityNoClobber(c_object, err_msg);
2171 c_object.status = .failure;2196 c_object.status = .failure;
2172 return error.AnalysisFail;2197 return error.AnalysisFail;
2173}2198}
...@@ -2324,7 +2349,7 @@ test "classifyFileExt" {...@@ -2324,7 +2349,7 @@ test "classifyFileExt" {
2324 std.testing.expectEqual(FileExt.zir, classifyFileExt("foo.zir"));2349 std.testing.expectEqual(FileExt.zir, classifyFileExt("foo.zir"));
2325}2350}
23262351
2327fn haveFramePointer(comp: *Compilation) bool {2352fn haveFramePointer(comp: *const Compilation) bool {
2328 // If you complicate this logic make sure you update the parent cache hash.2353 // If you complicate this logic make sure you update the parent cache hash.
2329 // Right now it's not in the cache hash because the value depends on optimize_mode2354 // Right now it's not in the cache hash because the value depends on optimize_mode
2330 // and strip which are both already part of the hash.2355 // and strip which are both already part of the hash.