D12743: Unit tests fixes

[Gleb Popov]

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'm far from being a C++ expert, maybe there is some subtle UB?

I've fired up IDA Pro and here is decompiled code of patched version:

  KPluginMetaData::rawData((KPluginMetaData *)&pluginInfoThis);
  QString::QString(&string_KPlugin, "KPlugin");
  LODWORD(v10) = QJsonObject::operator[](&pluginInfoThis, &string_KPlugin);
  v25 = v10;
  v26 = v11;
  QJsonValueRef::toObject((QJsonValueRef *)&v27);
  QString::~QString((QString *)&string_KPlugin);
  QJsonObject::~QJsonObject((QJsonObject *)&pluginInfoThis);
  QString::QString(&string_EnabledByDef, "EnabledByDefault");
  LODWORD(v12) = QJsonObject::operator[](&v27, &string_EnabledByDef);
  v21 = v12;
  v22 = v13;
  QString::~QString((QString *)&string_EnabledByDef);
  v16 = 1;
  if ( !(QJsonValueRef::isNull((QJsonValueRef *)&v21) & 1) )

Note that before `QJsonValueRef::isNull` is called, 3 destructors are run:

  QString::~QString((QString *)&string_KPlugin);
  QJsonObject::~QJsonObject((QJsonObject *)&pluginInfoThis);
  QString::~QString((QString *)&string_EnabledByDef);

And here is decompiled code of the current version:

  KPluginMetaData::rawData((KPluginMetaData *)&pluginInfoThis);
  QString::QString(&string_KPlugin, "KPlugin");
  LODWORD(v10) = QJsonObject::operator[](&pluginInfoThis, &string_KPlugin);
  v23 = v10;
  v24 = v11;
  QJsonValueRef::toObject((QJsonValueRef *)&v25);
  QString::QString(&string_EnabledByDef, "EnabledByDefault");
  LODWORD(v12) = QJsonObject::operator[](&v25, &string_EnabledByDef);
  v26 = v12;
  v27 = v13;
  QString::~QString((QString *)&string_EnabledByDef);
  QJsonObject::~QJsonObject((QJsonObject *)&v25);
  QString::~QString((QString *)&string_KPlugin);
  QJsonObject::~QJsonObject((QJsonObject *)&pluginInfoThis);
  v16 = 1;
  if ( !(QJsonValueRef::isNull((QJsonValueRef *)&v26) & 1) )

There 4 destructors are run - 3 from above and additional `QJsonObject::~QJsonObject((QJsonObject *)&v25);`, which is an object that `QJsonObject::operator[](&v25, &string_EnabledByDef);` operates on. I suspect this is what causes the problem.

There also might be miscompilation on the clang side. I'll try to use 5.0 instead of 6.0.

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/20180508/41bf4986/attachment.html>