Bug 783660 - boost 1.48.0-8 doesn't compile on arm sfp/hfp
boost 1.48.0-8 doesn't compile on arm sfp/hfp
Status: CLOSED RAWHIDE
Product: Fedora
Classification: Fedora
Component: boost (Show other bugs)
rawhide
Unspecified Unspecified
unspecified Severity unspecified
: ---
: ---
Assigned To: Petr Machata
Fedora Extras Quality Assurance
:
Depends On:
Blocks: ARMTracker
  Show dependency treegraph
 
Reported: 2012-01-21 10:05 EST by Peter Robinson
Modified: 2015-05-04 21:36 EDT (History)
5 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2012-01-26 08:45:54 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:


Attachments (Terms of Use)
Build fix (2.94 KB, patch)
2012-01-24 17:47 EST, Petr Machata
no flags Details | Diff
Patch for ignoring __NO_LONG_DOUBLE_MATH (819 bytes, patch)
2012-01-24 19:59 EST, Petr Machata
no flags Details | Diff
Additional build fix (1.05 KB, patch)
2012-01-25 09:02 EST, Petr Machata
no flags Details | Diff

  None (edit)
Description Peter Robinson 2012-01-21 10:05:50 EST
Full output: http://arm.koji.fedoraproject.org/koji/taskinfo?taskID=269906

Excerpt 

[ 33%] Building CXX object libs/math/src/tr1/CMakeFiles/boost_math_c99f-static-debug.dir/asinhf.cpp.o
cd /builddir/build/BUILD/boost_1_48_0/serial/libs/math/src/tr1 && /usr/bin/c++   -DBOOST_ALL_NO_LIB=1 -DBOOST_IOSTREAMS_USE_DEPRECATED -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4  -march=armv7-a -mfpu=vfpv3-d16  -mfloat-abi=hard  -I/builddir/build/BUILD/boost_1_48_0    -I/builddir/build/BUILD/boost_1_48_0/libs/math/src/tr1  -g     -o CMakeFiles/boost_math_c99f-static-debug.dir/asinhf.cpp.o -c /builddir/build/BUILD/boost_1_48_0/libs/math/src/tr1/asinhf.cpp
In file included from /builddir/build/BUILD/boost_1_48_0/boost/math/tr1.hpp:381:0,
                 from /builddir/build/BUILD/boost_1_48_0/libs/math/src/tr1/acoshl.cpp:10:
