Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ostinato.git: mld.cpp:(.text+0x901): undefined reference to `unsigned long qbswap<unsigned long>(unsigned long) #265

Closed
a17r opened this issue May 24, 2018 · 14 comments

Comments

@a17r
Copy link
Contributor

commented May 24, 2018

git master does not build for me at this point, full build.log:

https://gist.github.com/a17r/47c7e19db125400fce6affab7019f457#file-ostinato-git-build-log-L394

@pstavirs

This comment has been minimized.

Copy link
Owner

commented May 30, 2018

@a17r What OS/Distro/Version are you running? What is the Qt version?

@a17r

This comment has been minimized.

Copy link
Contributor Author

commented May 31, 2018

This is Gentoo Linux with Qt 5.11.0. Other dependencies are protobuf-3.5.2 and libpcap-1.8.1-r2.

@pstavirs

This comment has been minimized.

Copy link
Owner

commented Jun 1, 2018

See if adding a #include <qendian.h> to mld.cpp helps.

Is this happening with 32-bit compilation also?

@a17r

This comment has been minimized.

Copy link
Contributor Author

commented Jun 10, 2018

Unfortunately no change...

@paul-blankenbaker

This comment has been minimized.

Copy link

commented Jun 18, 2018

I also saw this issue when attempting to use the latest ostinato commit (9a4e7e7) on a x86_64 Fedora 28 machine with Qt 5.10.1, protobuf 3.5.0 and libpcap 1.8.1.

I was able to work around it by adding the following block of code to define the C++ template implementation to the common/mld.cpp file:

template <> inline Q_DECL_CONSTEXPR unsigned long qbswap<unsigned long>(unsigned long source) {
  if (sizeof(unsigned long) == 8) {
    return qbswap<quint64>(quint64(source));
  } else {
    return qbswap<quint32>(quint32(source));
  }
}

I inserted the code block right after the #includes near the top of the file:

#include "mld.h"

#include "iputils.h"

#include <QHostAddress>
#include <QStringList>

template <> inline Q_DECL_CONSTEXPR unsigned long qbswap<unsigned long>(unsigned long source) {
  if (sizeof(unsigned long) == 8) {
    return qbswap<quint64>(quint64(source));
  } else {
    return qbswap<quint32>(quint32(source));
  }
}

This probably is not the best solution, but it allowed the build to complete.

@pstavirs

This comment has been minimized.

Copy link
Owner

commented Jun 25, 2018

@paul-blankenbaker Thanks Paul! That's a valid workaround. However, I'm still not clear why this is even needed. Going by the fact that this happens only for mld.cpp, I tried to find what is unique in that file and not used elsewhere - seems mld is the only .proto that uses the protobuf fixed64 type. Other proto files use uint64 - maybe protobuf is translating fixed64 and uint64 to different types. To test this hypothesis, can you attach gmp.pb.h and stp.pb.h from your environment where you see this problem?

@paul-blankenbaker

This comment has been minimized.

Copy link

commented Jun 26, 2018

I am uncertain as to why it is needed as well - the fixed64 must not be mapping back to something that has a declared template body. Maybe it has something to do with the fact that I am building with v3.5.0 (protobuf-3.5.0-4.fc28.x86_64 package on fedora).

I have attached the gmp.pb.h and stp.pb.h files that are generated during the build that fails (the github interface required that I put them in a zip file).

header-files.zip

Hope that helps.

@a17r

This comment has been minimized.

Copy link
Contributor Author

commented Jun 26, 2018

I confirm this fixed the build.

@pstavirs

This comment has been minimized.

Copy link
Owner

commented Jun 26, 2018

@paul-blankenbaker Thanks for the files. But unfortunately the files show both uint64 and fixed64 map to ::google::protobuf::uint64, so my theory wasn't correct. Looking again at the error, I see it complains about missing unsigned long qbswap<unsigned long>(unsigned long) which is the variant of qToBigEndian which accepts only a single input param while mld.cpp only uses the variant which accepts two input params - which is even more confusing!

I'm hesitant to commit the workaround without understanding why it happens. Also, I worry in other environments adding the workaround may lead to duplicate definition errors

@paul-blankenbaker

This comment has been minimized.

Copy link

commented Jun 26, 2018

I agree, I would not commit my work around as I think it is specific to my environment. And as you've mentioned, I would be worried about duplicate definition errors when building in other environments.

@ZeroChaos-

This comment has been minimized.

Copy link

commented Jun 26, 2018

even with the suggested patch

x86_64-pc-linux-gnu-g++ -c -Os -mtune=nocona -pipe -frecord-gcc-switches -isystem /usr/include/qt5 -fPIC -D_REENTRANT -Wall -W -DQT_NO_DEBUG -DQT_NETWORK_LIB -DQT_SCRIPT_LIB -DQT_XML_LIB -DQT_CORE_LIB -I. -isystem /usr/include/qt5 -isystem /usr/include/qt5/QtNetwork -isystem /usr/include/qt5/QtScript -isystem /usr/include/qt5/QtXml -isystem /usr/include/qt5/QtCore -I. -I/usr/lib64/qt5/mkspecs/linux-g++ -o mld.o mld.cpp
mld.cpp: In function ‘constexpr T qbswap(T) [with T = long unsigned int]’:
mld.cpp:29:27: error: call to non-constexpr function ‘T qbswap(T) [with T = long long unsigned int]’
     return qbswap<quint64>(quint64(source));
            ~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~
make[1]: *** [Makefile.ostproto:1044: mld.o] Error 1
@hansfn

This comment has been minimized.

Copy link

commented Aug 23, 2018

The work-around by paul-blankenbaker actually breaks building on my Gentoo with GCC 6.4, QT 5.9.6, libpcap 1.9.0 and protobuf 3.5.2. (The Gentoo package had the work-around/patch included by default.) Compiling without the work-around worked ;-)

Before I discovered this, I Googled a bit and found a goldendict issue that might be related - look at the patch. Or maybe it's something different.

@pstavirs

This comment has been minimized.

Copy link
Owner

commented Oct 11, 2018

By chance I got a system where I saw this problem and was able to debug and find root cause.

For 64-bit arch, Qt defines quint64 as unsigned long long (and quint32 as unsigned int), but Protobuf's google::protobuf::uint64 is defined as unsigned long. Now Qt defines qbswap only for the Qt defined 8/16/32/64 integer types aka q[u]intXXX. So qbswap<unsigned long long> and qbswap<unsigned int> is defined but not qbswap<unsigned long>.

mld.cpp was using qToBigEndian (which uses qbswap in turn) with a protobuf uint64 triggering the undefined references.

It would be good to get confirmation that the attached patch fixes the problem for others too before I commit it to the repo.

Patch File

@n9t12ne

This comment has been minimized.

Copy link

commented Jan 2, 2019

It would be good to get confirmation that the attached patch fixes the problem for others too before I commit it to the repo.

Patch File

@pstavirs , i can submit, that i can successfully build Ostinato (7b8c5a8) after applying patch 👍

My Environment : Ubuntu 18.10 (x64), GCC 8.2.0, libpcap-dev 1.8.1, libprotobuf-dev 3.0.0, qmake 3.1, QT 5.11.1

Thank You!

@pstavirs pstavirs closed this in c43f7d1 Jun 26, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.