[REG] r11x gcc-4.9 miscompiles Qt

[bog-dan-ro]

It is regression because gcc 4.9 from r10 didn't had this problem.

It seems the newest GCC mess up static variables.
In QPlatformIntegrationFactory::create [1] we're using loader() which is a static function defined a few lines above [2], check [3] to see how Q_GLOBAL_STATIC_WITH_ARGS is defined. In this moment something very strange happens, instead to use the loader() defined in [2] it uses another one which is defined in another file [4]! I'd like to mention that both files are part of the same .so file.

Please let me know if you need help to reproduce it.

[1] http://code.qt.io/cgit/qt/qtbase.git/tree/src/gui/kernel/qplatformintegrationfactory.cpp#n73
[2] http://code.qt.io/cgit/qt/qtbase.git/tree/src/gui/kernel/qplatformintegrationfactory.cpp#n46
[3] http://code.qt.io/cgit/qt/qtbase.git/tree/src/corelib/global/qglobalstatic.h
[4] http://code.qt.io/cgit/qt/qtbase.git/tree/src/gui/accessible/qaccessible.cpp#n462

[DanAlbert]

Does clang work? That'll be a quicker fix than us revving GCC.

We do have an updated (still 4.9, but synchronized with the chromeos toolchain) GCC that will be in r12 (hopefully in the beta next week), so this might already be fixed (though I haven't dug in to see what's wrong yet).

[bog-dan-ro]

clang has more problems :) and for the moment (actually for at least 1-2 releases) is a no go for Qt...
I'll be more than happy to test the updated GCC for you, if you'll tell me from where I can download it.

[DanAlbert]

Thanks! I just submitted the update for GCC. It'll be a few hours before the build is cooked, but I'll upload it to drive and share a link with you when it's ready.

[DanAlbert]

Oops. I didn't actually submit the GCC update yet. Will do that now and have a build tomorrow...

[DanAlbert]

More delays happened. Beta should be live on Wednesday anyway so I guess there's not much sense in uploading one specially now.

[bog-dan-ro]

In this case, I'll patiently wait for it.

Thanks Dan!

[DanAlbert]

Give it a shot: https://github.com/android-ndk/ndk/wiki#current-beta-release

[bog-dan-ro]

I have good news :) Now gcc works ok. Thanks a lot! I'll test it a little bit more these days to make sure it doesn't do any funny things in other places :)

I tested clang again, it seems to works ok though #34 is not fixed, I found an ugly workaround https://codereview.qt-project.org/#/c/153564/3/src/corelib/kernel/qjni.cpp but I prefer the right way :). But I found an worse problem, it seem the executable size grow up a LOT, now clang bins are more than twice bigger than gcc's (+100% more!), I'd like to mention that I used the same compiler flags, here https://www.kdab.com/~bogdan.vatra/compare.tar.bz2 I uploaded qt bins compiled with gcc and clang.

[bog-dan-ro]

Tested again with ndk r12b and I can reproduce it. The problem happens only when I do a debug build, if I replace -O0 with -Os (release build) everything works as expected.

[DanAlbert]

Tested again with ndk r12b and I can reproduce it.

Was your prior report of this working tested against a non-debug build? We haven't updated GCC since r12 beta 1.

[bog-dan-ro]

Yes, sadly I did a non-debug build.
A release + debug info (-g -Os) still works ok, but -g -O0 doesn't...
With NDK r10 everything is ok

[iamsergio]

This bug was reproduced on Linux/x86_64 last year. We tracked it down and reported a gcc bug:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65309

The bug is fixed upstream, so it's just a matter of android-ndk including the fix or upgrading to 4.9.3

[bog-dan-ro]

@DanAlbert Is is possible to have the fix in next beta release?

@iamsergio it will be nicer an upgrade to gcc 6.1 or at least to 5.4 ... ;-)

[enh]

we're looking at the 4.9.3 changes to see if there's anything critical in there, but probably won't invest in upgrading. we definitely won't be offering a 5.x or later.

we should probably put the "PSA: everyone should be switching to clang, GCC is no longer supported" back in the release notes... i suspect there are people who missed it while it was there.

[bog-dan-ro]

This bug is quite critical for Qt project, it is the reason why we tell people to keep using r10 and stay away from r11/r12 ndks... which is not good for Google nor for Qt project. It will be great if you can at least cherry-pick that fix.

