After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 614767 - libgstreamer build failure on OS X x86_64
libgstreamer build failure on OS X x86_64
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gstreamer (core)
0.10.26
Other Mac OS
: Normal blocker
: 0.10.29
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2010-04-03 20:23 UTC by Daniel Macks
Modified: 2010-04-13 03:21 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
patch (1.16 KB, patch)
2010-04-05 20:48 UTC, David Schleef
none Details | Review
revised patch (1.26 KB, patch)
2010-04-09 21:52 UTC, David Schleef
none Details | Review

Description Daniel Macks 2010-04-03 20:23:19 UTC
Passing along a report from a fink user: trying to build gstreamer-0.10.26 on OS X 10.5/64bit fails:

> LINK  libgstreamer-0.10.la
>Undefined symbols:
> "___udivti3", referenced from:
>     __gst_util_uint64_scale_int in libgstreamer_0.10_la-gstutils.o
>     __gst_util_uint64_scale in libgstreamer_0.10_la-gstutils.o
>ld: symbol(s) not found
Comment 1 Daniel Macks 2010-04-03 20:27:02 UTC
Other users have reported success on 10.4 and on 10.5/32bit
Comment 2 David Schleef 2010-04-03 20:47:02 UTC
This looks suspiciously like a toolchain bug.  That symbol is added by the compiler, and normally will be resolved by the linker using libgcc.  One possibility is that the linker is using a libgcc that doesn't match the compiler.
Comment 3 Daniel Macks 2010-04-05 17:55:32 UTC
Also have successful report on 10.6/64bit and going from 10.5 to 10.6 OS X got more 64bit support, so Comment #2 sounds like it's on the right track. The symbol is in /usr/lib/gcc/i686-apple-darwin9/4.2.1/x86_64/libgcc.a but doesn't appear to be in 4.0.1's libgcc (which is what appears to be the default for my 10.5/64bit tester). I see lots of datatype/word-size and other #if controls in the sources, I wonder if something is being mis-guessed, an unexpected (based on other platforms) wordsize for the given gcc version?. Will keep trying various combinations....
Comment 4 Tim-Philipp Müller 2010-04-05 18:24:14 UTC
So NOTGNOME then?
Comment 5 Daniel Macks 2010-04-05 18:37:13 UTC
No, YESGNOME...it's apparently gstreamer that's apparently misdetermining the wordsize (or some similar platform-specific feature) on this platform. The workaround might be "change platform" (compiler toolchain, kernel version, etc.), but that seems more like using local hack around an upstream problem not actually solving upstream problem. The upstream resolution may be "we don't intend support that specific platform combination": once it's known what specific platform change resolves the problem, if that's the solution you want, the unsupported combination can be tested-for and reported directly with a configure test and clear instructions.
Comment 6 David Schleef 2010-04-05 20:31:34 UTC
__udivti3 is for 128-bit division, so what's probably happening is that GStreamer detects existence of uint128_t, and assumes that the compiler will support 128-bit division.  In the ideal situation, if this assumption were not true, I'd complain about a broken toolchain, but I can think of several innocuous reasons why a particular system might be broken in this way (say, mismatched compiler and system headers).  So we should probably check that the toolchain supports 128-bit divide directly.

In the meantime, you can work around it by setting "gst_cv_uint128_t=no" in the environment (or config.site) before running configure.
Comment 7 David Schleef 2010-04-05 20:48:49 UTC
Created attachment 158002 [details] [review]
patch
Comment 8 David Schleef 2010-04-05 20:50:38 UTC
Please test this.  It should be fairly obvious to port the patch to configure in the release tarball.
Comment 9 Daniel Macks 2010-04-07 14:07:21 UTC
My tester reports that with that configure.ac patch and autoreconf -fi:

>Still getting
>
>checking for __uint128_t... yes
>
>and thus same error.

but with export gst_cv_uint128_t=no it works and builds successfully on both 10.5/32bit and 10.5/64bit.
Comment 10 David Schleef 2010-04-07 20:16:49 UTC
Thanks.

If you feel up to it, could you write a simple 10-line C program that does a uint128_t by uint128_t divide that triggers this bug?  Apparently, the code in the current test doesn't.  I'm guessing that the compiler optimizes out the divide.

(By the way, Autoconf takes the 5 lines visible in the patch above, wraps it with main(), and attempts to compile it.)

