Disable class-memaccess compiler warnings where not wanted
ClosedPublic

Authored by kossebau on Jul 5 2018, 10:06 AM.

Details

Summary

The given code does those exceptions by design.

Diff Detail

Repository
R32 KDevelop
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
kossebau created this revision.Jul 5 2018, 10:06 AM
Restricted Application added a project: KDevelop. · View Herald TranscriptJul 5 2018, 10:06 AM
Restricted Application added a subscriber: kdevelop-devel. · View Herald Transcript
kossebau requested review of this revision.Jul 5 2018, 10:06 AM

Target branch: 5.2

I have got those warnings with gcc version 8.1.1 now, and they messed the build log to some degree :) No idea if something similar is needed for other compilers?
Also no idea if this needs some more version check or if this is proper backward compatible with older gcc?

kfunk added a subscriber: kfunk.Jul 6 2018, 7:53 AM

What was the exact warning message to begin with? Can't this be fixed differently (i.e. without using the PP)?

kossebau added a comment.EditedJul 6 2018, 2:03 PM

What was the exact warning message to begin with? Can't this be fixed differently (i.e. without using the PP)?

Find below a sample warning message per each entry.

I have no deep understanding of the actual code, all seem more elaborated low-level memory magic to me, which though has existed for years without (known?) issues, so my motivation to try to rewrite it using proper (modern?) C++ expressions is rather low :) Perhaps worth a comment, but then I have no idea what to write besides "Dear clever code reader, please consider to rewrite this code using normal C++ class code" which though is IMHO an implicit with most/any given diagnostic pragmas ;) But feel challenged to be the clever code reader now :P I know it's just me to keep fingers from keyboard here.

And the code from the affected headers is used quite often, which messes up the build log with the warnings quite a lot. Thus a bigger motivation to just silence those warnings down, given no-one might be able to do something soon, so it's useless log pain making one miss other more real issues.

[ 30%] Building CXX object kdevplatform/language/CMakeFiles/KDevPlatformLanguage.dir/duchain/topducontext.cpp.o
In file included from /home/koder/Kode/kdegit/kf5/extragear/kdevelop/kdevelop/kdevplatform/language/util/basicsetrepository.h:21,
                 from /home/koder/Kode/kdegit/kf5/extragear/kdevelop/kdevelop/kdevplatform/language/util/setrepository.h:17,
                 from /home/koder/Kode/kdegit/kf5/extragear/kdevelop/kdevelop/kdevplatform/language/duchain/topducontext.h:24,
                 from /home/koder/Kode/kdegit/kf5/extragear/kdevelop/kdevelop/kdevplatform/language/duchain/topducontext.cpp:20:
/home/koder/Kode/kdegit/kf5/extragear/kdevelop/kdevelop/kdevplatform/serialization/itemrepository.h: In instantiation of ‘void KDevelop::Bucket<Item, ItemRequest, markForReferenceCounting, fixedItemSize>::deleteItem(short unsigned int, unsigned int, Repository&) [with Repository = KDevelop::ItemRepository<Utils::SetNodeData, Utils::SetNodeDataRequest, false, false, 24>; Item = Utils::SetNodeData; ItemRequest = Utils::SetNodeDataRequest; bool markForReferenceCounting = false; unsigned int fixedItemSize = 24]’:
/home/koder/Kode/kdegit/kf5/extragear/kdevelop/kdevelop/kdevplatform/serialization/itemrepository.h:675:15:   required from ‘int KDevelop::Bucket<Item, ItemRequest, markForReferenceCounting, fixedItemSize>::finalCleanup(Repository&) [with Repository = KDevelop::ItemRepository<Utils::SetNodeData, Utils::SetNodeDataRequest, false, false, 24>; Item = Utils::SetNodeData; ItemRequest = Utils::SetNodeDataRequest; bool markForReferenceCounting = false; unsigned int fixedItemSize = 24]’
/home/koder/Kode/kdegit/kf5/extragear/kdevelop/kdevelop/kdevplatform/serialization/itemrepository.h:2070:17:   required from ‘int KDevelop::ItemRepository<Item, ItemRequest, markForReferenceCounting, threadSafe, fixedItemSize, targetBucketHashSize>::finalCleanup() [with Item = Utils::SetNodeData; ItemRequest = Utils::SetNodeDataRequest; bool markForReferenceCounting = false; bool threadSafe = false; unsigned int fixedItemSize = 24; unsigned int targetBucketHashSize = 1048576]’
/home/koder/Kode/kdegit/kf5/extragear/kdevelop/kdevelop/kdevplatform/serialization/itemrepository.h:2063:7:   required from here
/home/koder/Kode/kdegit/kf5/extragear/kdevelop/kdevelop/kdevplatform/serialization/itemrepository.h:564:13: warning: ‘void* memset(void*, int, size_t)’ clearing an object of non-trivial type ‘struct Utils::SetNodeData’; use assignment or value-initialization instead [-Wclass-memaccess]
       memset(item, 0, size); //For debugging, so we notice the data is wrong
       ~~~~~~^~~~~~~~~~~~~~~
