qiurui
/
qtdeclarative
mirror of https://github.com/qt/qtdeclarative.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Fix delayed loading of arguments in binary expressions 

Consider the following functions:
    function f(x) {
        return x + (++x)
    }

    function g(x) {
        return x + x
    }

In f() it is not correct to delay the load of x on the left-hand side of
the + operator, while in g() it is. The reason is that calculating the
right-hand side of the + operator in f() will change the value of x.

So, if an argument is written to in an expression in a statement, it
cannot be delay-loaded. The same is true for member/field accesses,
because the accessors can be overwritten and do anything.

Change-Id: I5bed4b0d03919edc1c94a82127e2dd705fc1d9b1
Reviewed-by: Lars Knoll <lars.knoll@qt.io>
Reviewed-by: Simon Hausmann <simon.hausmann@qt.io>
This commit is contained in:
Erik Verbruggen 2017-09-19 14:22:18 +02:00
parent 3d076faeea
commit 7372758263
3 changed files with 136 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

110
src/qml/compiler/qv4codegen.cpp
View File

@ -287,7 +287,11 @@ void Codegen::statement(Statement *ast)
RegisterScope scope(this);
bytecodeGenerator->setLocation(ast->firstSourceLocation());
VolatileMemoryLocations vLocs = scanVolatileMemoryLocations(ast);
qSwap(_volataleMemoryLocations, vLocs);
accept(ast);
qSwap(_volataleMemoryLocations, vLocs);
}
void Codegen::statement(ExpressionNode *ast)
@ -299,10 +303,16 @@ void Codegen::statement(ExpressionNode *ast)
} else {
Result r(nx);
qSwap(_expr, r);
VolatileMemoryLocations vLocs = scanVolatileMemoryLocations(ast);
qSwap(_volataleMemoryLocations, vLocs);
accept(ast);
qSwap(_volataleMemoryLocations, vLocs);
qSwap(_expr, r);
if (hasError)
return;
qSwap(_expr, r);
if (r.result().loadTriggersSideEffect())
r.result().loadInAccumulator(); // triggers side effects
}
@ -1482,7 +1492,7 @@ Codegen::Reference Codegen::referenceForName(const QString &name, bool isLhs)
const int argIdx = c->findArgument(name);
if (argIdx != -1) {
Q_ASSERT(!c->argumentsCanEscape && c->usesArgumentsObject != Context::ArgumentsObjectUsed);
return Reference::fromArgument(this, argIdx);
return Reference::fromArgument(this, argIdx, _volataleMemoryLocations.isVolatile(name));
}
c = c->parent;
}
@ -1510,7 +1520,7 @@ Codegen::Reference Codegen::referenceForName(const QString &name, bool isLhs)
return Reference::fromScopedLocal(this, idx, scope);
} else {
Q_ASSERT(scope == 0);
return Reference::fromArgument(this, argIdx);
return Reference::fromArgument(this, argIdx, _volataleMemoryLocations.isVolatile(name));
}
}
@ -2848,6 +2858,97 @@ QQmlRefPointer<CompiledData::CompilationUnit> Codegen::createUnitForLoading()
return result;
}
class Codegen::VolatileMemoryLocationScanner: protected QQmlJS::AST::Visitor
{
VolatileMemoryLocations locs;
public:
Codegen::VolatileMemoryLocations scan(AST::Node *s)
{
s->accept(this);
return locs;
}
bool visit(ArrayMemberExpression *) Q_DECL_OVERRIDE
{
locs.setAllVolatile();
return false;
}
bool visit(FieldMemberExpression *) Q_DECL_OVERRIDE
{
locs.setAllVolatile();
return false;
}
bool visit(PostIncrementExpression *e) Q_DECL_OVERRIDE
{
collectIdentifiers(locs.specificLocations, e->base);
return false;
}
bool visit(PostDecrementExpression *e) Q_DECL_OVERRIDE
{
collectIdentifiers(locs.specificLocations, e->base);
return false;
}
bool visit(PreIncrementExpression *e) Q_DECL_OVERRIDE
{
collectIdentifiers(locs.specificLocations, e->expression);
return false;
}
bool visit(PreDecrementExpression *e) Q_DECL_OVERRIDE
{
collectIdentifiers(locs.specificLocations, e->expression);
return false;
}
bool visit(BinaryExpression *e) Q_DECL_OVERRIDE
{
switch (e->op) {
case QSOperator::InplaceAnd:
case QSOperator::InplaceSub:
case QSOperator::InplaceDiv:
case QSOperator::InplaceAdd:
case QSOperator::InplaceLeftShift:
case QSOperator::InplaceMod:
case QSOperator::InplaceMul:
case QSOperator::InplaceOr:
case QSOperator::InplaceRightShift:
case QSOperator::InplaceURightShift:
case QSOperator::InplaceXor:
collectIdentifiers(locs.specificLocations, e);
return false;
default:
return true;
}
}
private:
void collectIdentifiers(QVector<QStringView> &ids, AST::Node *node) const {
class Collector: public QQmlJS::AST::Visitor {
QVector<QStringView> &ids;
public:
Collector(QVector<QStringView> &ids): ids(ids) {}
virtual bool visit(IdentifierExpression *ie) {
ids.append(ie->name);
return false;
}
};
Collector collector(ids);
node->accept(&collector);
}
};
Codegen::VolatileMemoryLocations Codegen::scanVolatileMemoryLocations(AST::Node *ast) const
{
VolatileMemoryLocationScanner scanner;
return scanner.scan(ast);
}
#ifndef V4_BOOTSTRAP
@ -2954,6 +3055,7 @@ Codegen::Reference &Codegen::Reference::operator =(const Reference &other)
codegen = other.codegen;
isReadonly = other.isReadonly;
stackSlotIsLocalOrArgument = other.stackSlotIsLocalOrArgument;
isVolatile = other.isVolatile;
global = other.global;
return *this;
}
@ -3044,7 +3146,7 @@ void Codegen::Reference::storeOnStack(int slotIndex) const
Codegen::Reference Codegen::Reference::doStoreOnStack(int slotIndex) const
{
if (isStackSlot() && slotIndex == -1)
if (isStackSlot() && slotIndex == -1 && !(stackSlotIsLocalOrArgument && isVolatile))
return *this;
if (isStackSlot()) { // temp-to-temp move

23
src/qml/compiler/qv4codegen_p.h
View File

@ -103,6 +103,21 @@ public:
CompilationMode mode = GlobalCode);
public:
class VolatileMemoryLocationScanner;
class VolatileMemoryLocations {
friend VolatileMemoryLocationScanner;
bool allVolatile = false;
QVector<QStringView> specificLocations;
public:
bool isVolatile(const QStringView &name) {
if (allVolatile)
return true;
return specificLocations.contains(name);
}
void add(const QStringRef &name) { if (!allVolatile) specificLocations.append(name); }
void setAllVolatile() { allVolatile = true; }
};
class RValue {
Codegen *codegen;
enum Type {
@ -215,10 +230,11 @@ public:
r.stackSlotIsLocalOrArgument = isLocal;
return r;
}
static Reference fromArgument(Codegen *cg, int index) {
static Reference fromArgument(Codegen *cg, int index, bool isVolatile) {
Reference r(cg, StackSlot);
r.theStackSlot = Moth::StackSlot::createRegister(index + sizeof(CallData)/sizeof(Value) - 1);
r.stackSlotIsLocalOrArgument = true;
r.isVolatile = isVolatile;
return r;
}
static Reference fromScopedLocal(Codegen *cg, int index, int scope) {
@ -327,6 +343,7 @@ public:
mutable bool isArgOrEval = false;
bool isReadonly = false;
bool stackSlotIsLocalOrArgument = false;
bool isVolatile = false;
bool global = false;
Codegen *codegen;
@ -625,6 +642,7 @@ protected:
friend struct ControlFlowCatch;
friend struct ControlFlowFinally;
Result _expr;
VolatileMemoryLocations _volataleMemoryLocations;
Module *_module;
int _returnAddress;
Context *_context;
@ -638,6 +656,9 @@ protected:
bool _fileNameIsUrl;
bool hasError;
QList<QQmlJS::DiagnosticMessage> _errors;
private:
VolatileMemoryLocations scanVolatileMemoryLocations(AST::Node *ast) const;
};
}

8
tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp
View File

@ -343,6 +343,7 @@ private slots:
void freeze_empty_object();
void singleBlockLoops();
void qtbug_60547();
void delayLoadingArgs();
private:
// static void propertyVarWeakRefCallback(v8::Persistent<v8::Value> object, void* parameter);
@ -8371,6 +8372,13 @@ void tst_qqmlecmascript::qtbug_60547()
QCOMPARE(object->property("counter"), QVariant(int(1)));
}
void tst_qqmlecmascript::delayLoadingArgs()
{
QJSEngine engine;
QJSValue ret = engine.evaluate("(function(x){return x + (x+=2)})(20)");
QCOMPARE(ret.toInt(), 42); // esp. not 44.
}
QTEST_MAIN(tst_qqmlecmascript)
#include "tst_qqmlecmascript.moc"