Skip to content

Commit

Permalink
copy out keys also in HashMap.copy_keys (#1569)
Browse files Browse the repository at this point in the history
* copy out keys also in HashMap.copy_keys
* test for copying keys out
  • Loading branch information
hwchen authored Oct 25, 2024
1 parent 8ce6310 commit fd1898b
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 2 deletions.
6 changes: 5 additions & 1 deletion lib/std/collections/hashmap.c3
Original file line number Diff line number Diff line change
Expand Up @@ -313,7 +313,11 @@ fn Key[] HashMap.copy_keys(&map, Allocator allocator = allocator::heap())
{
while (entry)
{
list[index++] = entry.key;
$if COPY_KEYS:
list[index++] = entry.key.copy(allocator);
$else
list[index++] = entry.key;
$endif
entry = entry.next;
}
}
Expand Down
1 change: 1 addition & 0 deletions releasenotes.md
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@
- `&self` argument not implicitly null checked. #1556.
- `(uptr)&((Foo*)null).a` incorrectly inserts a null check. #1544
- Incorrect error message when `$eval` is provided an invalid string. #1570
- `HashMap.copy_keys` did not properly copy keys which needed to be allocated #1569

### Stdlib changes
- Remove unintended print of `char[]` as String
Expand Down
21 changes: 20 additions & 1 deletion test/unit/stdlib/collections/copy_map.c3
Original file line number Diff line number Diff line change
Expand Up @@ -28,4 +28,23 @@ fn void! copy_map() @test
y.free();
assert(alloc.allocated() == 0);
};
}
}

/*
Some Keys (including Strings) are deep_copied into the hashmap on insertion.
When copying keys out, the keys themselves must also be deep-copied out.
Otherwise when the map is freed, the copied-in keys will also be freed,
resulting in use-after-free.
*/
fn void! copy_keys() @test
{
String[] y;
@pool() {
IntMap x;
x.temp_init();
x.set("hello", 0); // keys copied into temp hashmap
y = x.copy_keys(allocator::heap()); // keys copied out
// end of pool: hashmap and its copied-in keys dropped
};
assert(y == {"hello"});
}

0 comments on commit fd1898b

Please sign in to comment.