⚠ Archived content — this site is no longer maintained.   Current WebKit documentation is at docs.webkit.org.

Changeset 245918 in webkit


Ignore:
Timestamp:
May 30, 2019, 5:29:53 PM (7 years ago)
Author:
Alan Coon
Message:

Cherry-pick r245018. rdar://problem/51264878

tryCachePutByID should not crash if target offset changes
​https://bugs.webkit.org/show_bug.cgi?id=197311
<rdar://problem/48033612>

Reviewed by Filip Pizlo.

JSTests:

Add a series of tests related tryCachePutByID. Two of these tests used to crash and were fixed
by this patch: cache-put-by-id-different-attributes.js and cache-put-by-id-different-offset.js

  • stress/cache-put-by-id-delete-prototype.js: Added. (A.prototype.set y): (A): (B.prototype.set y): (B): (C):
  • stress/cache-put-by-id-different-proto.js: Added. (A.prototype.set y): (A): (B1): (B2.prototype.set y): (B2): (C): (D):
  • stress/cache-put-by-id-different-attributes.js: Added. (Foo): (set x):
  • stress/cache-put-by-id-different-offset.js: Added. (Foo): (set x):
  • stress/cache-put-by-id-insert-prototype.js: Added. (A.prototype.set y): (A): (C):
  • stress/cache-put-by-id-poly-proto.js: Added. (Foo): (set _): (createBar.Bar): (createBar):

Source/JavaScriptCore:

When tryCachePutID is called with a cacheable setter, if the target object where the setter was
found is still in the prototype chain and there's no poly protos in the chain, we use
generateConditionsForPrototypePropertyHit to validate that the target object remains the same.
It checks for the absence of the property in every object in the prototype chain from the base
down to the target object and checks that the property is still present in the target object. It
also bails if there are any uncacheable objects, proxies or dictionary objects in the prototype
chain. However, it does not consider two edge cases:

  • It asserts that the property should still be at the same offset in the target object, but this assertion does not hold if the setter deletes properties of the object and causes the structure to be flattened after the deletion. Instead of asserting, we just use the updated offset.
  • It does not check whether the new slot is also a setter, which leads to a crash in case it's not.
  • jit/Repatch.cpp: (JSC::tryCachePutByID):

git-svn-id: ​https://svn.webkit.org/repository/webkit/trunk@245018 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Location:
branches/safari-607-branch
Files:
6 added
3 edited

Legend:

