authorgravatar for andrew@ziglang.orgAndrew Kelley <andrew@ziglang.org> 2024-03-16 15:45:10-07:00
committergravatar for noreply@github.comGitHub <noreply@github.com> 2024-03-16 18:45:10-04:00
log242ab81112c05fa815523551a6f612c5a12c52b2
tree32b581f036137436932883ffdfab24aec64576db
parent1b8d1b18c7ec1c8002046d8a7e131bf21ccf92ca
signaturebadge-check Signed by PGP key B5690EEEBB952194

std: introduce pointer stability locks to hash maps (#17719)

This adds std.debug.SafetyLock and uses it in std.HashMapUnmanaged by adding lockPointers() and unlockPointers(). This provides a way to detect when an illegal modification has happened and panic rather than invoke undefined behavior.

3 files changed, 203 insertions(+), 31 deletions(-)

lib/std/array_hash_map.zig+102-5
......@@ -137,6 +137,23 @@ pub fn ArrayHashMap(
137137 self.* = undefined;
138138 }
139139
140 /// Puts the hash map into a state where any method call that would
141 /// cause an existing key or value pointer to become invalidated will
142 /// instead trigger an assertion.
143 ///
144 /// An additional call to `lockPointers` in such state also triggers an
145 /// assertion.
146 ///
147 /// `unlockPointers` returns the hash map to the previous state.
148 pub fn lockPointers(self: *Self) void {
149 self.unmanaged.lockPointers();
150 }
151
152 /// Undoes a call to `lockPointers`.
153 pub fn unlockPointers(self: *Self) void {
154 self.unmanaged.unlockPointers();
155 }
156
140157 /// Clears the map but retains the backing allocation for future use.
141158 pub fn clearRetainingCapacity(self: *Self) void {
142159 return self.unmanaged.clearRetainingCapacity();
......@@ -403,6 +420,7 @@ pub fn ArrayHashMap(
403420 /// Set the map to an empty state, making deinitialization a no-op, and
404421 /// returning a copy of the original.
405422 pub fn move(self: *Self) Self {
423 self.pointer_stability.assertUnlocked();
406424 const result = self.*;
407425 self.unmanaged = .{};
408426 return result;
......@@ -495,6 +513,9 @@ pub fn ArrayHashMapUnmanaged(
495513 /// by how many total indexes there are.
496514 index_header: ?*IndexHeader = null,
497515
516 /// Used to detect memory safety violations.
517 pointer_stability: std.debug.SafetyLock = .{},
518
498519 comptime {
499520 std.hash_map.verifyContext(Context, K, K, u32, true);
500521 }
......@@ -589,6 +610,7 @@ pub fn ArrayHashMapUnmanaged(
589610 /// Note that this does not free keys or values. You must take care of that
590611 /// before calling this function, if it is needed.
591612 pub fn deinit(self: *Self, allocator: Allocator) void {
613 self.pointer_stability.assertUnlocked();
592614 self.entries.deinit(allocator);
593615 if (self.index_header) |header| {
594616 header.free(allocator);
......@@ -596,8 +618,28 @@ pub fn ArrayHashMapUnmanaged(
596618 self.* = undefined;
597619 }
598620
621 /// Puts the hash map into a state where any method call that would
622 /// cause an existing key or value pointer to become invalidated will
623 /// instead trigger an assertion.
624 ///
625 /// An additional call to `lockPointers` in such state also triggers an
626 /// assertion.
627 ///
628 /// `unlockPointers` returns the hash map to the previous state.
629 pub fn lockPointers(self: *Self) void {
630 self.pointer_stability.lock();
631 }
632
633 /// Undoes a call to `lockPointers`.
634 pub fn unlockPointers(self: *Self) void {
635 self.pointer_stability.unlock();
636 }
637
599638 /// Clears the map but retains the backing allocation for future use.
600639 pub fn clearRetainingCapacity(self: *Self) void {
640 self.pointer_stability.lock();
641 defer self.pointer_stability.unlock();
642
601643 self.entries.len = 0;
602644 if (self.index_header) |header| {
603645 switch (header.capacityIndexType()) {
......@@ -610,6 +652,9 @@ pub fn ArrayHashMapUnmanaged(
610652
611653 /// Clears the map and releases the backing allocation
612654 pub fn clearAndFree(self: *Self, allocator: Allocator) void {
655 self.pointer_stability.lock();
656 defer self.pointer_stability.unlock();
657
613658 self.entries.shrinkAndFree(allocator, 0);
614659 if (self.index_header) |header| {
615660 header.free(allocator);
......@@ -795,6 +840,9 @@ pub fn ArrayHashMapUnmanaged(
795840 return self.ensureTotalCapacityContext(allocator, new_capacity, undefined);
796841 }
797842 pub fn ensureTotalCapacityContext(self: *Self, allocator: Allocator, new_capacity: usize, ctx: Context) !void {
843 self.pointer_stability.lock();
844 defer self.pointer_stability.unlock();
845
798846 if (new_capacity <= linear_scan_max) {
799847 try self.entries.ensureTotalCapacity(allocator, new_capacity);
800848 return;
......@@ -1079,6 +1127,9 @@ pub fn ArrayHashMapUnmanaged(
10791127 return self.fetchSwapRemoveContextAdapted(key, ctx, undefined);
10801128 }
10811129 pub fn fetchSwapRemoveContextAdapted(self: *Self, key: anytype, key_ctx: anytype, ctx: Context) ?KV {
1130 self.pointer_stability.lock();
1131 defer self.pointer_stability.unlock();
1132
10821133 return self.fetchRemoveByKey(key, key_ctx, if (store_hash) {} else ctx, .swap);
10831134 }
10841135
......@@ -1100,6 +1151,9 @@ pub fn ArrayHashMapUnmanaged(
11001151 return self.fetchOrderedRemoveContextAdapted(key, ctx, undefined);
11011152 }
11021153 pub fn fetchOrderedRemoveContextAdapted(self: *Self, key: anytype, key_ctx: anytype, ctx: Context) ?KV {
1154 self.pointer_stability.lock();
1155 defer self.pointer_stability.unlock();
1156
11031157 return self.fetchRemoveByKey(key, key_ctx, if (store_hash) {} else ctx, .ordered);
11041158 }
11051159
......@@ -1121,6 +1175,9 @@ pub fn ArrayHashMapUnmanaged(
11211175 return self.swapRemoveContextAdapted(key, ctx, undefined);
11221176 }
11231177 pub fn swapRemoveContextAdapted(self: *Self, key: anytype, key_ctx: anytype, ctx: Context) bool {
1178 self.pointer_stability.lock();
1179 defer self.pointer_stability.unlock();
1180
11241181 return self.removeByKey(key, key_ctx, if (store_hash) {} else ctx, .swap);
11251182 }
11261183
......@@ -1142,6 +1199,9 @@ pub fn ArrayHashMapUnmanaged(
11421199 return self.orderedRemoveContextAdapted(key, ctx, undefined);
11431200 }
11441201 pub fn orderedRemoveContextAdapted(self: *Self, key: anytype, key_ctx: anytype, ctx: Context) bool {
1202 self.pointer_stability.lock();
1203 defer self.pointer_stability.unlock();
1204
11451205 return self.removeByKey(key, key_ctx, if (store_hash) {} else ctx, .ordered);
11461206 }
11471207
......@@ -1154,6 +1214,9 @@ pub fn ArrayHashMapUnmanaged(
11541214 return self.swapRemoveAtContext(index, undefined);
11551215 }
11561216 pub fn swapRemoveAtContext(self: *Self, index: usize, ctx: Context) void {
1217 self.pointer_stability.lock();
1218 defer self.pointer_stability.unlock();
1219
11571220 self.removeByIndex(index, if (store_hash) {} else ctx, .swap);
11581221 }
11591222
......@@ -1167,6 +1230,9 @@ pub fn ArrayHashMapUnmanaged(
11671230 return self.orderedRemoveAtContext(index, undefined);
11681231 }
11691232 pub fn orderedRemoveAtContext(self: *Self, index: usize, ctx: Context) void {
1233 self.pointer_stability.lock();
1234 defer self.pointer_stability.unlock();
1235
11701236 self.removeByIndex(index, if (store_hash) {} else ctx, .ordered);
11711237 }
11721238
......@@ -1196,6 +1262,7 @@ pub fn ArrayHashMapUnmanaged(
11961262 /// Set the map to an empty state, making deinitialization a no-op, and
11971263 /// returning a copy of the original.
11981264 pub fn move(self: *Self) Self {
1265 self.pointer_stability.assertUnlocked();
11991266 const result = self.*;
12001267 self.* = .{};
12011268 return result;
......@@ -1271,6 +1338,9 @@ pub fn ArrayHashMapUnmanaged(
12711338 sort_ctx: anytype,
12721339 ctx: Context,
12731340 ) void {
1341 self.pointer_stability.lock();
1342 defer self.pointer_stability.unlock();
1343
12741344 switch (mode) {
12751345 .stable => self.entries.sort(sort_ctx),
12761346 .unstable => self.entries.sortUnstable(sort_ctx),
......@@ -1288,6 +1358,9 @@ pub fn ArrayHashMapUnmanaged(
12881358 return self.shrinkRetainingCapacityContext(new_len, undefined);
12891359 }
12901360 pub fn shrinkRetainingCapacityContext(self: *Self, new_len: usize, ctx: Context) void {
1361 self.pointer_stability.lock();
1362 defer self.pointer_stability.unlock();
1363
12911364 // Remove index entries from the new length onwards.
12921365 // Explicitly choose to ONLY remove index entries and not the underlying array list
12931366 // entries as we're going to remove them in the subsequent shrink call.
......@@ -1307,6 +1380,9 @@ pub fn ArrayHashMapUnmanaged(
13071380 return self.shrinkAndFreeContext(allocator, new_len, undefined);
13081381 }
13091382 pub fn shrinkAndFreeContext(self: *Self, allocator: Allocator, new_len: usize, ctx: Context) void {
1383 self.pointer_stability.lock();
1384 defer self.pointer_stability.unlock();
1385
13101386 // Remove index entries from the new length onwards.
13111387 // Explicitly choose to ONLY remove index entries and not the underlying array list
13121388 // entries as we're going to remove them in the subsequent shrink call.
......@@ -1325,6 +1401,9 @@ pub fn ArrayHashMapUnmanaged(
13251401 return self.popContext(undefined);
13261402 }
13271403 pub fn popContext(self: *Self, ctx: Context) KV {
1404 self.pointer_stability.lock();
1405 defer self.pointer_stability.unlock();
1406
13281407 const item = self.entries.get(self.entries.len - 1);
13291408 if (self.index_header) |header|
13301409 self.removeFromIndexByIndex(self.entries.len - 1, if (store_hash) {} else ctx, header);
......@@ -1346,9 +1425,13 @@ pub fn ArrayHashMapUnmanaged(
13461425 return if (self.entries.len == 0) null else self.popContext(ctx);
13471426 }
13481427
1349 // ------------------ No pub fns below this point ------------------
1350
1351 fn fetchRemoveByKey(self: *Self, key: anytype, key_ctx: anytype, ctx: ByIndexContext, comptime removal_type: RemovalType) ?KV {
1428 fn fetchRemoveByKey(
1429 self: *Self,
1430 key: anytype,
1431 key_ctx: anytype,
1432 ctx: ByIndexContext,
1433 comptime removal_type: RemovalType,
1434 ) ?KV {
13521435 const header = self.index_header orelse {
13531436 // Linear scan.
13541437 const key_hash = if (store_hash) key_ctx.hash(key) else {};
......@@ -1377,7 +1460,15 @@ pub fn ArrayHashMapUnmanaged(
13771460 .u32 => self.fetchRemoveByKeyGeneric(key, key_ctx, ctx, header, u32, removal_type),
13781461 };
13791462 }
1380 fn fetchRemoveByKeyGeneric(self: *Self, key: anytype, key_ctx: anytype, ctx: ByIndexContext, header: *IndexHeader, comptime I: type, comptime removal_type: RemovalType) ?KV {
1463 fn fetchRemoveByKeyGeneric(
1464 self: *Self,
1465 key: anytype,
1466 key_ctx: anytype,
1467 ctx: ByIndexContext,
1468 header: *IndexHeader,
1469 comptime I: type,
1470 comptime removal_type: RemovalType,
1471 ) ?KV {
13811472 const indexes = header.indexes(I);
13821473 const entry_index = self.removeFromIndexByKey(key, key_ctx, header, I, indexes) orelse return null;
13831474 const slice = self.entries.slice();
......@@ -1389,7 +1480,13 @@ pub fn ArrayHashMapUnmanaged(
13891480 return removed_entry;
13901481 }
13911482
1392 fn removeByKey(self: *Self, key: anytype, key_ctx: anytype, ctx: ByIndexContext, comptime removal_type: RemovalType) bool {
1483 fn removeByKey(
1484 self: *Self,
1485 key: anytype,
1486 key_ctx: anytype,
1487 ctx: ByIndexContext,
1488 comptime removal_type: RemovalType,
1489 ) bool {
13931490 const header = self.index_header orelse {
13941491 // Linear scan.
13951492 const key_hash = if (store_hash) key_ctx.hash(key) else {};
lib/std/debug.zig+23
......@@ -2838,6 +2838,29 @@ pub fn ConfigurableTrace(comptime size: usize, comptime stack_frame_count: usize
28382838 };
28392839}
28402840
2841pub const SafetyLock = struct {
2842 state: State = .unlocked,
2843
2844 pub const State = if (runtime_safety) enum { unlocked, locked } else enum { unlocked };
2845
2846 pub fn lock(l: *SafetyLock) void {
2847 if (!runtime_safety) return;
2848 assert(l.state == .unlocked);
2849 l.state = .locked;
2850 }
2851
2852 pub fn unlock(l: *SafetyLock) void {
2853 if (!runtime_safety) return;
2854 assert(l.state == .locked);
2855 l.state = .unlocked;
2856 }
2857
2858 pub fn assertUnlocked(l: SafetyLock) void {
2859 if (!runtime_safety) return;
2860 assert(l.state == .unlocked);
2861 }
2862};
2863
28412864test {
28422865 _ = &dump_hex;
28432866}
lib/std/hash_map.zig+78-26
......@@ -416,6 +416,23 @@ pub fn HashMap(
416416 };
417417 }
418418
419 /// Puts the hash map into a state where any method call that would
420 /// cause an existing key or value pointer to become invalidated will
421 /// instead trigger an assertion.
422 ///
423 /// An additional call to `lockPointers` in such state also triggers an
424 /// assertion.
425 ///
426 /// `unlockPointers` returns the hash map to the previous state.
427 pub fn lockPointers(self: *Self) void {
428 self.unmanaged.lockPointers();
429 }
430
431 /// Undoes a call to `lockPointers`.
432 pub fn unlockPointers(self: *Self) void {
433 self.unmanaged.unlockPointers();
434 }
435
419436 /// Release the backing array and invalidate this map.
420437 /// This does *not* deinit keys, values, or the context!
421438 /// If your keys or values need to be released, ensure
......@@ -672,6 +689,7 @@ pub fn HashMap(
672689 /// Set the map to an empty state, making deinitialization a no-op, and
673690 /// returning a copy of the original.
674691 pub fn move(self: *Self) Self {
692 self.unmanaged.pointer_stability.assertUnlocked();
675693 const result = self.*;
676694 self.unmanaged = .{};
677695 return result;
......@@ -722,6 +740,9 @@ pub fn HashMapUnmanaged(
722740 /// `max_load_percentage`.
723741 available: Size = 0,
724742
743 /// Used to detect memory safety violations.
744 pointer_stability: std.debug.SafetyLock = .{},
745
725746 // This is purely empirical and not a /very smart magic constant™/.
726747 /// Capacity of the first grow when bootstrapping the hashmap.
727748 const minimal_capacity = 8;
......@@ -884,11 +905,29 @@ pub fn HashMapUnmanaged(
884905 };
885906 }
886907
908 /// Puts the hash map into a state where any method call that would
909 /// cause an existing key or value pointer to become invalidated will
910 /// instead trigger an assertion.
911 ///
912 /// An additional call to `lockPointers` in such state also triggers an
913 /// assertion.
914 ///
915 /// `unlockPointers` returns the hash map to the previous state.
916 pub fn lockPointers(self: *Self) void {
917 self.pointer_stability.lock();
918 }
919
920 /// Undoes a call to `lockPointers`.
921 pub fn unlockPointers(self: *Self) void {
922 self.pointer_stability.unlock();
923 }
924
887925 fn isUnderMaxLoadPercentage(size: Size, cap: Size) bool {
888926 return size * 100 < max_load_percentage * cap;
889927 }
890928
891929 pub fn deinit(self: *Self, allocator: Allocator) void {
930 self.pointer_stability.assertUnlocked();
892931 self.deallocate(allocator);
893932 self.* = undefined;
894933 }
......@@ -905,6 +944,8 @@ pub fn HashMapUnmanaged(
905944 return ensureTotalCapacityContext(self, allocator, new_size, undefined);
906945 }
907946 pub fn ensureTotalCapacityContext(self: *Self, allocator: Allocator, new_size: Size, ctx: Context) Allocator.Error!void {
947 self.pointer_stability.lock();
948 defer self.pointer_stability.unlock();
908949 if (new_size > self.size)
909950 try self.growIfNeeded(allocator, new_size - self.size, ctx);
910951 }
......@@ -919,14 +960,18 @@ pub fn HashMapUnmanaged(
919960 }
920961
921962 pub fn clearRetainingCapacity(self: *Self) void {
963 self.pointer_stability.lock();
964 defer self.pointer_stability.unlock();
922965 if (self.metadata) |_| {
923966 self.initMetadatas();
924967 self.size = 0;
925 self.available = @as(u32, @truncate((self.capacity() * max_load_percentage) / 100));
968 self.available = @truncate((self.capacity() * max_load_percentage) / 100);
926969 }
927970 }
928971
929972 pub fn clearAndFree(self: *Self, allocator: Allocator) void {
973 self.pointer_stability.lock();
974 defer self.pointer_stability.unlock();
930975 self.deallocate(allocator);
931976 self.size = 0;
932977 self.available = 0;
......@@ -997,9 +1042,11 @@ pub fn HashMapUnmanaged(
9971042 return self.putNoClobberContext(allocator, key, value, undefined);
9981043 }
9991044 pub fn putNoClobberContext(self: *Self, allocator: Allocator, key: K, value: V, ctx: Context) Allocator.Error!void {
1000 assert(!self.containsContext(key, ctx));
1001 try self.growIfNeeded(allocator, 1, ctx);
1002
1045 {
1046 self.pointer_stability.lock();
1047 defer self.pointer_stability.unlock();
1048 try self.growIfNeeded(allocator, 1, ctx);
1049 }
10031050 self.putAssumeCapacityNoClobberContext(key, value, ctx);
10041051 }
10051052
......@@ -1028,7 +1075,7 @@ pub fn HashMapUnmanaged(
10281075
10291076 const hash = ctx.hash(key);
10301077 const mask = self.capacity() - 1;
1031 var idx = @as(usize, @truncate(hash & mask));
1078 var idx: usize = @truncate(hash & mask);
10321079
10331080 var metadata = self.metadata.? + idx;
10341081 while (metadata[0].isUsed()) {
......@@ -1280,17 +1327,21 @@ pub fn HashMapUnmanaged(
12801327 return self.getOrPutContextAdapted(allocator, key, key_ctx, undefined);
12811328 }
12821329 pub fn getOrPutContextAdapted(self: *Self, allocator: Allocator, key: anytype, key_ctx: anytype, ctx: Context) Allocator.Error!GetOrPutResult {
1283 self.growIfNeeded(allocator, 1, ctx) catch |err| {
1284 // If allocation fails, try to do the lookup anyway.
1285 // If we find an existing item, we can return it.
1286 // Otherwise return the error, we could not add another.
1287 const index = self.getIndex(key, key_ctx) orelse return err;
1288 return GetOrPutResult{
1289 .key_ptr = &self.keys()[index],
1290 .value_ptr = &self.values()[index],
1291 .found_existing = true,
1330 {
1331 self.pointer_stability.lock();
1332 defer self.pointer_stability.unlock();
1333 self.growIfNeeded(allocator, 1, ctx) catch |err| {
1334 // If allocation fails, try to do the lookup anyway.
1335 // If we find an existing item, we can return it.
1336 // Otherwise return the error, we could not add another.
1337 const index = self.getIndex(key, key_ctx) orelse return err;
1338 return GetOrPutResult{
1339 .key_ptr = &self.keys()[index],
1340 .value_ptr = &self.values()[index],
1341 .found_existing = true,
1342 };
12921343 };
1293 };
1344 }
12941345 return self.getOrPutAssumeCapacityAdapted(key, key_ctx);
12951346 }
12961347
......@@ -1495,6 +1546,7 @@ pub fn HashMapUnmanaged(
14951546 /// Set the map to an empty state, making deinitialization a no-op, and
14961547 /// returning a copy of the original.
14971548 pub fn move(self: *Self) Self {
1549 self.pointer_stability.assertUnlocked();
14981550 const result = self.*;
14991551 self.* = .{};
15001552 return result;
......@@ -1506,28 +1558,28 @@ pub fn HashMapUnmanaged(
15061558 assert(new_cap > self.capacity());
15071559 assert(std.math.isPowerOfTwo(new_cap));
15081560
1509 var map = Self{};
1561 var map: Self = .{};
15101562 defer map.deinit(allocator);
1563 map.pointer_stability.lock();
15111564 try map.allocate(allocator, new_cap);
15121565 map.initMetadatas();
15131566 map.available = @truncate((new_cap * max_load_percentage) / 100);
15141567
15151568 if (self.size != 0) {
15161569 const old_capacity = self.capacity();
1517 var i: Size = 0;
1518 var metadata = self.metadata.?;
1519 const keys_ptr = self.keys();
1520 const values_ptr = self.values();
1521 while (i < old_capacity) : (i += 1) {
1522 if (metadata[i].isUsed()) {
1523 map.putAssumeCapacityNoClobberContext(keys_ptr[i], values_ptr[i], ctx);
1524 if (map.size == self.size)
1525 break;
1526 }
1570 for (
1571 self.metadata.?[0..old_capacity],
1572 self.keys()[0..old_capacity],
1573 self.values()[0..old_capacity],
1574 ) |m, k, v| {
1575 if (!m.isUsed()) continue;
1576 map.putAssumeCapacityNoClobberContext(k, v, ctx);
1577 if (map.size == self.size) break;
15271578 }
15281579 }
15291580
15301581 self.size = 0;
1582 self.pointer_stability = .{ .state = .unlocked };
15311583 std.mem.swap(Self, self, &map);
15321584 }
15331585