diff options
author | Simon Hausmann <simon.hausmann@qt.io> | 2018-07-06 14:34:27 +0200 |
---|---|---|
committer | Simon Hausmann <simon.hausmann@qt.io> | 2018-07-10 09:08:56 +0000 |
commit | 3dd77c4c1dabc757f6a441dafe7b27e32719c994 (patch) | |
tree | 8b5ecb91c5dc7ff6e2127ba19d79c4de6c8e4459 /src/qml/jsruntime/qv4identifiertable_p.h | |
parent | 1771d298f33543a3fe47decfe0fff10609b01ab1 (diff) |
Fix invalid object property key conversions
Different property names are mapped to the same property key through a
the identifier hash table. In the case of QTBUG-69280 we map a for-in
loop "for (var prop in testCase)" in TestCase.qml to an internal
iterator object, which signals whether it's finished or not by setting a
"done" property on the iterator result object to a boolean. On the
setter side, the property is specified via
result->put(engine->newString("done"))
and on the getter side the code uses
result->get(engine->id_done())
For this to work, the newly created string has to map to the same
identifier, otherwise we end up with differing property keys and wrong
values.
The test failure of QTBUG-69280 reproduced a scenario where two strings,
"pathChanged" and "done", mapped to the same index in the hash table
after a rehashing (growing), despite different string hash values. As a
consequence of the hash collision they had adjacent entries in the hash
table, with "pathChanged" coming first.
A subsequent garbage collection run ended up with "pathChanged" being
not marked and subject to removal from the identifier table.
IdentifierTable::sweep() wiped the entry for "pathChanged" and lastEntry
to the now free index and also remembered the exact string hash value.
In the next iteration, sweep() looked at the entry for "done", which
should move to the now free slot, as both strings map to the same index.
However sweep() didn't do that because the comparison of string hash
values failed.
The fix is to compare table indices (covered by
sweepFirstEntryInSameBucketWithDifferingHash) and respect bucket boundaries
(covered by dontSweepAcrossBucketBoundaries).
However it may happen that entries that would map to the same bucket end
up after another bucket because of the insertion order. This would lead
to
Q_ASSERT(table[lastIdx] == nullptr);
failing right after
lastIdx = (lastIdx + 1) % alloc;
Instead the determination of the next free slot must follow the same
logic as in addEntry, by finding the first null entry. This is covered
by sweepAcrossBucketBoundariesIfFirstBucketFull.
Task-number: QTBUG-69280
Change-Id: I284f53418d0a75e2edb631f8bacca8c5a596e603
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
Reviewed-by: Ulf Hermann <ulf.hermann@qt.io>
Diffstat (limited to 'src/qml/jsruntime/qv4identifiertable_p.h')
-rw-r--r-- | src/qml/jsruntime/qv4identifiertable_p.h | 8 |
1 files changed, 4 insertions, 4 deletions
diff --git a/src/qml/jsruntime/qv4identifiertable_p.h b/src/qml/jsruntime/qv4identifiertable_p.h index 4f1656dddf..3674ece84f 100644 --- a/src/qml/jsruntime/qv4identifiertable_p.h +++ b/src/qml/jsruntime/qv4identifiertable_p.h @@ -60,7 +60,7 @@ QT_BEGIN_NAMESPACE namespace QV4 { -struct IdentifierTable +struct Q_QML_PRIVATE_EXPORT IdentifierTable { ExecutionEngine *engine; @@ -76,7 +76,7 @@ struct IdentifierTable public: - IdentifierTable(ExecutionEngine *engine); + IdentifierTable(ExecutionEngine *engine, int numBits = 8); ~IdentifierTable(); Heap::String *insertString(const QString &s); @@ -97,8 +97,8 @@ public: PropertyKey asPropertyKeyImpl(const Heap::String *str); Heap::StringOrSymbol *resolveId(PropertyKey i) const; - Q_QML_PRIVATE_EXPORT Heap::String *stringForId(PropertyKey i) const; - Q_QML_PRIVATE_EXPORT Heap::Symbol *symbolForId(PropertyKey i) const; + Heap::String *stringForId(PropertyKey i) const; + Heap::Symbol *symbolForId(PropertyKey i) const; void markObjects(MarkStack *markStack); void sweep(); |