bug#51591: webkitgtk fails to build on i686-linux; possibly a clang issue

[Liliana Marie Prikler]

Hi,

Am Donnerstag, den 04.11.2021, 19:15 -0400 schrieb Mark H Weaver:
> Liliana Marie Prikler <liliana.prikler@ist.tugraz.at> writes:
> 
> > Am Mittwoch, den 03.11.2021, 17:04 -0400 schrieb Mark H Weaver:
> > > Earlier, I wrote:
> > > > libwebkit2gtk-4.0.so fails to link on i686-linux, due to an
> > > > undefined reference to '__mulodi4'.
> > > 
> > > Here are some relevant links:
> > > 
> > >   https://bugs.webkit.org/show_bug.cgi?id=190208
> > >   https://trac.webkit.org/changeset/272140/webkit
> > >   https://github.com/android/ndk/issues/506
> > This error does not occur when compiling with GCC [1].
> 
> Right.  As mentioned in the first link above:
> 
>   "This is because clang generates code using the __mulodi4 symbol 
>    for __builtin_mul_overflow.  But this symbol is available only in
>    compiler-rt, and not in the libgcc runtime used by most Linux
>    distributions of clang."
> 
> So, one possible solution might be to link with compiler-rt, which is
> the 'clang-runtime-11' package in Guix.  However, it's possible that
> this might cause other complications.
I think the overhead might be comparable to using GCC 10 everywhere
Webkit is linked.

> A more conservative approach would be to apply a patch to
> trunk/Source/WTF/wtf/CheckedArithmetic.h analogous to the one in the
> second link I cited above, namely this one:
> 
>   https://trac.webkit.org/changeset/272140/webkit
> 
> However, it would need to be changed slightly.  The patch above
> arranges to avoid using __builtin_mul_overflow on 32-bit ARM
> systems.  We would need to do the same for 32-bit x86 as well.  So,
> where the patch above has this:
> 
> --8<---------------cut here---------------start------------->8---
> /* On Linux with clang, libgcc is usually used instead of compiler-
> rt, and it does    
>  * not provide the __mulodi4 symbol used by clang for
> __builtin_mul_overflow    
>  */    
> #if COMPILER(GCC) || (COMPILER(CLANG) && !(CPU(ARM) &&
> OS(LINUX)))    
> #define USE_MUL_OVERFLOW 1    
> #endif    
> --8<---------------cut here---------------end--------------->8---
> 
> We would need to change "CPU(ARM)" to "(CPU(ARM) || CPU(XXX))", where
> XXX is the appropriate symbol for 32-bit x86.  Or maybe there's
> another solution.
> 
> I won't be able to look at this in the next couple of days, so
> hopefully someone else can pick this up.
The #else case doesn't look so bad on this, so we could for the time
being just do #if COMPILER(GCC) or even #if 0.

> > However, now dependant packages fail to link Webkit [2].  We might
> > have
> > to add GCC 11 to all of them -- or at least to a fair number.  I've
> > verified that gnome-online-accounts builds with GCC 11 added, we
> > might
> > want to make sure we check the rest of the gnome package as well.
> 
> I'm not sure about this approach.  Maybe it's feasible, but there
> might be problems if *any* C++ library built using GCC 7 is linked
> together with WebKitGTK.
Given that GTK is mostly a plain C land, I don't think too many C++
libraries are actually linked against it, perhaps stuff built on top of
glibmm might get the same treatment.

> > > Also, I just noticed that "-mfpmath=sse -msse2" is being passed
> > > on the compile command line.  Historically, we've chosen not to
> > > assume the availability of SSE or SSE2 on i686-linux, so it would
> > > be good to inhibit those flags.
> > This is still true for the GCC build.  Could you add the necessary
> > flags to disable them?
> 
> I don't know when I'll be able to look into it.  It's a busy time for
> me.
I did some static code analysis and it seems we could comment out the
line
  CHECK_FOR_SSE2()
in Source/cmake/DetectSSE2.cmake.  In my personal opinion, we should
probably do so on all platforms.

Cheers