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

6 files changed, 127 insertions, 22 deletions

diff --git a/libc/include/string.h b/libc/include/string.h
index d409ba848..226566c0f 100644
--- a/libc/include/string.h
+++ b/libc/include/string.h
@@ -63,11 +63,8 @@ char* strchr(const char* __s, int __ch) __attribute_pure__ __overloadable __RENA
 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 @@ size_t strxfrm_l(char* __dst, const char* __src, size_t __n, locale_t __l) __INT
  * 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 44438340e..19c2f0d0a 100644
--- a/tools/versioner/src/DeclarationDatabase.cpp
+++ b/tools/versioner/src/DeclarationDatabase.cpp
@@ -90,7 +90,7 @@ class Visitor : public RecursiveASTVisitor<Visitor> {
     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 @@ class Visitor : public RecursiveASTVisitor<Visitor> {
       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 @@ class Visitor : public RecursiveASTVisitor<Visitor> {
       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 @@ class Visitor : public RecursiveASTVisitor<Visitor> {
 
     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 000000000..9b2d122c9
--- /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 000000000..de26d2115
--- /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 000000000..257cc5642
--- /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 000000000..50d9b5c10
--- /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/