In file included from /home/koder/Kode/kdegit/kf5/extragear/kdevelop/kdevelop/kdevplatform/language/util/setrepository.h:17,
                 from /home/koder/Kode/kdegit/kf5/extragear/kdevelop/kdevelop/kdevplatform/language/duchain/topducontext.h:24,
                 from /home/koder/Kode/kdegit/kf5/extragear/kdevelop/kdevelop/kdevplatform/language/duchain/topducontext.cpp:20:
/home/koder/Kode/kdegit/kf5/extragear/kdevelop/kdevelop/kdevplatform/language/util/basicsetrepository.h:61:36: note: ‘struct Utils::SetNodeData’ declared here
 struct KDEVPLATFORMLANGUAGE_EXPORT SetNodeData {
                                    ^~~~~~~~~~~
[ 30%] Building CXX object kdevplatform/language/CMakeFiles/KDevPlatformLanguage.dir/duchain/topducontextdynamicdata.cpp.o
/home/koder/Kode/kdegit/kf5/extragear/kdevelop/kdevelop/kdevplatform/language/duchain/topducontextdynamicdata.cpp: In function ‘void {anonymous}::saveDUChainItem(QVector<KDevelop::TopDUContextDynamicData::ArrayWithPosition>&, KDevelop::DUChainBase&, uint&, bool)’:
/home/koder/Kode/kdegit/kf5/extragear/kdevelop/kdevelop/kdevplatform/language/duchain/topducontextdynamicdata.cpp:93:40: warning: ‘void* memcpy(void*, const void*, size_t)’ writing to an object of non-trivially copyable type ‘class KDevelop::DUChainBaseData’; use copy-assignment or copy-initialization instead [-Wclass-memaccess]
     memcpy(&target, item.d_func(), size);
                                        ^
In file included from /home/koder/Kode/kdegit/kf5/extragear/kdevelop/kdevelop/kdevplatform/language/duchain/problem.h:28,
                 from /home/koder/Kode/kdegit/kf5/extragear/kdevelop/kdevelop/kdevplatform/language/duchain/topducontextdynamicdata.h:24,
                 from /home/koder/Kode/kdegit/kf5/extragear/kdevelop/kdevelop/kdevplatform/language/duchain/topducontextdynamicdata.cpp:21:
/home/koder/Kode/kdegit/kf5/extragear/kdevelop/kdevelop/kdevplatform/language/duchain/duchainbase.h:59:35: note: ‘class KDevelop::DUChainBaseData’ declared here
 class KDEVPLATFORMLANGUAGE_EXPORT DUChainBaseData {
                                   ^~~~~~~~~~~~~~~
/home/koder/Kode/kdegit/kf5/extragear/kdevelop/kdevelop/kdevplatform/language/duchain/topducontextdynamicdata.cpp: In member function ‘KDevelop::TopDUContextDynamicData::ItemDataInfo KDevelop::TopDUContextDynamicData::writeDataInfo(const KDevelop::TopDUContextDynamicData::ItemDataInfo&, const KDevelop::DUChainBaseData*, uint&)’:
/home/koder/Kode/kdegit/kf5/extragear/kdevelop/kdevelop/kdevplatform/language/duchain/topducontextdynamicdata.cpp:768:28: warning: ‘void* memcpy(void*, const void*, size_t)’ writing to an object of non-trivially copyable type ‘class KDevelop::DUChainBaseData’; use copy-assignment or copy-initialization instead [-Wclass-memaccess]
   memcpy(target, data, size);
                            ^
In file included from /home/koder/Kode/kdegit/kf5/extragear/kdevelop/kdevelop/kdevplatform/language/duchain/problem.h:28,
                 from /home/koder/Kode/kdegit/kf5/extragear/kdevelop/kdevelop/kdevplatform/language/duchain/topducontextdynamicdata.h:24,
                 from /home/koder/Kode/kdegit/kf5/extragear/kdevelop/kdevelop/kdevplatform/language/duchain/topducontextdynamicdata.cpp:21:
/home/koder/Kode/kdegit/kf5/extragear/kdevelop/kdevelop/kdevplatform/language/duchain/duchainbase.h:59:35: note: ‘class KDevelop::DUChainBaseData’ declared here
 class KDEVPLATFORMLANGUAGE_EXPORT DUChainBaseData {
                                   ^~~~~~~~~~~~~~~
In file included from /home/koder/Kode/kdegit/kf5/extragear/kdevelop/kdevelop/kdevplatform/language/duchain/codemodel.cpp:27:
/home/koder/Kode/kdegit/kf5/extragear/kdevelop/kdevelop/kdevplatform/util/embeddedfreetree.h: In instantiation of ‘bool KDevelop::EmbeddedTreeAddItem<Data, ItemHandler, increaseFraction, rebuildIfInsertionMoreExpensive>::insertSorted(const Data&, int, int, int, bool) [with Data = KDevelop::CodeModelItem; ItemHandler = KDevelop::CodeModelItemHandler; int increaseFraction = 5; int rebuildIfInsertionMoreExpensive = 20]’:
/home/koder/Kode/kdegit/kf5/extragear/kdevelop/kdevelop/kdevplatform/util/embeddedfreetree.h:505:22:   required from ‘bool KDevelop::EmbeddedTreeAddItem<Data, ItemHandler, increaseFraction, rebuildIfInsertionMoreExpensive>::apply(bool) [with Data = KDevelop::CodeModelItem; ItemHandler = KDevelop::CodeModelItemHandler; int increaseFraction = 5; int rebuildIfInsertionMoreExpensive = 20]’
/home/koder/Kode/kdegit/kf5/extragear/kdevelop/kdevelop/kdevplatform/util/embeddedfreetree.h:248:33:   required from ‘KDevelop::EmbeddedTreeAddItem<Data, ItemHandler, increaseFraction, rebuildIfInsertionMoreExpensive>::EmbeddedTreeAddItem(Data*, uint, int&, const Data&) [with Data = KDevelop::CodeModelItem; ItemHandler = KDevelop::CodeModelItemHandler; int increaseFraction = 5; int rebuildIfInsertionMoreExpensive = 20; uint = unsigned int]’
/home/koder/Kode/kdegit/kf5/extragear/kdevelop/kdevelop/kdevplatform/language/duchain/codemodel.cpp:201:140:   required from here
/home/koder/Kode/kdegit/kf5/extragear/kdevelop/kdevelop/kdevplatform/util/embeddedfreetree.h:700:28: warning: ‘void* memmove(void*, const void*, size_t)’ writing to an object of type ‘struct KDevelop::CodeModelItem’ with no trivial copy-assignment; use copy-assignment or copy-initialization instead [-Wclass-memaccess]
                     memmove(m_items+bound+1, m_items+bound, sizeof(Data)*(pos-bound));
                     ~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from /home/koder/Kode/kdegit/kf5/extragear/kdevelop/kdevelop/kdevplatform/language/duchain/codemodel.cpp:19:
/home/koder/Kode/kdegit/kf5/extragear/kdevelop/kdevelop/kdevplatform/language/duchain/codemodel.h:35:10: note: ‘struct KDevelop::CodeModelItem’ declared here
   struct CodeModelItem
          ^~~~~~~~~~~~~
In file included from /home/koder/Kode/kdegit/kf5/extragear/kdevelop/kdevelop/kdevplatform/language/duchain/codemodel.cpp:27:
/home/koder/Kode/kdegit/kf5/extragear/kdevelop/kdevelop/kdevplatform/util/embeddedfreetree.h:709:28: warning: ‘void* memmove(void*, const void*, size_t)’ writing to an object of type ‘struct KDevelop::CodeModelItem’ with no trivial copy-assignment; use copy-assignment or copy-initialization instead [-Wclass-memaccess]
                     memmove(m_items+pos, m_items+pos+1, sizeof(Data)*(bound-pos-1));
                     ~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from /home/koder/Kode/kdegit/kf5/extragear/kdevelop/kdevelop/kdevplatform/language/duchain/codemodel.cpp:19:
/home/koder/Kode/kdegit/kf5/extragear/kdevelop/kdevelop/kdevplatform/language/duchain/codemodel.h:35:10: note: ‘struct KDevelop::CodeModelItem’ declared here
   struct CodeModelItem
          ^~~~~~~~~~~~~
In file included from /home/koder/Kode/kdegit/kf5/extragear/kdevelop/kdevelop/kdevplatform/language/duchain/codemodel.cpp:27:
/home/koder/Kode/kdegit/kf5/extragear/kdevelop/kdevelop/kdevplatform/util/embeddedfreetree.h:712:22: warning: ‘void* memcpy(void*, const void*, size_t)’ writing to an object of type ‘struct KDevelop::CodeModelItem’ with no trivial copy-assignment; use copy-assignment or copy-initialization instead [-Wclass-memaccess]
                memcpy(m_items+target, backup, sizeof(Data));
                ~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from /home/koder/Kode/kdegit/kf5/extragear/kdevelop/kdevelop/kdevplatform/language/duchain/codemodel.cpp:19:
/home/koder/Kode/kdegit/kf5/extragear/kdevelop/kdevelop/kdevplatform/language/duchain/codemodel.h:35:10: note: ‘struct KDevelop::CodeModelItem’ declared here
   struct CodeModelItem
          ^~~~~~~~~~~~~
/home/koder/Kode/kdegit/kf5/extragear/kdevelop/kdevelop/plugins/qmljs/3rdparty/qtcreator-libs/qmljs/parser/qmljsparser.cpp: In member function ‘void QmlJS::Parser::reallocateStack()’:
/home/koder/Kode/kdegit/kf5/extragear/kdevelop/kdevelop/plugins/qmljs/3rdparty/qtcreator-libs/qmljs/parser/qmljsparser.cpp:76:104: warning: ‘void* realloc(void*, size_t)’ moving an object of non-trivially copyable type ‘class QStringRef’; use ‘new’ and ‘delete’ instead [-Wclass-memaccess]
     string_stack = reinterpret_cast<QStringRef*> (realloc(string_stack, stack_size * sizeof(QStringRef)));
                                                                                                        ^
In file included from /usr/include/qt5/QtCore/qhashfunctions.h:44,
                 from /usr/include/qt5/QtCore/qlist.h:47,
                 from /usr/include/qt5/QtCore/qhash.h:46,
                 from /usr/include/qt5/QtCore/qshareddata.h:46,
                 from /home/koder/Kode/kdegit/kf5/extragear/kdevelop/kdevelop/plugins/qmljs/3rdparty/qtcreator-libs/qmljs/parser/qmljsmemorypool_p.h:48,
                 from /home/koder/Kode/kdegit/kf5/extragear/kdevelop/kdevelop/plugins/qmljs/3rdparty/qtcreator-libs/qmljs/parser/qmljsengine_p.h:47,
                 from /home/koder/Kode/kdegit/kf5/extragear/kdevelop/kdevelop/plugins/qmljs/3rdparty/qtcreator-libs/qmljs/parser/qmljsparser.cpp:31:
/usr/include/qt5/QtCore/qstring.h:1418:21: note: ‘class QStringRef’ declared here
 class Q_CORE_EXPORT QStringRef {
                     ^~~~~~~~~~

On the surface the compiler is right, this comes very close to undefined behavior. But there is so much crazy stuff happening in the DUChain that it might be fine in the end.

I have no deep understanding of the actual code, all seem more elaborated low-level memory magic to me, which though has existed for years without (known?) issues, so my motivation to try to rewrite it using proper (modern?) C++ expressions is rather low :)

Well, actually I do remember a few crashes in DUChain code. But in the few cases when I didn't have something better to do, I was immediately scared away by the code. I find it really hard to understand.

Perhaps worth a comment, but then I have no idea what to write besides "Dear clever code reader, please consider to rewrite this code using normal C++ class code" which though is IMHO an implicit with most/any given diagnostic pragmas ;) But feel challenged to be the clever code reader now :P I know it's just me to keep fingers from keyboard here.

Yes, somebody should probably rewrite that. But it's hard to know where to start.

plugins/qmljs/3rdparty/qtcreator-libs/qmljs/parser/qmljsparser.cpp
51–53 ↗(On Diff #37179)

If that is true, the change should be made somewhere else.

@kfunk Ping? Do you agree those code snippets will not see a quick normal fix, so the current warnings filling the logs serve no good?

plugins/qmljs/3rdparty/qtcreator-libs/qmljs/parser/qmljsparser.cpp
51–53 ↗(On Diff #37179)

Well spotted, had missed myself. before. Having looked more at it, this file is part of the snapshot copied over from Qt sources, and only regenerated over there during Qt development, at least so far not in the forked code copy here in KDevelop.

How to test applying this ifdef to qmljs.g I have to leave to those who know about the QML code copy.
Myself I would for now just apply to the generated code, to get rid of the rather useless warning noise hiding real issues.

kfunk accepted this revision.Jul 18 2018, 9:01 AM

Though for qtcreator-libs/ I'd probably disable the warning at the build system level, for that particular library. As we don't want to modify 3rdparty code, ideally.

This revision is now accepted and ready to land.Jul 18 2018, 9:01 AM

Though for qtcreator-libs/ I'd probably disable the warning at the build system level, for that particular library. As we don't want to modify 3rdparty code, ideally.

Striving for the ideal, I changed the patch to do that :) Also added the missing compiler version checks.

Thanks for review everyone.

This revision was automatically updated to reflect the committed changes.
antonanikin added a subscriber: antonanikin.EditedJul 19 2018, 6:24 AM

Hi, Friedrich. Sorry, but this commit completely broke my KDevelop :) Now it always crashes on start.

I think this patch is wrong, because instead of hiding compiler warnings you also disable memset() calls. Patched parts should be something like this:

#if defined(__GNUC__) && !defined(__INTEL_COMPILER) && (((__GNUC__ * 100) + __GNUC_MINOR__) >= 800)
#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Wclass-memaccess"
#endif

      memset(...);

#if defined(__GNUC__) && !defined(__INTEL_COMPILER) && (((__GNUC__ * 100) + __GNUC_MINOR__) >= 800)
#pragma GCC diagnostic pop
#endif

Hi, Friedrich. Sorry, but this commit completely broke my KDevelop :) Now it always crashes on start.

I think this patch is wrong, because instead of hiding compiler warnings you also disable memset() calls.

Ouch, indeed. Thanks for pointing out. How *** of me. Will fix in half an hour, if no-one beats me to it.

Hi, Friedrich. Sorry, but this commit completely broke my KDevelop :) Now it always crashes on start.

Should be fixed with b5a20fd7485b07ce965767c22048bff8d18e3efa, pushed to 5.2 and merged to master now. Sorry for the breakage, It's hot these days and brain is melting, does that work as excuse? :)

> does that work as excuse? :)

Hi, Friedrich. New version works well, thanks.