Otherwise, I can look into this more in the next week or so.
Comment 11 Tim-Philipp Müller 2010-04-08 08:25:22 UTC
Marking as blocker for now, would be nice if we could get this fixed for the next release (core freezes this weekend, release scheduled for ca. April 23rd).
Comment 12 Benjamin Otte (Company) 2010-04-08 12:10:12 UTC
I wondered if making the __(In reply to comment #7)
> Created an attachment (id=158002) [details] [review]
> patch

Does it help if you make those variables volatile __uint128_t to get around gcc optimizing the test away?
Comment 13 Daniel Macks 2010-04-09 17:52:40 UTC
>jfm-2:tmp root# cat uint128_tst.c
>#include <stdio.h>
>int main ()
>{
>        volatile unsigned long long v1 = 1024ULL;
>        printf ("v1= %llu \n", v1);
>        volatile unsigned long long v2 = 0x8000000000000000ULL;
>        printf ("v2= %llu \n", v2);
>        volatile __uint128_t u = ((__uint128_t)v2)/((__uint128_t)v1);
>        printf ("u= %llu \n", u);
>        return 0;
>}
>jfm-2:tmp root# gcc -v -arch x86_64 -O0 -save-temps uint128_tst.c -o uint128_tst
[...]
>GNU C version 4.0.1 (Apple Inc. build 5493) (i686-apple-darwin9)
>        compiled by GNU C version 4.0.1 (Apple Inc. build 5493).
>jfm-2:tmp root# fgrep divq uint128_tst.s
>        divq    -56(%rbp)
>jfm-2:tmp root# uint128_tst
>v1= 1024
>v2= 9223372036854775808
>u= 9007199254740992
>
>The compiler obviously handles that division well !
>
>jfm-2:tmp root# nm -m uint128_tst.o
>00000000000000c8 (__TEXT,__eh_frame) non-external EH_frame1
>00000000000000a7 (__TEXT,__cstring) non-external LC0
>00000000000000b2 (__TEXT,__cstring) non-external LC1
>00000000000000bd (__TEXT,__cstring) non-external LC2
>0000000000000000 (__TEXT,__text) external _main
>00000000000000e0 (__TEXT,__eh_frame) external _main.eh
>                 (undefined) external _printf
>
>shows that nothing is needed at the linking stage

(e.g., no ___udivti3)
Comment 14 David Schleef 2010-04-09 20:22:12 UTC
Ok, it looks like writing a test case will be harder than I thought.  I won't be able to look at this until I upgrade my mac.
Comment 15 Daniel Macks 2010-04-09 20:51:15 UTC
Same bug (with some tracing of the actual compiler situation, which maybe means it's cross-platform not just OS=MAC) on pkgsrc mailing list:

http://mail-index.netbsd.org/pkgsrc-bugs/2010/03/30/msg037304.html

Includes a possible test. Looks like they (try to:) use actual __uint128_t variables with values rather than casting from a ulonglong in the division.
Comment 16 David Schleef 2010-04-09 21:52:32 UTC
Created attachment 158332 [details] [review]
revised patch

Ok, that provided an important clue.  Please try this patch.  If it works, we'll put it in the upcoming release.
Comment 17 Edward Hervey 2010-04-10 07:20:05 UTC
Patch makes sense. We should commit this once it's confirmed to detect the issue.
Comment 18 Daniel Macks 2010-04-11 07:12:59 UTC
My tester is tracing a lot of assembly internals for the compiled code, will get a summary soon. Mean time, he thinks might need to assign at least one value from an external source and do something external with the value rather than just calculating it:

>In fact, to be put in the configure test, it has to be complicated
>further: the tests are run with -Os, and to prevent the compiler
>from opimizing out the difficulty, I needed both a random input,
>and to do something with the result (print it..)
[...]
>      __uint128_t v1 = 1000ULL;
>      __uint128_t v2 = (__uint128_t) random();
>      __uint128_t u = (v2)/(v1);
>      printf ("u= %%llu \\n", u);

-Os is the apple-recommended optimization level. Seems reasonable that a compiler might optimize a whole "constant" block of math and then might omit that block entirely once the result is ignored and has no side effects?
Comment 19 Benjamin Otte (Company) 2010-04-11 10:38:04 UTC
That's why I suggested using "volatile" in the variable declaration, like so:

volatile __uint128_t v1 = 1000ULL;
volatile __uint128_t v2 = (__uint128_t) -1;
return v2 / v1;
Comment 20 David Schleef 2010-04-11 19:34:36 UTC
volatile doesn't work, because the compiler is free to ignore it when it understands the complete lifetime of the object.  "static", however, does work.  I would have just put them in the global scope, but that's harder to do with autoconf.  Yay for autoconf tricks.  Please test stuff based on my patch; it already does the autoconfy stuff correctly and has been tested on several platforms.
Comment 21 JF Mertens 2010-04-12 17:15:40 UTC
The revised patch with "static" seems perfect to me , 
except for the crucial point : one NEEDS also to replace
AC_TRY_COMPILE
by
AC_TRY_LINK


jfm
Comment 22 Tim-Philipp Müller 2010-04-12 23:04:22 UTC
ds: pushed your patch with the additional s/TRY_COMPILE/TRY_LINK/:

commit b090081f4f710e05817dca625717b5b49ac87347
Author: David Schleef <ds@schleef.org>
Date:   Mon Apr 5 13:46:23 2010 -0700

    configure: Change check for uint128_t
    
    Check for ability to divide uint128_t values, since that what
    we actually use it for (in gstutils.c).  The existence of a
    uint128_t type doesn't mean the compiler can actually generate
    code for it.  Also make sure that we can actually link the
    result successfully.
    
    Fixes bug #614767.
Comment 23 Daniel Macks 2010-04-13 01:51:05 UTC
For the record, JF Mertens was the actual tester behind my reports.
Comment 24 JF Mertens 2010-04-13 03:21:37 UTC
As a completely minor aside, my diff reads as :

bash-4.0# diff configure.ac.bak  configure.ac 
315,320c315,319
<     AC_TRY_COMPILE([ ], [
<       unsigned long long v1 = 1024ULL;
<       unsigned long long v2 = 0x8000000000000000ULL;
<       __uint128_t u = ((__uint128_t)v1)*((__uint128_t)v2);
< 
<       return 0;
---
>     AC_TRY_LINK([ ], [
> 	static __uint128_t v1 = 100;
> 	static __uint128_t v2 = 10;
> 	static __uint128_t u;
> 	u = v1 / v2;


difference is that the "return 0;" is also zapped,
a) because it is anyway added by the AC_TRY_ commands
b) because not zapping it makes an error in  config.log
a bit harder to read for someone who is not quite sure
of his C  syntax ...

Or would there be versions of autoconf that don't add
this  "return 0;"  ?

JF Mertens