People didn't missed that message :)! They(I'm one of them) are just pretty upset for your decision and we hope you'll soon realize that clang is still not as good as the ancient gcc[1] and, hopefully, we'll get an update ;-). "Sabotaging" (yes it's a joke) gcc will not force us to switch to clang, it will force us t keep using an older NDK or searching in other places for other NDKs :)

[1] yes gcc 4.9 is quite ancient comparing to gcc 6.1
bugs like #143 makes the blood to freeze in my veins ... and let's not forget that the binaries produced by clang are incredibly larger than the ones produced by gcc. This is the reason why people will not switch to clang no matter how many warnings you'll write.

[vok1980]

Is this problem solved in ndk r13?

[DanAlbert]

Is this problem solved in ndk r13?

There were no updates to GCC in r13. We are no longer updating GCC.

They(I'm one of them) are just pretty upset for your decision and we hope you'll soon realize that clang is still not as good as the ancient gcc[1] and, hopefully, we'll get an update ;-).

Sorry, I've been meaning to respond to this. I can sympathize with the fact that migrating to a new compiler can be a lot of work for some projects (I should know, I was one of the people that made it happen for Android).

The simple fact here though is that we don't have the bandwidth to support two compilers. Frankly, we (the NDK team) don't have the bandwidth to support even a single compiler; our Clang support is offloaded to a different team within Android (thanks to @stephenhines!). We used to do the same for GCC, but the team that used to do that for us has also switched to Clang.

Yes, there are cases where Clang still falls short of GCC, but it's worth noting that there are just as many cases where GCC falls short of Clang.

For those that haven't seen it, I explained our rationale for switching to Clang here.

[bog-dan-ro]

Yes, there are cases where Clang still falls short of GCC, but it's worth noting that there are just as many cases where GCC falls short of Clang.

+40% increase of binary size is not short at all ... check #133 . Even Qt now supports clang, we can't use it because +40% is way too much.

@DanAlbert: if you (NDK team) don't have time to maintain it, at least will you accept contributions for other people? You closed this bug ... but it's not fixed at all. I'd like to cherry pick the patch mentioned by @iamsergio, will you accept the contribution?

[DanAlbert]

+40% increase of binary size is not short at all ... check #133 . Even Qt now supports clang, we can't use it because +40% is way too much.

Qt seems to have managed to tickle all the worst cases in Clang. 40% is just absurd. I've poked that bug and asked Pirama to look at it.

if you (NDK team) don't have time to maintain it, at least will you accept contributions for other people?

The process of making a cherry-pick isn't the time consuming part, it's validation. That's unfortunately a task I can't offload :\

[bog-dan-ro]

Thanks a lot for understanding!

If you'll fix clang we'll (Qt project) more than happy to switch to clang. But we still need to keep using gnu stl (for binary compatibility), therefore pretty please with sugar on top do not remove it :) !

The process of making a cherry-pick isn't the time consuming part, it's validation. That's unfortunately a task I can't offload :\

Don't you have any automatic tests that we can just run to validate it?

[bog-dan-ro]

Regrading compiler validation, I think QT project can help on this matter. Qt has a few thousands auto tests that can be used to test any compiler updates ;-).

[eskilblomfeldt]

Given the severity of this bug, would you consider rolling back gcc to a version which works better? Or perhaps including some "community-supported", updated gcc version? The Qt Project is currently in a bit of a pickle, as the gcc in the NDK is broken to the point of being completely unusable, our customers are triggering this error on a regular basis and registering bugs for it with us, and clang is not an option given its current immaturity (see bugs registered by Bogdan) and the fact that it causes an increase in binary size which will be unacceptable for many users.

At this point, we manage to keep r10e working on our build machines and tell users to do the same, but in time this approach will probably get less and less viable.

In all humility: While gcc has had numerous problems in the past as well, my opinion is that the choice to deprecate it should have been delayed until a mature replacement was available. Or at least have a grace period where you cherry-pick critical fixes to stabilize the deprecated version, rather than upgrade it to a broken version and then immediately define this as the final upgrade.

[bog-dan-ro]

@eskilblomfeldt: I think the main issue here is deprecating gcc ... (see #26)