D12743: Unit tests fixes
Gleb Popov
noreply at phabricator.kde.org
Sat May 26 11:56:09 UTC 2018
arrowdodger added inline comments.
INLINE COMMENTS
> mwolff wrote in test_pluginenabling.cpp:138
> this doesn't change anything from a functionality point of view. Please use valgrind or similar to inspect the crash
I managed to come up with minimal repro testcase:
#include <QJsonObject>
QJsonObject makeObject()
{
QJsonObject ret, kplugin;
kplugin["EnabledByDefault"] = false;
ret["KPlugin"] = kplugin;
return ret;
}
int main()
{
// works
// auto p = makeObject()["KPlugin"].toObject();
// const auto enabledByDefaultValue = p["EnabledByDefault"];
// segfaults
const auto enabledByDefaultValue = makeObject()["KPlugin"].toObject()["EnabledByDefault"];
const bool enabledByDefault = enabledByDefaultValue.isNull();
return 0;
}
I debugged this code instruction-by-instruction and there is indeed use after free there. `enabledByDefaultValue` is a `QJsonValueRef` returned from the last `operator[]`. It holds a reference to QJsonObject's internal data in its `.d` and `.o` fields. After the last `operator[]` call, the compiler emits destructors for two created `QString`s as well as two created `QJsonObject`s. After the last `~QJsonObject` finishes, the reference count drop to 0, what makes internal data to be freed and `QJsonValueRef enabledByDefaultValue` to reference a freed memory. Exactly as I concluded after looking on IDA output.
I catched this because I was running development branch of FreeBSD, in which `free` doesn't only release the memory, but also fill it with the garbage.
As you requested, here is a Valgrind output for this testcase:
> [root at localhost qttst]# c++ -std=c++11 -g -I/usr/include/qt5/QtCore -I /usr/include/qt5 -fPIC -L /usr/lib64/qt5 -lQt5Core test.cpp -o fail
> [root at localhost qttst]# valgrind --track-origins=yes ./fail
> ==26895== Memcheck, a memory error detector
> ==26895== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
> ==26895== Using Valgrind-3.13.0 and LibVEX; rerun with -h for copyright info
> ==26895== Command: ./fail
> ==26895==
> ==26895== Invalid read of size 4
> ==26895== at 0x506C031: QJsonObject::valueAt(int) const (in /usr/lib64/libQt5Core.so.5.9.2)
> ==26895== by 0x506F8C6: QJsonValueRef::toValue() const (in /usr/lib64/libQt5Core.so.5.9.2)
> ==26895== by 0x40106F: QJsonValueRef::type() const (qjsonvalue.h:157)
> ==26895== by 0x4010C7: QJsonValueRef::isNull() const (qjsonvalue.h:158)
> ==26895== by 0x400F23: main (test.cpp:18)
> ==26895== Address 0xa7cd3d8 is 56 bytes inside a block of size 168 free'd
> ==26895== at 0x4C29D1D: free (vg_replace_malloc.c:530)
> ==26895== by 0x506AD20: QJsonObject::~QJsonObject() (in /usr/lib64/libQt5Core.so.5.9.2)
> ==26895== by 0x400EFF: main (test.cpp:15)
> ==26895== Block was alloc'd at
> ==26895== at 0x4C28C23: malloc (vg_replace_malloc.c:299)
> ==26895== by 0x506B359: QJsonObject::detach2(unsigned int) (in /usr/lib64/libQt5Core.so.5.9.2)
> ==26895== by 0x506B84B: QJsonObject::insert(QString const&, QJsonValue const&) (in /usr/lib64/libQt5Core.so.5.9.2)
> ==26895== by 0x506C0C6: QJsonObject::setValueAt(int, QJsonValue const&) (in /usr/lib64/libQt5Core.so.5.9.2)
> ==26895== by 0x506F8A9: QJsonValueRef::operator=(QJsonValue const&) (in /usr/lib64/libQt5Core.so.5.9.2)
> ==26895== by 0x400DB0: makeObject() (test.cpp:8)
> ==26895== by 0x400E9E: main (test.cpp:15)
> ==26895==
> ==26895== Invalid read of size 4
> ==26895== at 0x506C050: QJsonObject::valueAt(int) const (in /usr/lib64/libQt5Core.so.5.9.2)
> ==26895== by 0x506F8C6: QJsonValueRef::toValue() const (in /usr/lib64/libQt5Core.so.5.9.2)
> ==26895== by 0x40106F: QJsonValueRef::type() const (qjsonvalue.h:157)
> ==26895== by 0x4010C7: QJsonValueRef::isNull() const (qjsonvalue.h:158)
> ==26895== by 0x400F23: main (test.cpp:18)
> ==26895== Address 0xa7cd3dc is 60 bytes inside a block of size 168 free'd
> ==26895== at 0x4C29D1D: free (vg_replace_malloc.c:530)
> ==26895== by 0x506AD20: QJsonObject::~QJsonObject() (in /usr/lib64/libQt5Core.so.5.9.2)
> ==26895== by 0x400EFF: main (test.cpp:15)
> ==26895== Block was alloc'd at
> ==26895== at 0x4C28C23: malloc (vg_replace_malloc.c:299)
> ==26895== by 0x506B359: QJsonObject::detach2(unsigned int) (in /usr/lib64/libQt5Core.so.5.9.2)
> ==26895== by 0x506B84B: QJsonObject::insert(QString const&, QJsonValue const&) (in /usr/lib64/libQt5Core.so.5.9.2)
> ==26895== by 0x506C0C6: QJsonObject::setValueAt(int, QJsonValue const&) (in /usr/lib64/libQt5Core.so.5.9.2)
> ==26895== by 0x506F8A9: QJsonValueRef::operator=(QJsonValue const&) (in /usr/lib64/libQt5Core.so.5.9.2)
> ==26895== by 0x400DB0: makeObject() (test.cpp:8)
> ==26895== by 0x400E9E: main (test.cpp:15)
> ==26895==
> ==26895== Invalid read of size 4
> ==26895== at 0x506C05D: QJsonObject::valueAt(int) const (in /usr/lib64/libQt5Core.so.5.9.2)
> ==26895== by 0x506F8C6: QJsonValueRef::toValue() const (in /usr/lib64/libQt5Core.so.5.9.2)
> ==26895== by 0x40106F: QJsonValueRef::type() const (qjsonvalue.h:157)
> ==26895== by 0x4010C7: QJsonValueRef::isNull() const (qjsonvalue.h:158)
> ==26895== by 0x400F23: main (test.cpp:18)
> ==26895== Address 0xa7cd3f8 is 88 bytes inside a block of size 168 free'd
> ==26895== at 0x4C29D1D: free (vg_replace_malloc.c:530)
> ==26895== by 0x506AD20: QJsonObject::~QJsonObject() (in /usr/lib64/libQt5Core.so.5.9.2)
> ==26895== by 0x400EFF: main (test.cpp:15)
> ==26895== Block was alloc'd at
> ==26895== at 0x4C28C23: malloc (vg_replace_malloc.c:299)
> ==26895== by 0x506B359: QJsonObject::detach2(unsigned int) (in /usr/lib64/libQt5Core.so.5.9.2)
> ==26895== by 0x506B84B: QJsonObject::insert(QString const&, QJsonValue const&) (in /usr/lib64/libQt5Core.so.5.9.2)
> ==26895== by 0x506C0C6: QJsonObject::setValueAt(int, QJsonValue const&) (in /usr/lib64/libQt5Core.so.5.9.2)
> ==26895== by 0x506F8A9: QJsonValueRef::operator=(QJsonValue const&) (in /usr/lib64/libQt5Core.so.5.9.2)
> ==26895== by 0x400DB0: makeObject() (test.cpp:8)
> ==26895== by 0x400E9E: main (test.cpp:15)
> ==26895==
> ==26895== Invalid read of size 4
> ==26895== at 0x506ED88: QJsonValue::QJsonValue(QJsonPrivate::Data*, QJsonPrivate::Base*, QJsonPrivate::Value const&) (in /usr/lib64/libQt5Core.so.5.9.2)
> ==26895== by 0x506C06A: QJsonObject::valueAt(int) const (in /usr/lib64/libQt5Core.so.5.9.2)
> ==26895== by 0x506F8C6: QJsonValueRef::toValue() const (in /usr/lib64/libQt5Core.so.5.9.2)
> ==26895== by 0x40106F: QJsonValueRef::type() const (qjsonvalue.h:157)
> ==26895== by 0x4010C7: QJsonValueRef::isNull() const (qjsonvalue.h:158)
> ==26895== by 0x400F23: main (test.cpp:18)
> ==26895== Address 0xa7cd3e0 is 64 bytes inside a block of size 168 free'd
> ==26895== at 0x4C29D1D: free (vg_replace_malloc.c:530)
> ==26895== by 0x506AD20: QJsonObject::~QJsonObject() (in /usr/lib64/libQt5Core.so.5.9.2)
> ==26895== by 0x400EFF: main (test.cpp:15)
> ==26895== Block was alloc'd at
> ==26895== at 0x4C28C23: malloc (vg_replace_malloc.c:299)
> ==26895== by 0x506B359: QJsonObject::detach2(unsigned int) (in /usr/lib64/libQt5Core.so.5.9.2)
> ==26895== by 0x506B84B: QJsonObject::insert(QString const&, QJsonValue const&) (in /usr/lib64/libQt5Core.so.5.9.2)
> ==26895== by 0x506C0C6: QJsonObject::setValueAt(int, QJsonValue const&) (in /usr/lib64/libQt5Core.so.5.9.2)
> ==26895== by 0x506F8A9: QJsonValueRef::operator=(QJsonValue const&) (in /usr/lib64/libQt5Core.so.5.9.2)
> ==26895== by 0x400DB0: makeObject() (test.cpp:8)
> ==26895== by 0x400E9E: main (test.cpp:15)
> ==26895==
> ==26895==
> ==26895== HEAP SUMMARY:
> ==26895== in use at exit: 18,604 bytes in 6 blocks
> ==26895== total heap usage: 28 allocs, 22 frees, 19,735 bytes allocated
> ==26895==
> ==26895== LEAK SUMMARY:
> ==26895== definitely lost: 0 bytes in 0 blocks
> ==26895== indirectly lost: 0 bytes in 0 blocks
> ==26895== possibly lost: 0 bytes in 0 blocks
> ==26895== still reachable: 18,604 bytes in 6 blocks
> ==26895== suppressed: 0 bytes in 0 blocks
> ==26895== Rerun with --leak-check=full to see details of leaked memory
> ==26895==
> ==26895== For counts of detected and suppressed errors, rerun with: -v
> ==26895== ERROR SUMMARY: 4 errors from 4 contexts (suppressed: 0 from 0)
The main question is whose the bug is. Is it not allowed to chain `QJsonObject::operator[]` and we should fix it or it is a bug in Qt itself?
REPOSITORY
R32 KDevelop
REVISION DETAIL
https://phabricator.kde.org/D12743
To: arrowdodger, #kdevelop, mwolff
Cc: mwolff, kdevelop-devel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20180526/c8f85b4c/attachment.html>
More information about the KDevelop-devel
mailing list