/builddir/build/BUILD/boost_1_48_0/boost/math/tools/promotion.hpp: In instantiation of 'struct boost::math::tools::promote_args<long double, float, float, float, float, float>':
/builddir/build/BUILD/boost_1_48_0/libs/math/src/tr1/c_policy.hpp:129:1:   required by substitution of 'template<class T> typename boost::math::tools::promote_args<T>::type c_policies::acosh(T) [with T = long double]'
/builddir/build/BUILD/boost_1_48_0/libs/math/src/tr1/acoshl.cpp:16:63:   required from here
/builddir/build/BUILD/boost_1_48_0/boost/math/tools/promotion.hpp:141:1: error: invalid application of 'sizeof' to incomplete type 'boost::STATIC_ASSERTION_FAILURE<false>'
make[2]: Leaving directory `/builddir/build/BUILD/boost_1_48_0/serial'
make[2]: *** [libs/math/src/tr1/CMakeFiles/boost_math_c99l-mt-shared.dir/acoshl.cpp.o] Error 1
make[1]: *** [libs/math/src/tr1/CMakeFiles/boost_math_c99l-mt-shared.dir/all] Error 2
/usr/bin/cmake -E cmake_progress_report /builddir/build/BUILD/boost_1_48_0/serial/CMakeFiles 
make[1]: *** Waiting for unfinished jobs....


1.46.x previously compiled without issue
Comment 1 Denis Arnaud 2012-01-21 10:46:36 EST
The error seems alike bug #757385
Comment 2 Petr Machata 2012-01-23 08:22:30 EST
boost::STATIC_ASSERTION_FAILURE<false> is a compile-time assertion.  It's not supposed to be defined.  It seems BOOST_MATH_NO_LONG_DOUBLE_MATH_FUNCTIONS is defined on ARM.  But then libs/math/src/tr1/acoshl.cpp makes a call to c_policies::acosh(long double), and the assertion fires.  Either ARM indeed doesn't support long double, and then we need to guard against such uses, or it's a fluke, and we need to fix the BOOST_MATH_NO_LONG_DOUBLE_MATH_FUNCTIONS detection.
Comment 3 Petr Machata 2012-01-23 08:25:50 EST
And by fluke, I don't mean fluke, but rather the opposite :)
Comment 4 Jon Masters 2012-01-23 09:14:21 EST
ARM EABI defines long double as being double on 32-bit ARM. So it should be defined, but it's only guaranteed to be as long as a double. I will look into this a little more, it's odd.
Comment 5 Petr Machata 2012-01-23 12:43:25 EST
Hmm, that probably means that we are mis-detecting the BOOST_MATH_NO_LONG_DOUBLE_MATH_FUNCTIONS macro.  The static assertion essentially just checks whether the passed type is something else than long double, because it thinks that long double is not supported on that arch.

That macro is defined in boost/math/tools/config.hpp.  There are several places where this is defined, but the most plausible path is by going through an ifdef on __NO_LONG_DOUBLE_MATH.  That would have to come from glibc.

One additional note, the reason it compiled on 1.46 may be that there we didn't build math at all (it was omitted from the CMake harness by mistake).  Chances are this was broken back then as well.
Comment 6 Petr Machata 2012-01-24 05:32:36 EST
I got Fedora chroot running on my trimslice and confirmed that the above happens because someone (presumably glibc) defines __NO_LONG_DOUBLE_MATH.

So that seems like a bug.  Another bug likely is that we are trying to build the long-double variants even though they are reportedly not supported.
Comment 7 Petr Machata 2012-01-24 07:01:57 EST
Hmm, there's unconditional definition of __NO_LONG_DOUBLE_MATH in bits/mathdef.h.  I see that there's a bunch of target-specific mathdef's, but ARM doesn't have it's own, so the generic one is taken, apparently.  From the references that I found on the Internet it seems that in glibc parlance, having "no long double" means that there's no distinct bit width for long double, and therefore ARM is right in defining this.

So the bug is in boost.  I think we might be misrepresenting the meaning of __NO_LONG_DOUBLE here, but I'll have to think about it.
Comment 8 Petr Machata 2012-01-24 17:47:57 EST
Created attachment 557336 [details]
Build fix

This fixes the obvious problem, i.e. building long double math libraries in face of (apparent) lack of support of long double.  I'll have this propagate to Denis's boost-cmake repo.  I checked that modularized boost project have the same problem, and I'll get in touch with them to have this fixed.
Comment 9 Denis Arnaud 2012-01-24 18:21:24 EST
(In reply to comment #8)
> I'll have this propagate to Denis's boost-cmake repo.

Thanks for the patch!

I have integrated it in the CMake-ified Boost repositories (https://github.com/pocb/boost/commit/8aa99f2e318b512e168492b62662a35213fa4f06) and updated the corresponding patch in the Fedora Rawhide Git repository (http://pkgs.fedoraproject.org/gitweb/?p=boost.git;a=blobdiff;f=boost-1.48.0-cmakeify-full.patch;h=eca40d632212af337b2b801f794b0ce06a1ad9e2;hp=c0d4be05ac6f4e840ba09636833fb14a495dbff5;hb=a7311be69805670a2ccf00060439f2129ff7e265;hpb=8cbc559305d8becefa489906a986cd73cc209c7f).

I leave you the honour to launch a Koji build :)
Comment 10 Peter Robinson 2012-01-24 18:37:43 EST
Thanks for the fixes

> I leave you the honour to launch a Koji build :)

Why don't we split it and you do x86 and I'll kick of the ARM one.
Comment 11 Denis Arnaud 2012-01-24 19:04:17 EST
(In reply to comment #10)
> Why don't we split it and you do x86 and I'll kick of the ARM one.

Sorry, I meant that Petr would trigger the Koji build.
Comment 12 Peter Robinson 2012-01-24 19:09:09 EST
(In reply to comment #11)
> (In reply to comment #10)
> > Why don't we split it and you do x86 and I'll kick of the ARM one.
> 
> Sorry, I meant that Petr would trigger the Koji build.

haha :-) Sorry, still thanks to you both for you fixes
Comment 13 Petr Machata 2012-01-24 19:59:40 EST
Created attachment 557346 [details]
Patch for ignoring __NO_LONG_DOUBLE_MATH

The semantics of __NO_LONG_DOUBLE_MATH seem to be that there's no distinct bit width for variables of long double type.  The prototypes of impacted functions (asinhl etc.) are still visible, but they are aliased to their "plain" variant.  The syntax for "long double" was present since the first version of both C and C++ standards, so the prototypes are just always visible, and __NO_LONG_DOUBLE_MATH only determines whether long double has a distinct bit width, i.e. whether a distinct set of _implementations_ is necessary.

On boost side, BOOST_MATH_NO_LONG_DOUBLE_MATH_FUNCTIONS is somewhat documented in boost/math/tools/config.hpp.  The semantics seem to be that it's defined on targets where compiler or system libraries are buggy or the syntax is missing altogether, but it's apparently also used in the same sense as above ("Dinkumware's std C lib doesn't have true long double precision").

Interestingly, the prototypes are still published, but boost won't generate the corresponding boost_math_*l library with implementations.  From the perspective of a boost client, it all seems fine, until he tries to link.  (At least that's what I think would happen, I don't have full ARM build handy to check.)

Ultimately I don't think upstream is right to omit long double functions like that.  I understand that they would skip this on arches where stuff gets miscompiled or similar.  But ARM implementation is correct, it just happens to have "double" and "long double" of the same length.  It's like suppressing "long long" functions on x86_64. I think __NO_LONG_DOUBLE_MATH should be just ignored.

So this is what's in this patch, together with long-winded rationale.  I'll want to run this by boost upstream.  They might have motives that I don't see.
Comment 14 Petr Machata 2012-01-24 20:31:57 EST
(In reply to comment #9)
> I have integrated it in the CMake-ified Boost repositories
> (https://github.com/pocb/boost/commit/8aa99f2e318b512e168492b62662a35213fa4f06)
> and updated the corresponding patch in the Fedora Rawhide Git repository

Great, you are quicker than me.  I got all lost in the long double support ;)  Originally I intended to form a pull request on github.

> I leave you the honour to launch a Koji build :)

It's now all in Rawhide.

I'm still spinning the first variant (just the build patch) on ARM, to see what comes out of it, to have something to compare to.

(In reply to comment #10)
> Why don't we split it and you do x86 and I'll kick of the ARM one.

Peter, you may want to just try and grab what's in rawhide and build it.  It might even work :)  Please report back if you do, I'll gladly just download the thing from arm-koji.  I'd also kick start the mainstream rawhide build if it seems to work fine.
Comment 15 Petr Machata 2012-01-24 20:43:19 EST
Just to clarify, I'm spinning the first variant locally.  I'll then proceed to build the fully patched variant locally, unless you beat me to it, which you as well might, because that won't be until tomorrow morning-ish UTC.
Comment 16 Peter Robinson 2012-01-25 03:16:13 EST
> Peter, you may want to just try and grab what's in rawhide and build it.  It
> might even work :)  Please report back if you do, I'll gladly just download the
> thing from arm-koji.  I'd also kick start the mainstream rawhide build if it
> seems to work fine.

There's now newer build in mainline koji than the one from the 16th
Comment 17 Petr Machata 2012-01-25 09:02:01 EST
Created attachment 557454 [details]
Additional build fix

The previous build fix doesn't do the right thing because it sucks in the system-installed boost header instead of the one in the build tree.  I ended up having to create a dedicated project for the long double support detection, and try_compile that.  I'm spinning another local build now.

Denis, this patch should also go to your cmake-boost upstream.
Comment 18 Petr Machata 2012-01-26 07:11:28 EST
Ok, it seems that the ARM builds of long double functions are sane.  I made a manual run of test_tr1.cpp (on ARM), and it passes.  I'll apply another patch for a boost-polygon bug and start a build in mainline Fedora.
Comment 19 Petr Machata 2012-01-26 08:45:54 EST
I built boost with these patches in rawhide.  I'm going to close this now.  If boost still fails on ARM, or if you get a "long double not supported" message during configuration, please re-open.
Comment 20 Petr Machata 2012-01-27 10:30:49 EST
Upstream submission:
https://svn.boost.org/trac/boost/ticket/6459

Note You need to log in before you can comment on or make changes to this bug.