[runtime] Recreate enum cache on map updateIf we had one before, we probably want one after too.Bug: chromium:1470668Change-Id: Ib83f7b9549b5686a16d35dd7114bf88b12d0a3a0Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4771019Auto-Submit: Leszek Swirski <leszeks@chromium.org>Commit-Queue: Tobias Tebbi <tebbi@chromium.org>Reviewed-by: Tobias Tebbi <tebbi@chromium.org>Cr-Commit-Position: refs/heads/main@{#89488}
diff --git a/src/objects/map-updater.cc b/src/objects/map-updater.ccindex 7660fab..7a864d9 100644--- a/src/objects/map-updater.cc+++ b/src/objects/map-updater.cc@@ -12,6 +12,7 @@ #include "src/handles/handles.h" #include "src/heap/parked-scope-inl.h" #include "src/objects/field-type.h"+#include "src/objects/keys.h" #include "src/objects/objects-inl.h" #include "src/objects/objects.h" #include "src/objects/property-details.h"@@ -1038,6 +1039,12 @@ // the new descriptors to maintain descriptors sharing invariant. split_map->ReplaceDescriptors(isolate_, *new_descriptors);+ // If the old descriptors had an enum cache, make sure the new ones do too.+ if (old_descriptors_->enum_cache()->keys()->length() > 0) {+ FastKeyAccumulator::InitializeFastPropertyEnumCache(+ isolate_, new_map, new_map->NumberOfEnumerableProperties());+ }+ if (has_integrity_level_transition_) { target_map_ = new_map; state_ = kAtIntegrityLevelSource;
Có thể thấy, commit này thêm vào đoạn code giúp tạo ra enum cache cho map mới của object nếu map cũ có enum cache.
Commit ngày 14/08/2023:
[runtime] Don't try to create empty enum cache.When copying maps and the new map has no enumerable properties weshould not try to initialize an enum cache.This happens if the deprecation is due to making the only property ina map non enumerable.Bug: chromium:1472317, chromium:1470668Change-Id: I7a6af63e50dc30592e2caacce0caccfb31f534cfReviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4775581Reviewed-by: Tobias Tebbi <tebbi@chromium.org>Commit-Queue: Olivier Flückiger <olivf@chromium.org>Cr-Commit-Position: refs/heads/main@{#89534}
diff --git a/src/objects/map-updater.cc b/src/objects/map-updater.ccindex 7a864d9..9c20491 100644--- a/src/objects/map-updater.cc+++ b/src/objects/map-updater.cc@@ -1040,7 +1040,8 @@ split_map->ReplaceDescriptors(isolate_, *new_descriptors); // If the old descriptors had an enum cache, make sure the new ones do too.- if (old_descriptors_->enum_cache()->keys()->length() > 0) {+ if (old_descriptors_->enum_cache()->keys()->length() > 0 &&+ new_map->NumberOfEnumerableProperties() > 0) { FastKeyAccumulator::InitializeFastPropertyEnumCache( isolate_, new_map, new_map->NumberOfEnumerableProperties()); }
Có vẻ như việc tạo ra enum cache cho map mới có thể dẫn đến một empty enum cache. Commit này đã thêm vào một điều kiện giúp kiểm tra xem map mới có các thuộc tính enumerable hay không.
Cuối cùng, commit ngày 17/08/2023 đã merge hai commit trên lại với nhau:
diff --git a/src/objects/map-updater.cc b/src/objects/map-updater.ccindex 8b2e7f3..568df12 100644--- a/src/objects/map-updater.cc+++ b/src/objects/map-updater.cc@@ -12,6 +12,7 @@ #include "src/handles/handles.h" #include "src/heap/parked-scope-inl.h" #include "src/objects/field-type.h"+#include "src/objects/keys.h" #include "src/objects/objects-inl.h" #include "src/objects/objects.h" #include "src/objects/property-details.h"@@ -1037,6 +1038,13 @@ // the new descriptors to maintain descriptors sharing invariant. split_map->ReplaceDescriptors(isolate_, *new_descriptors);+ // If the old descriptors had an enum cache, make sure the new ones do too.+ if (old_descriptors_->enum_cache().keys().length() > 0 &&+ new_map->NumberOfEnumerableProperties() > 0) {+ FastKeyAccumulator::InitializeFastPropertyEnumCache(+ isolate_, new_map, new_map->NumberOfEnumerableProperties());+ }+ if (has_integrity_level_transition_) { target_map_ = new_map; state_ = kAtIntegrityLevelSource;
Preliminaries
Vòng lặp for in sẽ tìm enum cache trong instance descriptors của map. Sau đó, nó sẽ nạp lên mảng keys của enum cache.
Hàm ForInEnumerate sẽ giúp nạp lên keys của enum cache.
Trigger
Nếu thay đổi map của object3 thì sẽ làm thay đổi instance descriptors cũng như là enum cache của object2. Trước khi được fix, enum cache của object3 (cũng là của object2) chưa được khởi tạo mặc dù trước đó nó có hai phần tử là a và b.
Dẫn đến, khi object2 truy xuất thuộc tính a và b mà không thông qua JSLoadProperty do code đã được tối ưu (và bị thay thế bằng LoadFieldByIndex), nó sẽ sử dụng những index có trong indices của enum cache để truy xuất thuộc tính mà không kiểm tra xem indices có thực sự chứa những index đó hay không.
Giá trị thu được có thể là của một trường nào khác trong đối tượng FIXED_ARRAY_TYPE và không phải là index hợp lệ. Khi V8 sử dụng index này để truy xuất thuộc tính, nó có thể gây ra OOB access.
Sau khi object3 thay đổi map, vòng lặp for (let key in object1) { } sẽ giúp sinh ra enum cache cho thuộc tính a. Lúc này, indices của enum cache sẽ chuyển từ trạng thái rỗng sang trạng thái có một phần tử. Khi đó, giá trị của thuộc tính a vẫn được in ra như bình thường.
Tuy nhiên, đối với thuộc tính b thì lại khác. Ta sẽ xem xét các đoạn native code tại dòng console.log(object2[key]) đối với key là b sau khi map của object3 đã bị thay đổi.
Gắn GDB vào câu lệnh readline cuối cùng (trước khi gọi console.log(object2[key])).
Do pointer tagging, giá trị 0x000001f1 thực chất sẽ là:
gef➤ p 0x000001f1/2$3 = 0xf8
Important
Có thể thấy, giá trị này rất lớn và chắc chắn không phải index của thuộc tính. Xem xét vùng nhớ của indices thì ta thấy rằng chỉ có byte thứ 3 có chứa index của thuộc tính a: