versioner: properly handle extern "C", "C++".
extern "C" and "C++" are parsed as a LinkageSpecDecl with the real Decl
as a child node. This leads to the preprocessor sticking its guard
between the extern specifier and the declaration.
Update the AST visitor to add a special-case for calculating the
SourceRange on a LinkageSpecDecl, and add a test.
Bug: https://github.com/android-ndk/ndk/issues/440
Test: python run_tests.py
Change-Id: I76445fe366cef46cfd2f16fb93d534d410c5edca
diff --git a/libc/include/string.h b/libc/include/string.h
index d409ba8..226566c 100644
--- a/libc/include/string.h
+++ b/libc/include/string.h
@@ -63,11 +63,8 @@
char* __strchr_chk(const char* __s, int __ch, size_t __n) __INTRODUCED_IN(18);
#if defined(__USE_GNU)
#if defined(__cplusplus)
-/* The versioner doesn't handle C++ blocks yet, so manually guarded. */
-#if __ANDROID_API__ >= 24
extern "C++" char* strchrnul(char* __s, int __ch) __RENAME(strchrnul) __attribute_pure__ __INTRODUCED_IN(24);
extern "C++" const char* strchrnul(const char* __s, int __ch) __RENAME(strchrnul) __attribute_pure__ __INTRODUCED_IN(24);
-#endif /* __ANDROID_API__ >= 24 */
#else
char* strchrnul(const char* __s, int __ch) __attribute_pure__ __INTRODUCED_IN(24);
#endif
@@ -136,11 +133,8 @@
* It doesn't modify its argument, and in C++ it's const-correct.
*/
#if defined(__cplusplus)
-/* The versioner doesn't handle C++ blocks yet, so manually guarded. */
-#if __ANDROID_API__ >= 23
extern "C++" char* basename(char* __path) __RENAME(__gnu_basename) __INTRODUCED_IN(23);
extern "C++" const char* basename(const char* __path) __RENAME(__gnu_basename) __INTRODUCED_IN(23);
-#endif /* __ANDROID_API__ >= 23 */
#else
char* basename(const char* __path) __RENAME(__gnu_basename) __INTRODUCED_IN(23);
#endif
diff --git a/tools/versioner/src/DeclarationDatabase.cpp b/tools/versioner/src/DeclarationDatabase.cpp
index 4443834..19c2f0d 100644
--- a/tools/versioner/src/DeclarationDatabase.cpp
+++ b/tools/versioner/src/DeclarationDatabase.cpp
@@ -90,7 +90,7 @@
return "<unnamed>";
}
- bool VisitDecl(Decl* decl) {
+ bool VisitDeclaratorDecl(DeclaratorDecl* decl, SourceRange range) {
// Skip declarations inside of functions (function arguments, variable declarations inside of
// inline functions, etc).
if (decl->getParentFunctionOrMethod()) {
@@ -143,21 +143,6 @@
return true;
}
- auto start_loc = src_manager.getPresumedLoc(decl->getLocStart());
- auto end_loc = src_manager.getPresumedLoc(decl->getLocEnd());
-
- Location location = {
- .filename = start_loc.getFilename(),
- .start = {
- .line = start_loc.getLine(),
- .column = start_loc.getColumn(),
- },
- .end = {
- .line = end_loc.getLine(),
- .column = end_loc.getColumn(),
- }
- };
-
DeclarationAvailability availability;
// Find and parse __ANDROID_AVAILABILITY_DUMP__ annotations.
@@ -215,6 +200,24 @@
std::tie(symbol_it, dummy) = database.symbols.insert({ declaration_name, symbol });
}
+ auto expansion_range = src_manager.getExpansionRange(range);
+ auto filename = src_manager.getFilename(expansion_range.getBegin());
+ if (filename != src_manager.getFilename(expansion_range.getEnd())) {
+ errx(1, "expansion range filenames don't match");
+ }
+
+ Location location = {
+ .filename = filename,
+ .start = {
+ .line = src_manager.getExpansionLineNumber(expansion_range.getBegin()),
+ .column = src_manager.getExpansionColumnNumber(expansion_range.getBegin()),
+ },
+ .end = {
+ .line = src_manager.getExpansionLineNumber(expansion_range.getEnd()),
+ .column = src_manager.getExpansionColumnNumber(expansion_range.getEnd()),
+ }
+ };
+
// Find or insert an entry for the declaration.
if (auto declaration_it = symbol_it->second.declarations.find(location);
declaration_it != symbol_it->second.declarations.end()) {
@@ -238,6 +241,38 @@
return true;
}
+
+ bool VisitDeclaratorDecl(DeclaratorDecl* decl) {
+ return VisitDeclaratorDecl(decl, decl->getSourceRange());
+ }
+
+ bool TraverseLinkageSpecDecl(LinkageSpecDecl* decl) {
+ // Make sure that we correctly calculate the SourceRange of a declaration that has a non-braced
+ // extern "C"/"C++".
+ if (!decl->hasBraces()) {
+ DeclaratorDecl* child = nullptr;
+ for (auto child_decl : decl->decls()) {
+ if (child != nullptr) {
+ errx(1, "LinkageSpecDecl has multiple children");
+ }
+
+ if (DeclaratorDecl* declarator_decl = dyn_cast<DeclaratorDecl>(child_decl)) {
+ child = declarator_decl;
+ } else {
+ errx(1, "child of LinkageSpecDecl is not a DeclaratorDecl");
+ }
+ }
+
+ return VisitDeclaratorDecl(child, decl->getSourceRange());
+ }
+
+ for (auto child : decl->decls()) {
+ if (!TraverseDecl(child)) {
+ return false;
+ }
+ }
+ return true;
+ }
};
bool DeclarationAvailability::merge(const DeclarationAvailability& other) {
diff --git a/tools/versioner/tests/preprocessor_extern_cpp/expected/foo.h b/tools/versioner/tests/preprocessor_extern_cpp/expected/foo.h
new file mode 100644
index 0000000..9b2d122
--- /dev/null
+++ b/tools/versioner/tests/preprocessor_extern_cpp/expected/foo.h
@@ -0,0 +1,27 @@
+#define __RENAME(x) asm(#x)
+
+#if defined(__cplusplus)
+
+#if __ANDROID_API__ >= 24
+extern "C++" const char* strchrnul(const char*, int) __RENAME(strchrnul) __INTRODUCED_IN(24);
+#endif /* __ANDROID_API__ >= 24 */
+
+#endif
+
+#if defined(__cplusplus)
+extern "C" int foo();
+#endif
+
+#if defined(__cplusplus)
+extern "C" {
+#endif
+
+
+#if __ANDROID_API__ >= 24
+char* strchrnul(char*, int) __INTRODUCED_IN(24);
+#endif /* __ANDROID_API__ >= 24 */
+
+
+#if defined(__cplusplus)
+}
+#endif
diff --git a/tools/versioner/tests/preprocessor_extern_cpp/headers/foo.h b/tools/versioner/tests/preprocessor_extern_cpp/headers/foo.h
new file mode 100644
index 0000000..de26d21
--- /dev/null
+++ b/tools/versioner/tests/preprocessor_extern_cpp/headers/foo.h
@@ -0,0 +1,19 @@
+#define __RENAME(x) asm(#x)
+
+#if defined(__cplusplus)
+extern "C++" const char* strchrnul(const char*, int) __RENAME(strchrnul) __INTRODUCED_IN(24);
+#endif
+
+#if defined(__cplusplus)
+extern "C" int foo();
+#endif
+
+#if defined(__cplusplus)
+extern "C" {
+#endif
+
+char* strchrnul(char*, int) __INTRODUCED_IN(24);
+
+#if defined(__cplusplus)
+}
+#endif
diff --git a/tools/versioner/tests/preprocessor_extern_cpp/platforms/android-9/arch-arm/symbols/libc.so.functions.txt b/tools/versioner/tests/preprocessor_extern_cpp/platforms/android-9/arch-arm/symbols/libc.so.functions.txt
new file mode 100644
index 0000000..257cc56
--- /dev/null
+++ b/tools/versioner/tests/preprocessor_extern_cpp/platforms/android-9/arch-arm/symbols/libc.so.functions.txt
@@ -0,0 +1 @@
+foo
diff --git a/tools/versioner/tests/preprocessor_extern_cpp/run.sh b/tools/versioner/tests/preprocessor_extern_cpp/run.sh
new file mode 100644
index 0000000..50d9b5c
--- /dev/null
+++ b/tools/versioner/tests/preprocessor_extern_cpp/run.sh
@@ -0,0 +1,29 @@
+set -e
+
+function run_test {
+ SRC=$1
+ DST=$2
+ rm -rf $2
+ versioner -a 9 -a 12 -a 13 -a 14 -a 15 $1 -i -o $2
+ diff -q -w -B $2 expected
+}
+
+run_test headers out
+run_test headers/ out
+run_test headers out/
+run_test headers/ out/
+
+run_test `pwd`/headers out
+run_test `pwd`/headers/ out
+run_test `pwd`/headers out/
+run_test `pwd`/headers/ out/
+
+run_test headers `pwd`/out
+run_test headers/ `pwd`/out
+run_test headers `pwd`/out/
+run_test headers/ `pwd`/out/
+
+run_test `pwd`/headers `pwd`/out
+run_test `pwd`/headers/ `pwd`/out
+run_test `pwd`/headers `pwd`/out/
+run_test `pwd`/headers/ `pwd`/out/