versioner: compile headers in both C and C++ mode.
Bug: https://github.com/android-ndk/ndk/issues/440
Test: python run_tests.py
Change-Id: Ib572a8fdcc00f6b88a25003a085b16ce9698d692
diff --git a/tools/versioner/src/Android.bp b/tools/versioner/src/Android.bp
index c5afa56..b86228f 100644
--- a/tools/versioner/src/Android.bp
+++ b/tools/versioner/src/Android.bp
@@ -1,6 +1,8 @@
cc_binary_host {
name: "versioner",
+ cpp_std: "gnu++17",
+
srcs: [
"versioner.cpp",
"Arch.cpp",
@@ -29,6 +31,7 @@
"-Wextra",
"-Werror",
"-Wno-unused-parameter",
+ "-fno-omit-frame-pointer",
"-D__STDC_CONSTANT_MACROS",
"-D__STDC_LIMIT_MACROS",
@@ -37,7 +40,6 @@
target: {
host: {
cppflags: [
- "-std=gnu++1z",
"-fno-rtti",
],
},
diff --git a/tools/versioner/src/CompilationType.cpp b/tools/versioner/src/CompilationType.cpp
index e9d31cb..7e7bb5d 100644
--- a/tools/versioner/src/CompilationType.cpp
+++ b/tools/versioner/src/CompilationType.cpp
@@ -21,6 +21,7 @@
std::string to_string(const CompilationType& type) {
std::stringstream ss;
- ss << to_string(type.arch) << "-" << type.api_level << " [fob = " << type.file_offset_bits << "]";
+ ss << to_string(type.arch) << "-" << type.api_level << " [" << (type.cpp ? "c++" : "c")
+ << ", fob = " << type.file_offset_bits << "]";
return ss.str();
}
diff --git a/tools/versioner/src/CompilationType.h b/tools/versioner/src/CompilationType.h
index 7d516b2..2f4cf5c 100644
--- a/tools/versioner/src/CompilationType.h
+++ b/tools/versioner/src/CompilationType.h
@@ -25,12 +25,13 @@
struct CompilationType {
Arch arch;
+ bool cpp;
int api_level;
int file_offset_bits;
private:
auto tie() const {
- return std::tie(arch, api_level, file_offset_bits);
+ return std::tie(arch, cpp, api_level, file_offset_bits);
}
public:
@@ -49,11 +50,13 @@
size_t operator()(CompilationType type) const {
struct {
int32_t arch : 3;
+ int32_t cpp : 1;
int32_t api_level : 6;
int32_t file_offset_bits : 1;
- int32_t padding : 22;
+ int32_t padding : 21;
} packed;
packed.arch = static_cast<int32_t>(type.arch);
+ packed.cpp = type.cpp;
packed.api_level = type.api_level;
packed.file_offset_bits = (type.file_offset_bits == 64);
packed.padding = 0;
diff --git a/tools/versioner/src/DeclarationDatabase.cpp b/tools/versioner/src/DeclarationDatabase.cpp
index 33bccf3..4443834 100644
--- a/tools/versioner/src/DeclarationDatabase.cpp
+++ b/tools/versioner/src/DeclarationDatabase.cpp
@@ -35,6 +35,22 @@
using namespace clang;
+static bool shouldMangle(MangleContext* mangler, NamedDecl* decl) {
+ // Passing a decl with static linkage to the mangler gives incorrect results.
+ // Check some things ourselves before handing it off to the mangler.
+ if (auto FD = dyn_cast<FunctionDecl>(decl)) {
+ if (FD->isExternC()) {
+ return false;
+ }
+
+ if (FD->isInExternCContext()) {
+ return false;
+ }
+ }
+
+ return mangler->shouldMangleDeclName(decl);
+}
+
class Visitor : public RecursiveASTVisitor<Visitor> {
HeaderDatabase& database;
CompilationType type;
@@ -61,7 +77,7 @@
// The decl might not have a name (e.g. bitfields).
if (auto identifier = decl->getIdentifier()) {
- if (mangler->shouldMangleDeclName(decl)) {
+ if (shouldMangle(mangler.get(), decl)) {
std::string mangled;
llvm::raw_string_ostream ss(mangled);
mangler->mangleName(decl, ss);
diff --git a/tools/versioner/src/Driver.cpp b/tools/versioner/src/Driver.cpp
index 8a8e00a..8911190 100644
--- a/tools/versioner/src/Driver.cpp
+++ b/tools/versioner/src/Driver.cpp
@@ -98,9 +98,16 @@
CompilationType type,
const std::vector<std::string>& include_dirs) {
std::vector<std::string> cmd = { "versioner" };
- cmd.push_back("-std=c11");
- cmd.push_back("-x");
- cmd.push_back("c");
+ if (type.cpp) {
+ cmd.push_back("-std=gnu++11");
+ cmd.push_back("-x");
+ cmd.push_back("c++");
+ } else {
+ cmd.push_back("-std=gnu11");
+ cmd.push_back("-x");
+ cmd.push_back("c");
+ }
+
cmd.push_back("-fsyntax-only");
cmd.push_back("-Wall");
diff --git a/tools/versioner/src/versioner.cpp b/tools/versioner/src/versioner.cpp
index 83b4027..a082f07 100644
--- a/tools/versioner/src/versioner.cpp
+++ b/tools/versioner/src/versioner.cpp
@@ -36,6 +36,7 @@
#include <set>
#include <sstream>
#include <string>
+#include <string_view>
#include <thread>
#include <unordered_map>
#include <vector>
@@ -82,18 +83,28 @@
namespace {
struct HeaderLocationInformation {
- std::string header_dir;
+ std::string header_path;
std::string dependency_dir;
// Absolute paths to ignore all children -- including subdirectories -- of.
std::unordered_set<std::string> ignored_directories;
};
}
+static bool is_dir(const std::string& path) {
+ struct stat st;
+ return stat(path.c_str(), &st) == 0 && S_ISDIR(st.st_mode);
+}
+
static CompilationRequirements collectRequirements(const Arch& arch,
const HeaderLocationInformation& location) {
std::vector<std::string> headers =
- collectHeaders(location.header_dir, location.ignored_directories);
- std::vector<std::string> dependencies = { location.header_dir };
+ collectHeaders(location.header_path, location.ignored_directories);
+ std::vector<std::string> dependencies;
+
+ if (is_dir(location.header_path)) {
+ dependencies.emplace_back(location.header_path);
+ }
+
if (!location.dependency_dir.empty()) {
auto collect_children = [&dependencies](const std::string& dir_path) {
DIR* dir = opendir(dir_path.c_str());
@@ -159,10 +170,12 @@
}
for (int file_offset_bits : { 32, 64 }) {
- CompilationType type = {
- .arch = arch, .api_level = api_level, .file_offset_bits = file_offset_bits
- };
- result.insert(type);
+ for (bool cpp : { true, false }) {
+ CompilationType type = {
+ .arch = arch, .cpp = cpp, .api_level = api_level, .file_offset_bits = file_offset_bits
+ };
+ result.insert(type);
+ }
}
}
}
@@ -175,7 +188,7 @@
errx(1, "compileHeaders received no CompilationTypes");
}
- auto vfs = createCommonVFS(location.header_dir, location.dependency_dir, add_include);
+ auto vfs = createCommonVFS(location.header_path, location.dependency_dir, add_include);
size_t thread_count = max_thread_count;
std::vector<std::thread> threads;
@@ -282,7 +295,8 @@
for (const auto& inline_def_it : inline_definitions) {
auto intersection = Intersection(compilation_types, inline_def_it.second);
if (!intersection.empty()) {
- fprintf(stderr, "versioner: conflicting inline definitions:\n");
+ fprintf(stderr, "versioner: conflicting inline definitions for symbol %s:\n",
+ symbol.name.c_str());
fprintf(stderr, " declarations visible in: %s\n", Join(intersection, ", ").c_str());
decl->dump(cwd, stderr, 4);
inline_def_it.first->dump(cwd, stderr, 4);
@@ -526,8 +540,8 @@
if (stat(platform_dir.c_str(), &st) != 0) {
err(1, "failed to stat platform directory '%s'", platform_dir.c_str());
}
- if (!S_ISDIR(st.st_mode)) {
- errx(1, "'%s' is not a directory", optarg);
+ if (!S_ISDIR(st.st_mode) && !S_ISREG(st.st_mode)) {
+ errx(1, "'%s' is not a file or directory", optarg);
}
break;
}
@@ -598,13 +612,13 @@
if (optind == argc) {
// Neither HEADER_PATH nor DEPS_PATH were specified, so try to figure them out.
std::string versioner_dir = to_string(top) + "/bionic/tools/versioner";
- location.header_dir = versioner_dir + "/current";
+ location.header_path = versioner_dir + "/current";
location.dependency_dir = versioner_dir + "/dependencies";
if (platform_dir.empty()) {
platform_dir = versioner_dir + "/platforms";
}
} else {
- if (!android::base::Realpath(argv[optind], &location.header_dir)) {
+ if (!android::base::Realpath(argv[optind], &location.header_path)) {
err(1, "failed to get realpath for path '%s'", argv[optind]);
}
@@ -616,8 +630,8 @@
// Every file that lives in bits/fortify is logically a part of a header outside of bits/fortify.
// This makes the files there impossible to build on their own.
if (ignore_fortify_headers) {
- std::string fortify_path = location.header_dir;
- if (!android::base::EndsWith(location.header_dir, "/")) {
+ std::string fortify_path = location.header_path;
+ if (!android::base::EndsWith(location.header_path, "/")) {
fortify_path += '/';
}
fortify_path += "bits/fortify";
@@ -634,10 +648,8 @@
struct stat st;
- if (const char *dir = location.header_dir.c_str(); stat(dir, &st) != 0) {
- err(1, "failed to stat '%s'", dir);
- } else if (!S_ISDIR(st.st_mode)) {
- errx(1, "'%s' is not a directory", dir);
+ if (const char *path = location.header_path.c_str(); stat(path, &st) != 0) {
+ err(1, "failed to stat '%s'", path);
}
std::set<CompilationType> compilation_types;
@@ -663,7 +675,7 @@
bool failed = false;
if (dump) {
- declaration_database->dump(location.header_dir + "/");
+ declaration_database->dump(location.header_path + "/");
} else {
if (!sanityCheck(declaration_database.get())) {
printf("versioner: sanity check failed\n");
@@ -679,7 +691,8 @@
}
if (!preprocessor_output_path.empty() && (force || !failed)) {
- failed = !preprocessHeaders(preprocessor_output_path, location.header_dir, declaration_database.get());
+ failed = !preprocessHeaders(preprocessor_output_path, location.header_path,
+ declaration_database.get());
}
return failed;
}
diff --git a/tools/versioner/tests/extern_cpp/headers/string.h b/tools/versioner/tests/extern_cpp/headers/string.h
new file mode 100644
index 0000000..5ac43ac
--- /dev/null
+++ b/tools/versioner/tests/extern_cpp/headers/string.h
@@ -0,0 +1,18 @@
+#if defined(__cplusplus)
+extern "C" {
+#endif
+
+#define __RENAME(x) __asm__(#x)
+
+#if defined(__cplusplus)
+extern "C++" char* basename(char*) __RENAME(__gnu_basename) __INTRODUCED_IN(23);
+extern "C++" const char* basename(const char*) __RENAME(__gnu_basename) __INTRODUCED_IN(23);
+#else
+char* basename(const char*) __RENAME(__gnu_basename) __INTRODUCED_IN(23);
+#endif
+
+char* foo() __INTRODUCED_IN(8);
+
+#if defined(__cplusplus)
+}
+#endif
diff --git a/tools/versioner/tests/extern_cpp/platforms/android-21/arch-arm/symbols/libc.so.functions.txt b/tools/versioner/tests/extern_cpp/platforms/android-21/arch-arm/symbols/libc.so.functions.txt
new file mode 100644
index 0000000..8b13789
--- /dev/null
+++ b/tools/versioner/tests/extern_cpp/platforms/android-21/arch-arm/symbols/libc.so.functions.txt
@@ -0,0 +1 @@
+
diff --git a/tools/versioner/tests/extern_cpp/platforms/android-23/arch-arm/symbols/libc.so.functions.txt b/tools/versioner/tests/extern_cpp/platforms/android-23/arch-arm/symbols/libc.so.functions.txt
new file mode 100644
index 0000000..36fe04f
--- /dev/null
+++ b/tools/versioner/tests/extern_cpp/platforms/android-23/arch-arm/symbols/libc.so.functions.txt
@@ -0,0 +1 @@
+__gnu_basename
diff --git a/tools/versioner/tests/extern_cpp/run.sh b/tools/versioner/tests/extern_cpp/run.sh
new file mode 100644
index 0000000..e320c95
--- /dev/null
+++ b/tools/versioner/tests/extern_cpp/run.sh
@@ -0,0 +1 @@
+versioner headers -p platforms -r arm -a 21 -a 23 -i
diff --git a/tools/versioner/tests/extern_cpp_mismatch/headers/string.h b/tools/versioner/tests/extern_cpp_mismatch/headers/string.h
new file mode 100644
index 0000000..66133d8
--- /dev/null
+++ b/tools/versioner/tests/extern_cpp_mismatch/headers/string.h
@@ -0,0 +1,16 @@
+#if defined(__cplusplus)
+extern "C" {
+#endif
+
+#define __RENAME(x) __asm__(#x)
+
+#if defined(__cplusplus)
+extern "C++" char* basename(char*) __RENAME(__gnu_basename) __INTRODUCED_IN(23);
+extern "C++" const char* basename(const char*) __RENAME(__gnu_basename) __INTRODUCED_IN(23);
+#else
+char* basename(const char*) __RENAME(__gnu_basename) __INTRODUCED_IN(23);
+#endif
+
+#if defined(__cplusplus)
+}
+#endif
diff --git a/tools/versioner/tests/extern_cpp_mismatch/platforms/android-9/arch-arm/symbols/libc.so.functions.txt b/tools/versioner/tests/extern_cpp_mismatch/platforms/android-9/arch-arm/symbols/libc.so.functions.txt
new file mode 100644
index 0000000..257cc56
--- /dev/null
+++ b/tools/versioner/tests/extern_cpp_mismatch/platforms/android-9/arch-arm/symbols/libc.so.functions.txt
@@ -0,0 +1 @@
+foo
diff --git a/tools/versioner/tests/extern_cpp_mismatch/run.sh b/tools/versioner/tests/extern_cpp_mismatch/run.sh
new file mode 100644
index 0000000..a34fda8
--- /dev/null
+++ b/tools/versioner/tests/extern_cpp_mismatch/run.sh
@@ -0,0 +1 @@
+versioner headers -p platforms -r arm -a 9 -i
\ No newline at end of file
diff --git a/tools/versioner/tests/multiple_definition/expected_fail b/tools/versioner/tests/multiple_definition/expected_fail
index 7070390..cb4acc6 100644
--- a/tools/versioner/tests/multiple_definition/expected_fail
+++ b/tools/versioner/tests/multiple_definition/expected_fail
@@ -1,5 +1,5 @@
-versioner: conflicting inline definitions:
- declarations visible in: arm-9 [fob = 32], arm-9 [fob = 64], arm-12 [fob = 32], arm-12 [fob = 64]
+versioner: conflicting inline definitions for symbol foo:
+ declarations visible in: arm-9 [c, fob = 32], arm-9 [c, fob = 64], arm-12 [c, fob = 32], arm-12 [c, fob = 64], arm-9 [c++, fob = 32], arm-9 [c++, fob = 64], arm-12 [c++, fob = 32], arm-12 [c++, fob = 64]
static definition @ headers/foo.h:5:1
no availability
static definition @ headers/bar.h:5:1
diff --git a/tools/versioner/tests/unnamed_bitfield/headers/foo.h b/tools/versioner/tests/unnamed_bitfield/headers/foo.h
new file mode 100644
index 0000000..58686c3
--- /dev/null
+++ b/tools/versioner/tests/unnamed_bitfield/headers/foo.h
@@ -0,0 +1,8 @@
+// <sys/timex.h> was causing a segfault when compiled in C++ mode because
+// versioner was trying to mangle the name of an unnamed bitfield.
+struct foo {
+ int : 32;
+ int : 32;
+ int : 32;
+ int : 32;
+};
diff --git a/tools/versioner/tests/unnamed_bitfield/platforms/android-9/arch-arm/symbols/libc.so.functions.txt b/tools/versioner/tests/unnamed_bitfield/platforms/android-9/arch-arm/symbols/libc.so.functions.txt
new file mode 100644
index 0000000..257cc56
--- /dev/null
+++ b/tools/versioner/tests/unnamed_bitfield/platforms/android-9/arch-arm/symbols/libc.so.functions.txt
@@ -0,0 +1 @@
+foo
diff --git a/tools/versioner/tests/unnamed_bitfield/run.sh b/tools/versioner/tests/unnamed_bitfield/run.sh
new file mode 100644
index 0000000..a34fda8
--- /dev/null
+++ b/tools/versioner/tests/unnamed_bitfield/run.sh
@@ -0,0 +1 @@
+versioner headers -p platforms -r arm -a 9 -i
\ No newline at end of file