Skip to content

Commit c3be364

Browse files
committed
Fix a possible loss of property in the TypePath
In a rare case TinyDictionary could lose a value. We use the sign bit to tag last value in a bucket chain. We also use `FF` for uninitialized/empty buckets. In extreme rare cases 127th pid to insert hashes into a a bucket still unused by the previous 126 values. Once 127 is tagged it becomes `FF` which in will look like an empty bucket - the 127 value would be lost. The fix is simple - detect the case as described and make 127 to chain to `FF` marker. In such case it will not be the last in the chain and will not be tagged. OS:#17745531
1 parent 2d76ace commit c3be364

File tree

3 files changed

+54
-2
lines changed

3 files changed

+54
-2
lines changed

lib/Runtime/Types/TypePath.h

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,12 @@ namespace Js
4343
uint32 bucketIndex = ReduceKeyToIndex(key);
4444

4545
byte i = buckets[bucketIndex];
46-
if (i == NIL)
46+
47+
// if the bucket was empty (NIL), put a tagged value to indicate the end of the chain.
48+
// NB: [OS:17745531] In extreme rare cases 127th pid to insert hashes into a a bucket still unused by the previous 126 values.
49+
// We cannot tag 127 since it would become NIL and we would lose the value.
50+
// We can however chain 127 --> NIL, since it is the same as having two 127 in the bucket, which is ok.
51+
if ((i == NIL) & (value != 127))
4752
{
4853
// set the highest bit to mark the value as the last in the chain
4954
buckets[bucketIndex] = value | 128;

test/Bugs/OS_17745531.js

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
2+
var obj = {};
3+
4+
// create enough pids to pick from
5+
for (let i = 0; i < 20000; i++)
6+
{
7+
Object.defineProperty(obj, 'prop' + i, {
8+
value: i,
9+
writable: true
10+
});
11+
}
12+
13+
for (let j = 0; j < 127; j++)
14+
{
15+
16+
var obj1 = {};
17+
// fill with pids that prone to collisions - to have some empty buckets when inserting 127th property
18+
for (let i = 0; i < 127; i++)
19+
{
20+
Object.defineProperty(obj1, 'prop' + i * 144, {
21+
value: i,
22+
writable: true
23+
});
24+
}
25+
26+
// hopefully 'prop<j>' will hash into an empty bucket and it is also a 127th property.
27+
// we will try multiple j - just in case the empty bucket moves due to minor changes in
28+
// hashing or how pids are assigned.
29+
Object.defineProperty(obj1, 'prop' + j, {
30+
value: obj['prop' + j],
31+
writable: true
32+
});
33+
34+
35+
// compare the values, also keeps objects alive
36+
if (obj['prop' + j] != obj1['prop' + j])
37+
{
38+
console.log("fail");
39+
}
40+
}
41+
42+
console.log("pass");

test/Bugs/rlexe.xml

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -505,5 +505,10 @@
505505
<default>
506506
<files>bug_OS17614914.js</files>
507507
</default>
508-
</test>
508+
</test>
509+
<test>
510+
<default>
511+
<files>OS_17745531.js</files>
512+
</default>
513+
</test>
509514
</regress-exe>

0 commit comments

Comments
 (0)