Unmodified
Added
Removed

  • branches/safari-607-branch/JSTests/ChangeLog

    r245917 r245918  
     12019-05-30  Kocsen Chung  <kocsen_chung@apple.com>
     2
     3        Cherry-pick r245018. rdar://problem/51264878
     4
     5    tryCachePutByID should not crash if target offset changes
     6    https://bugs.webkit.org/show_bug.cgi?id=197311
     7    <rdar://problem/48033612>
     8   
     9    Reviewed by Filip Pizlo.
     10   
     11    JSTests:
     12   
     13    Add a series of tests related tryCachePutByID. Two of these tests used to crash and were fixed
     14    by this patch: `cache-put-by-id-different-attributes.js` and `cache-put-by-id-different-offset.js`
     15   
     16    * stress/cache-put-by-id-delete-prototype.js: Added.
     17    (A.prototype.set y):
     18    (A):
     19    (B.prototype.set y):
     20    (B):
     21    (C):
     22    * stress/cache-put-by-id-different-__proto__.js: Added.
     23    (A.prototype.set y):
     24    (A):
     25    (B1):
     26    (B2.prototype.set y):
     27    (B2):
     28    (C):
     29    (D):
     30    * stress/cache-put-by-id-different-attributes.js: Added.
     31    (Foo):
     32    (set x):
     33    * stress/cache-put-by-id-different-offset.js: Added.
     34    (Foo):
     35    (set x):
     36    * stress/cache-put-by-id-insert-prototype.js: Added.
     37    (A.prototype.set y):
     38    (A):
     39    (C):
     40    * stress/cache-put-by-id-poly-proto.js: Added.
     41    (Foo):
     42    (set _):
     43    (createBar.Bar):
     44    (createBar):
     45   
     46    Source/JavaScriptCore:
     47   
     48    When tryCachePutID is called with a cacheable setter, if the target object where the setter was
     49    found is still in the prototype chain and there's no poly protos in the chain, we use
     50    generateConditionsForPrototypePropertyHit to validate that the target object remains the same.
     51    It checks for the absence of the property in every object in the prototype chain from the base
     52    down to the target object and checks that the property is still present in the target object. It
     53    also bails if there are any uncacheable objects, proxies or dictionary objects in the prototype
     54    chain. However, it does not consider two edge cases:
     55    - It asserts that the property should still be at the same offset in the target object, but this
     56    assertion does not hold if the setter deletes properties of the object and causes the structure
     57    to be flattened after the deletion. Instead of asserting, we just use the updated offset.
     58    - It does not check whether the new slot is also a setter, which leads to a crash in case it's not.
     59   
     60    * jit/Repatch.cpp:
     61    (JSC::tryCachePutByID):
     62   
     63   
     64    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@245018 268f45cc-cd09-0410-ab3c-d52691b4dbfc
     65
     66    2019-05-07  Tadeu Zagallo  <tzagallo@apple.com>
     67
     68            tryCachePutByID should not crash if target offset changes
     69            https://bugs.webkit.org/show_bug.cgi?id=197311
     70            <rdar://problem/48033612>
     71
     72            Reviewed by Filip Pizlo.
     73
     74            Add a series of tests related tryCachePutByID. Two of these tests used to crash and were fixed
     75            by this patch: `cache-put-by-id-different-attributes.js` and `cache-put-by-id-different-offset.js`
     76
     77            * stress/cache-put-by-id-delete-prototype.js: Added.
     78            (A.prototype.set y):
     79            (A):
     80            (B.prototype.set y):
     81            (B):
     82            (C):
     83            * stress/cache-put-by-id-different-__proto__.js: Added.
     84            (A.prototype.set y):
     85            (A):
     86            (B1):
     87            (B2.prototype.set y):
     88            (B2):
     89            (C):
     90            (D):
     91            * stress/cache-put-by-id-different-attributes.js: Added.
     92            (Foo):
     93            (set x):
     94            * stress/cache-put-by-id-different-offset.js: Added.
     95            (Foo):
     96            (set x):
     97            * stress/cache-put-by-id-insert-prototype.js: Added.
     98            (A.prototype.set y):
     99            (A):
     100            (C):
     101            * stress/cache-put-by-id-poly-proto.js: Added.
     102            (Foo):
     103            (set _):
     104            (createBar.Bar):
     105            (createBar):
     106
    11072019-05-30  Kocsen Chung  <kocsen_chung@apple.com>
    2108

  • branches/safari-607-branch/Source/JavaScriptCore/ChangeLog

    r245917 r245918  
     12019-05-30  Kocsen Chung  <kocsen_chung@apple.com>
     2
     3        Cherry-pick r245018. rdar://problem/51264878
     4
     5    tryCachePutByID should not crash if target offset changes
     6    https://bugs.webkit.org/show_bug.cgi?id=197311
     7    <rdar://problem/48033612>
     8   
     9    Reviewed by Filip Pizlo.
     10   
     11    JSTests:
     12   
     13    Add a series of tests related tryCachePutByID. Two of these tests used to crash and were fixed
     14    by this patch: `cache-put-by-id-different-attributes.js` and `cache-put-by-id-different-offset.js`
     15   
     16    * stress/cache-put-by-id-delete-prototype.js: Added.
     17    (A.prototype.set y):
     18    (A):
     19    (B.prototype.set y):
     20    (B):
     21    (C):
     22    * stress/cache-put-by-id-different-__proto__.js: Added.
     23    (A.prototype.set y):
     24    (A):
     25    (B1):
     26    (B2.prototype.set y):
     27    (B2):
     28    (C):
     29    (D):
     30    * stress/cache-put-by-id-different-attributes.js: Added.
     31    (Foo):
     32    (set x):
     33    * stress/cache-put-by-id-different-offset.js: Added.
     34    (Foo):
     35    (set x):
     36    * stress/cache-put-by-id-insert-prototype.js: Added.
     37    (A.prototype.set y):
     38    (A):
     39    (C):
     40    * stress/cache-put-by-id-poly-proto.js: Added.
     41    (Foo):
     42    (set _):
     43    (createBar.Bar):
     44    (createBar):
     45   
     46    Source/JavaScriptCore:
     47   
     48    When tryCachePutID is called with a cacheable setter, if the target object where the setter was
     49    found is still in the prototype chain and there's no poly protos in the chain, we use
     50    generateConditionsForPrototypePropertyHit to validate that the target object remains the same.
     51    It checks for the absence of the property in every object in the prototype chain from the base
     52    down to the target object and checks that the property is still present in the target object. It
     53    also bails if there are any uncacheable objects, proxies or dictionary objects in the prototype
     54    chain. However, it does not consider two edge cases:
     55    - It asserts that the property should still be at the same offset in the target object, but this
     56    assertion does not hold if the setter deletes properties of the object and causes the structure
     57    to be flattened after the deletion. Instead of asserting, we just use the updated offset.
     58    - It does not check whether the new slot is also a setter, which leads to a crash in case it's not.
     59   
     60    * jit/Repatch.cpp:
     61    (JSC::tryCachePutByID):
     62   
     63   
     64    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@245018 268f45cc-cd09-0410-ab3c-d52691b4dbfc
     65
     66    2019-05-07  Tadeu Zagallo  <tzagallo@apple.com>
     67
     68            tryCachePutByID should not crash if target offset changes
     69            https://bugs.webkit.org/show_bug.cgi?id=197311
     70            <rdar://problem/48033612>
     71
     72            Reviewed by Filip Pizlo.
     73
     74            When tryCachePutID is called with a cacheable setter, if the target object where the setter was
     75            found is still in the prototype chain and there's no poly protos in the chain, we use
     76            generateConditionsForPrototypePropertyHit to validate that the target object remains the same.
     77            It checks for the absence of the property in every object in the prototype chain from the base
     78            down to the target object and checks that the property is still present in the target object. It
     79            also bails if there are any uncacheable objects, proxies or dictionary objects in the prototype
     80            chain. However, it does not consider two edge cases:
     81            - It asserts that the property should still be at the same offset in the target object, but this
     82            assertion does not hold if the setter deletes properties of the object and causes the structure
     83            to be flattened after the deletion. Instead of asserting, we just use the updated offset.
     84            - It does not check whether the new slot is also a setter, which leads to a crash in case it's not.
     85
     86            * jit/Repatch.cpp:
     87            (JSC::tryCachePutByID):
     88
    1892019-05-30  Kocsen Chung  <kocsen_chung@apple.com>
    290

  • branches/safari-607-branch/Source/JavaScriptCore/jit/Repatch.cpp

    r240375 r245918  
    576576                            return GiveUpOnCache;
    577577
    578                         PropertyOffset conditionSetOffset = conditionSet.slotBaseCondition().offset();
    579                         if (UNLIKELY(offset != conditionSetOffset))
    580                             CRASH_WITH_INFO(offset, conditionSetOffset, slot.base()->type(), baseCell->type(), conditionSet.size());
     578                        if (!(conditionSet.slotBaseCondition().attributes() & PropertyAttribute::Accessor))
     579                            return GiveUpOnCache;
     580
     581                        offset = conditionSet.slotBaseCondition().offset();
    581582                    }
    582583
Note: See TracChangeset for help on using the changeset viewer.