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

Problem with make_standalond_toolchain.py #204

Closed
robiwano opened this issue Sep 20, 2016 · 5 comments
Closed

Problem with make_standalond_toolchain.py #204

robiwano opened this issue Sep 20, 2016 · 5 comments
Assignees
Milestone

Comments

@robiwano
Copy link

robiwano commented Sep 20, 2016

With the standalone toolchain from ndk r12b:
./build/tools/make_standalone_toolchain.py --api 21 --arch arm --stl libc++ --force --install-dir standalone/arm

the following code fails:

#include <assert.h>
#include <cmath>
#include <iostream>

int main(int, const char* [])
{
    auto value = std::log2(17.0);
    std::cout << "value = " << value << std::endl;
    return 0;
}

compiled with "standalone\arm\bin\clang++.cmd" -std=c++11 -pie -static-libstdc++ main.cpp yields following erroneous result:
value = -1077

whilst compiling with non-standalone it seems to yield correct values. Ideas?

@robiwano
Copy link
Author

Same code works with NDK r11c standalone toolchain.

@DanAlbert
Copy link
Member

Just confirmed that this is still a problem with r13-beta2, and that this also affects the shared library variant.

If I had to guess, I'd say this is similar to https://code.google.com/p/android/issues/detail?id=212634, but I haven't really dug in yet.

@DanAlbert DanAlbert self-assigned this Sep 20, 2016
@DanAlbert DanAlbert added this to the r14 milestone Sep 20, 2016
@DanAlbert
Copy link
Member

Well, that was a fun one. Turns out the problem is that libandroid_support's (the library that backs libc++ to support old android versions) had a completely broken implementation of log2 for armeabi (armeabi-v7a was fine).

There's a bug in Clang that means that standalone toolchains will always load the arm5 version of a library, not the arm7 one: #151 (comment) (Googlers, http://b/26180518 is the first investigation of this bug).

This didn't show up in ndk-build because ndk-build doesn't rely on the compiler being able to find libraries; it passes the full path to the library to the linker.

I haven't dug in to why this worked in r11. We did change the way libc++/libandroid_support were built and linked in r12, so my guess is that the libandroid_support definition simply wasn't getting used in r11.

Anyway, the fix is now up for review: https://android-review.googlesource.com/#/c/276095/

Since this fix is trivially correct and non-invasive, I'm going to cherry-pick this to r13.

@DanAlbert DanAlbert modified the milestones: r13, r14 Sep 21, 2016
@robiwano
Copy link
Author

Nice!!

@DanAlbert
Copy link
Member

Fix is merged into r13.

miodragdinic pushed a commit to MIPS/ndk that referenced this issue Apr 17, 2018
libandroid_support's definition of the byte-order for doubles on
armeabi was wrongly big endian rather than following the byte order of
the ABI. This was due to a missing check for __ARM_EABI__ (a bug in
the upstream FreeBSD libm source that bionic did not have because it
has been updated since it was fixed).

I haven't pulled the full update of math_private.h since it also
needs a lot of new macros for complex math support. I'll be cleaning
up libandroid_support more thoroughly soon anyway.

Note that this bug could surface even for arm7 when using a
standalone toolchain because we still have the Clang bug where it
doesn't search architecture specific lib directories, meaning that
arm5 gets used instead of arm7 (http://b/26180518 and
android/ndk#151).

Test: ./run_tests.py --filter log2
Bug: android/ndk#204
Change-Id: I2caaeac3afe0830008b865bc124f48200ac5f4b6
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants