Skip to content

Commit

Permalink
Add the appropriate cxx_builtin_include_directory entries for clang t…
Browse files Browse the repository at this point in the history
…o the Android NDK crosstool created by android_ndk_repository.

Also, stop setting -isystem for the builtin include directories in the clang toolchains. Previously, we were incorrectly setting cxx_builtin_include_directory for clang toolchains to the gcc include directories. We were also setting -isystem on these directories, so when an Android build attempted to include an NDK header (like arm_neon.h), clang got gcc's version of that header.

A followup change will stop setting -isystem for gcc.

Fixes #2601.

Note that I intentionally did not attempt to fix the bug for NDK10. NDK10 is very old, defaults to GCC and contains two separate clang/LLVMs. As such, it would be more complicated to get right and test properly.

Also adds an integration test that attempts to compile an NDK header with clang.

This change does not entirely fix Tensorflow's Android sample app build with NDK13 (the motivation for #2601), however I believe that the remaining fixes are on Tensorflow's side. E.g. setting -Wno-c++11-narrowing in copts.

--
PiperOrigin-RevId: 149719100
MOS_MIGRATED_REVID=149719100
  • Loading branch information
aj-michael authored and vladmos committed Mar 10, 2017
1 parent 48e1d5b commit b6ea0d3
Show file tree
Hide file tree
Showing 17 changed files with 182 additions and 100 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -115,18 +115,27 @@ public String createGccToolchainPath(String toolchainName) {
.replace("%hostPlatform%", hostPlatform);
}

public void addToolchainIncludePaths(
/**
* Adds {@code cxx_builtin_include_directory} to the toolchain and also sets -isystem for that
* directory. Note that setting -isystem should be entirely unnecessary since builtin include
* directories are on the compiler search path by default by definition. This should be cleaned
* up (b/36091573).
*
* <p>Note also that this method is only for gcc include paths. The clang include paths follow a
* different path template and are not separated by architecture.
*/
public void addGccToolchainIncludePaths(
List<CToolchain.Builder> toolchains,
String toolchainName,
String targetPlatform,
String gccVersion) {

for (CToolchain.Builder toolchain : toolchains) {
addToolchainIncludePaths(toolchain, toolchainName, targetPlatform, gccVersion);
addGccToolchainIncludePaths(toolchain, toolchainName, targetPlatform, gccVersion);
}
}

public void addToolchainIncludePaths(
public void addGccToolchainIncludePaths(
CToolchain.Builder toolchain,
String toolchainName,
String targetPlatform,
Expand All @@ -141,6 +150,24 @@ public void addToolchainIncludePaths(
toolchain.addUnfilteredCxxFlag(includePath);
}
}

/**
* Gets the clang NDK builtin includes directories that exist in the NDK. These directories are
* always searched for header files by clang and should be added to the CROSSTOOL in the
* cxx_builtin_include_directories list.
*
* <p>You can see the list of directories and the order that they are searched in by running
* {@code clang -E -x c++ - -v < /dev/null}. Note that the same command works for {@code gcc}.
*/
public String createClangToolchainBuiltinIncludeDirectory(String clangVersion) {
String clangBuiltinIncludeDirectoryPathTemplate =
"external/%repositoryName%/ndk/toolchains/llvm/prebuilt/%hostPlatform%/lib64/clang/"
+ "%clangVersion%/include";
return clangBuiltinIncludeDirectoryPathTemplate
.replace("%repositoryName%", repositoryName)
.replace("%hostPlatform%", hostPlatform)
.replace("%clangVersion%", clangVersion);
}

private ImmutableList<String> createToolchainIncludePaths(
String toolchainName, String targetPlatform, String gccVersion) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ private CToolchain.Builder createAarch64Toolchain() {
.addCompilerFlag("-fno-omit-frame-pointer")
.addCompilerFlag("-fno-strict-aliasing"));

ndkPaths.addToolchainIncludePaths(toolchain, toolchainName, targetPlatform, "4.9");
ndkPaths.addGccToolchainIncludePaths(toolchain, toolchainName, targetPlatform, "4.9");
stlImpl.addStlImpl(toolchain, "4.9");
return toolchain;
}
Expand Down Expand Up @@ -186,7 +186,7 @@ private CToolchain.Builder createAarch64ClangToolchain(String clangVersion) {
.addCompilerFlag("-fno-omit-frame-pointer")
.addCompilerFlag("-fno-strict-aliasing"));

ndkPaths.addToolchainIncludePaths(toolchain, toolchainName, targetPlatform, "4.9");
ndkPaths.addGccToolchainIncludePaths(toolchain, toolchainName, targetPlatform, "4.9");
stlImpl.addStlImpl(toolchain, "4.9");
return toolchain;
}
Expand Down Expand Up @@ -309,7 +309,7 @@ private CToolchain.Builder createBaseArmeabiToolchain(
.addCompilerFlag("-fno-strict-aliasing"));
}

ndkPaths.addToolchainIncludePaths(toolchain, toolchainName, targetPlatform, gccVersion);
ndkPaths.addGccToolchainIncludePaths(toolchain, toolchainName, targetPlatform, gccVersion);
return toolchain;
}

Expand Down Expand Up @@ -442,7 +442,7 @@ private CToolchain.Builder createBaseArmeabiClangToolchain(String clangVersion,
.addCompilerFlag("-fno-strict-aliasing"));
}

ndkPaths.addToolchainIncludePaths(toolchain, toolchainName, targetPlatform, "4.8");
ndkPaths.addGccToolchainIncludePaths(toolchain, toolchainName, targetPlatform, "4.8");
return toolchain;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ private List<CToolchain.Builder> createMips64Toolchains() {
}

List<CToolchain.Builder> toolchains = toolchainsListBuilder.build();
ndkPaths.addToolchainIncludePaths(
ndkPaths.addGccToolchainIncludePaths(
toolchains, "mips64el-linux-android-4.9", "mips64el-linux-android", "4.9");
stlImpl.addStlImpl(toolchains, "4.9");
return toolchains;
Expand Down Expand Up @@ -114,7 +114,7 @@ private List<CToolchain.Builder> createMipsToolchains() {

.setBuiltinSysroot(ndkPaths.createBuiltinSysroot("mips"));

ndkPaths.addToolchainIncludePaths(
ndkPaths.addGccToolchainIncludePaths(
mipsClang, "mipsel-linux-android-4.8", "mipsel-linux-android", "4.8");
stlImpl.addStlImpl(mipsClang, "4.8");
toolchainsListBuilder.add(mipsClang);
Expand All @@ -138,7 +138,7 @@ private CToolchain.Builder createMipsToolchain(

.setBuiltinSysroot(ndkPaths.createBuiltinSysroot("mips"));

ndkPaths.addToolchainIncludePaths(
ndkPaths.addGccToolchainIncludePaths(
toolchain, "mipsel-linux-android-" + gccVersion, "mipsel-linux-android", gccVersion);
stlImpl.addStlImpl(toolchain, gccVersion);
return toolchain;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ ImmutableList<CToolchain.Builder> createCrosstools() {

.setBuiltinSysroot(ndkPaths.createBuiltinSysroot("x86"));

ndkPaths.addToolchainIncludePaths(x86Clang, "x86-4.8", "i686-linux-android", "4.8");
ndkPaths.addGccToolchainIncludePaths(x86Clang, "x86-4.8", "i686-linux-android", "4.8");
stlImpl.addStlImpl(x86Clang, "4.8");
toolchains.add(x86Clang);
}
Expand All @@ -85,7 +85,7 @@ ImmutableList<CToolchain.Builder> createCrosstools() {

.addCompilerFlag("-fstack-protector-strong");

ndkPaths.addToolchainIncludePaths(x8664, "x86_64-4.9", "x86_64-linux-android", "4.9");
ndkPaths.addGccToolchainIncludePaths(x8664, "x86_64-4.9", "x86_64-linux-android", "4.9");
stlImpl.addStlImpl(x8664, "4.9");
toolchains.add(x8664);

Expand All @@ -102,7 +102,7 @@ ImmutableList<CToolchain.Builder> createCrosstools() {

.setBuiltinSysroot(ndkPaths.createBuiltinSysroot("x86_64"));

ndkPaths.addToolchainIncludePaths(x8664Clang, "x86_64-4.9", "x86_64-linux-android", "4.9");
ndkPaths.addGccToolchainIncludePaths(x8664Clang, "x86_64-4.9", "x86_64-linux-android", "4.9");
stlImpl.addStlImpl(x8664Clang, "4.9");
toolchains.add(x8664Clang);
}
Expand Down Expand Up @@ -132,7 +132,7 @@ private CToolchain.Builder createX86Toolchain(

.addCompilerFlag(stackProtrectorFlag);

ndkPaths.addToolchainIncludePaths(
ndkPaths.addGccToolchainIncludePaths(
toolchain, "x86-" + gccVersion, "i686-linux-android", gccVersion);
stlImpl.addStlImpl(toolchain, gccVersion);
return toolchain;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,9 @@
* Generates a CrosstoolRelease proto for the Android NDK.
*/
final class AndroidNdkCrosstoolsR11 {
/** {@code ./ndk/toolchains/llvm/prebuilt/linux-x86_64/bin/clang --version} */
static final String CLANG_VERSION = "3.8.243773";

private AndroidNdkCrosstoolsR11() {}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ private CToolchain.Builder createAarch64Toolchain() {
.addCompilerFlag("-fno-omit-frame-pointer")
.addCompilerFlag("-fno-strict-aliasing"));

ndkPaths.addToolchainIncludePaths(toolchain, toolchainName, targetPlatform, "4.9");
ndkPaths.addGccToolchainIncludePaths(toolchain, toolchainName, targetPlatform, "4.9");
stlImpl.addStlImpl(toolchain, "4.9");
return toolchain;
}
Expand Down Expand Up @@ -169,7 +169,6 @@ private CToolchain.Builder createAarch64ClangToolchain() {
.addCompilerFlag("-fno-omit-frame-pointer")
.addCompilerFlag("-fno-strict-aliasing"));

ndkPaths.addToolchainIncludePaths(toolchain, toolchainName, targetPlatform, "4.9");
stlImpl.addStlImpl(toolchain, "4.9");
return toolchain;
}
Expand All @@ -186,13 +185,13 @@ private List<CToolchain.Builder> createArmeabiToolchains(boolean thumb,
.addCompilerFlag("-march=armv5te")
.addCompilerFlag("-mtune=xscale")
.addCompilerFlag("-msoft-float"),

createBaseArmeabiToolchain(thumb, excludedTools)
.setToolchainIdentifier(
createArmeabiName("arm-linux-androideabi-4.9-v7a", thumb))
.setTargetCpu(createArmeabiCpuName("armeabi-v7a", thumb))

.addCompilerFlag("-march=armv7-a")
.addCompilerFlag("-march=armv7-a")
.addCompilerFlag("-mfpu=vfpv3-d16")
.addCompilerFlag("-mfloat-abi=softfp")

Expand All @@ -203,12 +202,12 @@ private List<CToolchain.Builder> createArmeabiToolchains(boolean thumb,
.setToolchainIdentifier(
createArmeabiName("arm-linux-androideabi-4.9-v7a-hard", thumb))
.setTargetCpu(createArmeabiCpuName("armeabi-v7a-hard", thumb))
.addCompilerFlag("-march=armv7-a")

.addCompilerFlag("-march=armv7-a")
.addCompilerFlag("-mfpu=vfpv3-d16")
.addCompilerFlag("-mhard-float")
.addCompilerFlag("-D_NDK_MATH_NO_SOFTFP=1")

.addLinkerFlag("-march=armv7-a")
.addLinkerFlag("-Wl,--fix-cortex-a8")
.addLinkerFlag("-Wl,--no-warn-mismatch")
Expand Down Expand Up @@ -287,7 +286,7 @@ private CToolchain.Builder createBaseArmeabiToolchain(
.addCompilerFlag("-fno-strict-aliasing"));
}

ndkPaths.addToolchainIncludePaths(toolchain, toolchainName, targetPlatform, "4.9");
ndkPaths.addGccToolchainIncludePaths(toolchain, toolchainName, targetPlatform, "4.9");
return toolchain;
}

Expand All @@ -297,43 +296,43 @@ private List<CToolchain.Builder> createArmeabiClangToolchain(boolean thumb) {
createBaseArmeabiClangToolchain(thumb)
.setToolchainIdentifier(createArmeabiName("arm-linux-androideabi-clang3.8", thumb))
.setTargetCpu(createArmeabiCpuName("armeabi", thumb))

.addCompilerFlag("-target")
.addCompilerFlag("armv5te-none-linux-androideabi") // LLVM_TRIPLE
.addCompilerFlag("-march=armv5te")
.addCompilerFlag("-mtune=xscale")
.addCompilerFlag("-msoft-float")

.addLinkerFlag("-target")
// LLVM_TRIPLE
.addLinkerFlag("armv5te-none-linux-androideabi"),

createBaseArmeabiClangToolchain(thumb)
.setToolchainIdentifier(createArmeabiName("arm-linux-androideabi-clang3.8-v7a", thumb))
.setTargetCpu(createArmeabiCpuName("armeabi-v7a", thumb))

.addCompilerFlag("-target")
.addCompilerFlag("armv7-none-linux-androideabi") // LLVM_TRIPLE
.addCompilerFlag("-march=armv7-a")
.addCompilerFlag("-march=armv7-a")
.addCompilerFlag("-mfloat-abi=softfp")
.addCompilerFlag("-mfpu=vfpv3-d16")

.addLinkerFlag("-target")
.addLinkerFlag("armv7-none-linux-androideabi") // LLVM_TRIPLE
.addLinkerFlag("-Wl,--fix-cortex-a8"),

createBaseArmeabiClangToolchain(thumb)
.setToolchainIdentifier(
createArmeabiName("arm-linux-androideabi-clang3.8-v7a-hard", thumb))
.setTargetCpu(createArmeabiCpuName("armeabi-v7a-hard", thumb))

.addCompilerFlag("-target")
.addCompilerFlag("armv7-none-linux-androideabi") // LLVM_TRIPLE
.addCompilerFlag("-march=armv7-a")
.addCompilerFlag("-march=armv7-a")
.addCompilerFlag("-mfpu=vfpv3-d16")
.addCompilerFlag("-mhard-float")
.addCompilerFlag("-D_NDK_MATH_NO_SOFTFP=1")

.addLinkerFlag("-target")
.addLinkerFlag("armv7-none-linux-androideabi") // LLVM_TRIPLE
.addLinkerFlag("-Wl,--fix-cortex-a8")
Expand All @@ -355,7 +354,9 @@ private CToolchain.Builder createBaseArmeabiClangToolchain(boolean thumb) {
.setCompiler("clang3.8")

.addAllToolPath(ndkPaths.createClangToolpaths(toolchainName, targetPlatform, null))

.addCxxBuiltinIncludeDirectory(
ndkPaths.createClangToolchainBuiltinIncludeDirectory(
AndroidNdkCrosstoolsR11.CLANG_VERSION))
.setBuiltinSysroot(ndkPaths.createBuiltinSysroot("arm"))

// Compiler flags
Expand Down Expand Up @@ -411,7 +412,6 @@ private CToolchain.Builder createBaseArmeabiClangToolchain(boolean thumb) {
.addCompilerFlag("-fno-strict-aliasing"));
}

ndkPaths.addToolchainIncludePaths(toolchain, toolchainName, targetPlatform, "4.8");
return toolchain;
}

Expand All @@ -422,4 +422,4 @@ private static String createArmeabiName(String base, boolean thumb) {
private static String createArmeabiCpuName(String base, boolean thumb) {
return base + (thumb ? "-thumb" : "");
}
}
}
Loading

0 comments on commit b6ea0d3

Please sign in to comment.