Memory leak in EvalInstructionSelection.

Bug #1656851 reported by Ratchanan Srirattanamet
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
qtdeclarative-opensource-src (Ubuntu)
New
Undecided
Unassigned

Bug Description

QV4::EvalInstructionSelection::compile(bool generateUnitData) in src/qml/compiler/qv4isel_p.cpp, line 369-370:
    if ((unit->data == nullptr) && (do_save || generateUnitData))
        unit->data = jsGenerator->generateUnit();
Above this line, I see that do_save is always true. So, unit->data is always generated, regardless of the value of generateUnitData.

However, by tracing to callers of this function, it's revealed that callers expect unit->data to be NULL if generateUnitData is false. And by this assumption, they overwrite this variable. As it's allocated by malloc, this chunk of memory gets lost. For example, this is a trace from running unity8-dash under Valgrind:

==4360== 601,072 bytes in 96 blocks are definitely lost in loss record 1,495 of 1,495
==4360== at 0x483E358: malloc (vg_replace_malloc.c:296)
==4360== by 0x4C3120B: QV4::Compiler::JSUnitGenerator::generateUnit(QV4::Compiler::JSUnitGenerator::GeneratorOption) (qv4compiler.cpp:221)
==4360== by 0x4C3CB8F: QV4::EvalInstructionSelection::compile(bool) (qv4isel_p.cpp:370)
==4360== by 0x4C6BE0F: QQmlTypeCompiler::compile() (qqmltypecompiler.cpp:223)
==4360== by 0x4D4AD7D: QQmlTypeData::compile() (qqmltypeloader.cpp:2359)
.... (the rest is omitted as they're not relavant.)

QQmlTypeCompiler::compile() in src/qml/compiler/qqmltypecompiler.cpp, line 201-233:
    // Compile JS binding expressions and signal handlers
    if (!document->javaScriptCompilationUnit) {
        (line 203-222 omitted)
        document->javaScriptCompilationUnit = isel->compile(/*generated unit data*/false);
    }

    // Generate QML compiled type data structures

    QmlIR::QmlUnitGenerator qmlGenerator;
    QV4::CompiledData::Unit *qmlUnit = qmlGenerator.generate(*document);

    Q_ASSERT(document->javaScriptCompilationUnit);
    // The js unit owns the data and will free the qml unit.
    document->javaScriptCompilationUnit->data = qmlUnit;

You'll see that isel->compile() is called with generateUnitData set to false. Then, document->javaScriptCompilationUnit->data is set to the value of qmlUnit. This means that the old value of document->javaScriptCompilationUnit->data is overwritten without being freed properly.

I think the solution is to free this memory at the end of EvalInstructionSelection::compile() if generateUnitData is false.

All codes come from qtdeclarative-opensource-src version 5.4.1-1ubuntu11~overlay9~1 from stable-phone-overlay.

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.