Skip to content

Fix aliasing bugs in Vector::fill() and Vector::insertFill()#61491

Merged
webkit-commit-queue merged 1 commit into
WebKit:mainfrom
cdumez:310889_Vector_fill_aliasing
Mar 28, 2026
Merged

Fix aliasing bugs in Vector::fill() and Vector::insertFill()#61491
webkit-commit-queue merged 1 commit into
WebKit:mainfrom
cdumez:310889_Vector_fill_aliasing

Conversation

@cdumez
Copy link
Copy Markdown
Contributor

@cdumez cdumez commented Mar 27, 2026

80119b4

Fix aliasing bugs in Vector::fill() and Vector::insertFill()
https://bugs.webkit.org/show_bug.cgi?id=310889

Reviewed by Darin Adler.

Both `fill()` and `insertFill()` take a `const T&` parameter that may reference
an element within the vector itself. If the vector mutates (via shrink,
clear, reallocation, or element shifting), that reference becomes dangling
or points to the wrong value.

For `fill()`, `clear()` destroys all elements before reallocation, and `shrink()`
destroys elements past the new size -- either case invalidates a reference
to a vector element. For `insertFill()`, `expandCapacity()` may reallocate the
buffer, and even without reallocation, `moveOverlapping()` shifts elements
right, so a reference to an element at or after the insertion position
reads the wrong value (or a destructed object for non-trivial types).

Fix both by copying the value into a local before any mutation, matching
how std::vector is required to handle this case for `insert(pos, count, value)`
and `assign(count, value)`.

Test: Tools/TestWebKitAPI/Tests/WTF/Vector.cpp

* Source/WTF/wtf/Vector.h:
(WTF::Malloc>::fill):
(WTF::Malloc>::insertFill):
* Tools/TestWebKitAPI/Tests/WTF/Vector.cpp:
(TestWebKitAPI::TEST(WTF_Vector, FillAliasingGrowth)):
(TestWebKitAPI::TEST(WTF_Vector, FillAliasingShrink)):
(TestWebKitAPI::TEST(WTF_Vector, FillAliasingNoReallocation)):
(TestWebKitAPI::TEST(WTF_Vector, FillAliasingSameSize)):
(TestWebKitAPI::TEST(WTF_Vector, InsertFillAliasingWithReallocation)):
(TestWebKitAPI::TEST(WTF_Vector, InsertFillAliasingElementAfterPosition)):
(TestWebKitAPI::TEST(WTF_Vector, InsertFillAliasingElementAtPosition)):
(TestWebKitAPI::TEST(WTF_Vector, InsertFillAliasingElementBeforePosition)):

Canonical link: https://commits.webkit.org/310131@main

18d0f43

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows Apple Internal
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 win ✅ 🛠 ios-apple
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2 ❌ 🧪 win-tests ✅ 🛠 mac-apple
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe ✅ 🛠 vision-apple
✅ 🧪 ios-wk2-wpt ✅ 🧪 api-mac-debug ✅ 🛠 gtk3-libwebrtc
✅ 🛠 🧪 jsc ✅ 🧪 api-ios ✅ 🧪 mac-wk1 ✅ 🛠 gtk
❌ 🛠 🧪 jsc-debug-arm64 ✅ 🛠 ios-safer-cpp ✅ 🧪 mac-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 vision ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 api-gtk
✅ 🛠 🧪 merge ✅ 🛠 vision-sim ✅ 🧪 mac-wk2-stress ✅ 🛠 playstation
✅ 🧪 vision-wk2 ✅ 🧪 mac-intel-wk2 ✅ 🛠 jsc-armv7
✅ 🛠 tv ✅ 🛠 mac-safer-cpp ✅ 🧪 jsc-armv7-tests
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@cdumez cdumez self-assigned this Mar 27, 2026
@cdumez cdumez marked this pull request as ready for review March 27, 2026 16:07
@cdumez cdumez added the merge-queue Applied to send a pull request to merge-queue label Mar 28, 2026
https://bugs.webkit.org/show_bug.cgi?id=310889

Reviewed by Darin Adler.

Both `fill()` and `insertFill()` take a `const T&` parameter that may reference
an element within the vector itself. If the vector mutates (via shrink,
clear, reallocation, or element shifting), that reference becomes dangling
or points to the wrong value.

For `fill()`, `clear()` destroys all elements before reallocation, and `shrink()`
destroys elements past the new size -- either case invalidates a reference
to a vector element. For `insertFill()`, `expandCapacity()` may reallocate the
buffer, and even without reallocation, `moveOverlapping()` shifts elements
right, so a reference to an element at or after the insertion position
reads the wrong value (or a destructed object for non-trivial types).

Fix both by copying the value into a local before any mutation, matching
how std::vector is required to handle this case for `insert(pos, count, value)`
and `assign(count, value)`.

Test: Tools/TestWebKitAPI/Tests/WTF/Vector.cpp

* Source/WTF/wtf/Vector.h:
(WTF::Malloc>::fill):
(WTF::Malloc>::insertFill):
* Tools/TestWebKitAPI/Tests/WTF/Vector.cpp:
(TestWebKitAPI::TEST(WTF_Vector, FillAliasingGrowth)):
(TestWebKitAPI::TEST(WTF_Vector, FillAliasingShrink)):
(TestWebKitAPI::TEST(WTF_Vector, FillAliasingNoReallocation)):
(TestWebKitAPI::TEST(WTF_Vector, FillAliasingSameSize)):
(TestWebKitAPI::TEST(WTF_Vector, InsertFillAliasingWithReallocation)):
(TestWebKitAPI::TEST(WTF_Vector, InsertFillAliasingElementAfterPosition)):
(TestWebKitAPI::TEST(WTF_Vector, InsertFillAliasingElementAtPosition)):
(TestWebKitAPI::TEST(WTF_Vector, InsertFillAliasingElementBeforePosition)):

Canonical link: https://commits.webkit.org/310131@main
@webkit-commit-queue webkit-commit-queue force-pushed the 310889_Vector_fill_aliasing branch from 18d0f43 to 80119b4 Compare March 28, 2026 01:49
@webkit-commit-queue
Copy link
Copy Markdown
Collaborator

Committed 310131@main (80119b4): https://commits.webkit.org/310131@main

Reviewed commits have been landed. Closing PR #61491 and removing active labels.

@webkit-commit-queue webkit-commit-queue merged commit 80119b4 into WebKit:main Mar 28, 2026
@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Mar 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants