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

clang doesn't export explicit specializations function templates #34

Closed
bog-dan-ro opened this issue Mar 21, 2016 · 9 comments
Closed
Labels

Comments

@bog-dan-ro
Copy link

Here:

Same code just works with gcc.

Compile command line:
/home/bogdan/necessitas/android-ndk/toolchains/llvm/prebuilt/linux-x86_64/bin/clang++ -c -target armv7-none-linux-androideabi -march=armv7-a -mfpu=vfp -mfloat-abi=softfp -ffunction-sections -funwind-tables -fstack-protector -fno-short-enums -DANDROID -Wa,--noexecstack -fno-builtin-memmove --sysroot=/home/bogdan/necessitas/android-ndk/platforms/android-16/arch-arm/ -g -Os -fomit-frame-pointer -fno-strict-aliasing -mthumb -std=c++1z -fvisibility=hidden -fvisibility-inlines-hidden -Wall -W -Wdate-time -D_REENTRANT -fPIC -DQT_NO_MTDEV -DQT_NO_LIBUDEV -DQT_NO_TSLIB -DQT_NO_LIBINPUT -DQT_NO_USING_NAMESPACE -DQT_HAVE_POLL -DQT_BUILD_CORE_LIB -DQT_BUILDING_QT -DQT_NO_CAST_TO_ASCII -DQT_ASCII_CAST_WARNINGS -DQT_MOC_COMPAT -DQT_USE_QSTRINGBUILDER -DQT_DEPRECATED_WARNINGS -DQT_DISABLE_DEPRECATED_BEFORE=0x050000 -DQT_NO_DEBUG -I. -I/home/bogdan/work/qt/openssl-1.0.2f/include -Iglobal -I../3rdparty/pcre -I../3rdparty/harfbuzz/src -I../3rdparty/md5 -I../3rdparty/md4 -I../3rdparty/sha3 -I../3rdparty/double-conversion/include -I../3rdparty/double-conversion/include/double-conversion -I../3rdparty/forkfd -I../../include -I../../include/QtCore -I../../include/QtCore/5.7.0 -I../../include/QtCore/5.7.0/QtCore -I.moc -isystem /home/bogdan/necessitas/android-ndk/sources/cxx-stl/gnu-libstdc++/4.9/include -isystem /home/bogdan/necessitas/android-ndk/sources/cxx-stl/gnu-libstdc++/4.9/libs/armeabi-v7a/include -isystem /home/bogdan/necessitas/android-ndk/platforms/android-16/arch-arm/usr/include -I../../mkspecs/android-clang -o .obj/qjni.o kernel/qjni.cpp

@DanAlbert
Copy link
Member

You've set -fvisibility-hidden, so the compiler is doing what you asked.

Q_CORE_EXPORT seems to be configured as follows: http://stackoverflow.com/a/3148584/632035

Don't know if that's something you're supposed to define on the command line or if there's something in QT's headers that set it up, but it's not being set properly.

A contrived test case shows that specializations are exported properly (no one would be able to use the Clang for production work if it didn't).

@bog-dan-ro
Copy link
Author

A few clarifications:

  • Q_CORE_EXPORT is defined as attribute((visibility("default"))) when that file is compiled (you'll need to check QtCore/qglobal.h).
  • only a few explicit specializations function templates are not exported (e.g. callObjectMethod).
    • the other functions (including other explicit specializations function templates (i.e. callMethod) are exported properly).
  • the bug I reported is quite similar to https://llvm.org/bugs/show_bug.cgi?id=23667 which seems to be fixed ~6 months ago.

Because seeing is believing, I'll give you the steps to compile Qt yourself:

  • $ git clone git://code.qt.io/qt/qtbase.git
  • $ cd qtbase
  • $ git checkout 5.7

Next you'll need to cherry-pick https://codereview.qt-project.org/#/c/153564/

Next step is to make sure you revert the workaround I did to src/plugins/bearer/android/src/wrappers/androidconnectivitymanager.cpp. In this file you can see that we're using other methods and other explicit specializations function templates from that class, but *ONLY * callObjectMethod is not found.

Final step is to configure & compile Qt, you'll need Android NDK & SDK (with API-16 installed):

  • $ ./configure -developer-build -release -force-debug-info -xplatform android-clang -nomake tests -nomake examples -android-ndk /path/to/NDK android-sdk /path/to/SDK
  • $ make -j8 (you'll need to wait a few minutes)

@DanAlbert
Copy link
Member

The LLVM bug you point at looks to be specific to dllexport, which is a Windows thing. We do already have that patch as well.

@DanAlbert DanAlbert added the clang label Jul 1, 2016
qtprojectorg pushed a commit to qt/qtandroidextras that referenced this issue Jul 19, 2016
…port

Should be reverted when android/ndk#34 is fixed

Change-Id: Idcc951ff432dbadd57a09851bcb7486b019b3426
Reviewed-by: Oswald Buddenhagen <[email protected]>
Reviewed-by: Eskil Abrahamsen Blomfeldt <[email protected]>
qtprojectorg pushed a commit to qt/qtbase that referenced this issue Jul 20, 2016
Should be reverted when android/ndk#34 is fixed

Change-Id: Ic7fe394412afc25082a9689da59d36cba8b3dade
Reviewed-by: Eskil Abrahamsen Blomfeldt <[email protected]>
@pirama-arumuga-nainar
Copy link
Collaborator

Can you give me updated instructions to reproduce this issue? In particular, from your comment on March 22:

Next step is to make sure you revert the workaround I did to src/plugins/bearer/android/src/wrappers/androidconnectivitymanager.cpp.

What WAR should be reverted?

@bog-dan-ro
Copy link
Author

You need to revert qt/qtbase@cc119de

Same setup as for #143
If you already have it compiled, just go to qtbase folder and do:

$ git revert cc119dee73c6
$ make clean
$ make -j10

@pirama-arumuga-nainar
Copy link
Collaborator

After looking at this a bit, not sure why the symbols are hidden. callObjectMethod does not use the template type but that doesn't seem to be the reason behind the issue. setField, which takes a parameter of the template type, is also hidden.

I am attaching an easy reproduction for others to take a look. Run the following command on the .ii file (which is in a zip so github accepts it as an attachment):
clang++ -cc1 -triple thumbv7-none-linux-android -emit-llvm -emit-llvm-uselists -disable-free -disable-llvm-verifier -main-file-name qjni.cpp -mrelocation-model pic -pic-level 2 -mthread-model posix -mdisable-fp-elim -fmath-errno -masm-verbose -mconstructor-aliases -fuse-init-array -target-cpu cortex-a8 -target-feature +soft-float-abi -target-feature -fp-only-sp -target-feature -d16 -target-feature +vfp2 -target-feature -vfp3 -target-feature -fp16 -target-feature -vfp4 -target-feature -fp-armv8 -target-feature -neon -target-feature -crypto -target-abi aapcs-linux -mfloat-abi soft -target-linker-version 2.24 -v -dwarf-column-info -debugger-tuning=gdb -O3 -Wall -W -std=c++14 -fdeprecated-macro -ferror-limit 19 -fmessage-length 85 -fvisibility hidden -fvisibility-inlines-hidden -femulated-tls -stack-protector 2 -fallow-half-arguments-and-returns -fno-signed-char -fobjc-runtime=gcc -fcxx-exceptions -fexceptions -fdiagnostics-show-option -fcolor-diagnostics -vectorize-loops -vectorize-slp -disable-llvm-passes -x c++-cpp-output qjni.ii

The problematic functions will be marked hidden in the generated qjni.ll
qjni.zip

@pirama-arumuga-nainar
Copy link
Collaborator

Aah, found the reason after digging into how Clang's visiblity-determination code. Thes specializations are getting hidden visiblity because the types they specialize (jstring etc.) have hidden visibility based on -fvisibility-hidden.

I cannot find anything in the spec that explicitly clarifies this. But, I think Clang is reasonable here - based on the requested visiblity, these types, and specializations on them, cannot be visible outside this unit.

@bog-dan-ro You can either (a) restrict your workaround to just those functions that specialize jobject and its subclasses or (b) use GCC visiblity pragmas around jni.h (https://gcc.gnu.org/onlinedocs/gcc/Visibility-Pragmas.html).

@bog-dan-ro
Copy link
Author

@pirama-arumuga-nainar but I'm changing the visibility of the entire class ... so IMHO all the methods should be exported properly :)

@pirama-arumuga-nainar
Copy link
Collaborator

pirama-arumuga-nainar commented Aug 23, 2016

And, the visibility of jstring etc is hidden, so Clang concludes that this specialization jstring is not callable from outside the translational unit.

To determine a method's visiblity, Clang picks the strictest visibility among that of its class, types of its parameters and its template types. The way to override this is via the method's attribute (like in your WAR).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants