header-checker: Add -allow-adding-removing-weak-symbols

This commit adds `-allow-adding-removing-weak-symbols` option to
`header-abi-diff`.  This allows vendor-specific branch to add or remove
weak symbols from their VNDK shared libs.  This is sometimes necessary
when venders have their compiler toolchain or use different compiler
flags (e.g. inline heuristics).

Bug: 112760591
Test: ./tests/test.py
Change-Id: I94bceacc90cab093e8aa602bb7d866d6f433c5c7
Merged-In: I94bceacc90cab093e8aa602bb7d866d6f433c5c7

12 files changed, 189 insertions, 26 deletions

diff --git a/vndk/tools/header-checker/src/diff/abi_diff.cpp b/vndk/tools/header-checker/src/diff/abi_diff.cpp
index 0d2d759dc..b0f1e09a3 100644
--- a/vndk/tools/header-checker/src/diff/abi_diff.cpp
+++ b/vndk/tools/header-checker/src/diff/abi_diff.cpp
@@ -61,7 +61,7 @@ repr::CompatibilityStatusIR HeaderAbiDiff::CompareTUs(
   const AbiElementMap<const repr::TypeIR *> new_types =
       new_tu.GetTypeGraph();
 
-  // Collect fills in added, removed ,unsafe and safe function diffs.
+  // CollectDynsymExportables() fills in added, removed, unsafe, and safe function diffs.
   if (!CollectDynsymExportables(old_tu.GetFunctions(), new_tu.GetFunctions(),
                                 old_tu.GetElfFunctions(),
                                 new_tu.GetElfFunctions(),
@@ -225,11 +225,11 @@ bool HeaderAbiDiff::Collect(
     const AbiElementMap<const repr::TypeIR *> &old_types_map,
     const AbiElementMap<const repr::TypeIR *> &new_types_map) {
   if (!PopulateRemovedElements(
-          old_elements_map, new_elements_map, new_elf_map, ir_diff_dumper,
-          repr::DiffMessageIR::Removed, old_types_map) ||
+          old_elements_map, new_elements_map, old_elf_map, new_elf_map,
+          ir_diff_dumper, repr::DiffMessageIR::Removed, old_types_map) ||
       !PopulateRemovedElements(
-          new_elements_map, old_elements_map, old_elf_map, ir_diff_dumper,
-          repr::IRDiffDumper::DiffKind::Added, new_types_map)) {
+          new_elements_map, old_elements_map, new_elf_map, old_elf_map,
+          ir_diff_dumper, repr::DiffMessageIR::Added, new_types_map)) {
     llvm::errs() << "Populating functions in report failed\n";
     return false;
   }
@@ -257,6 +257,10 @@ bool HeaderAbiDiff::PopulateElfElements(
     repr::IRDiffDumper *ir_diff_dumper,
     repr::IRDiffDumper::DiffKind diff_kind) {
   for (auto &&elf_element : elf_elements) {
+    if (allow_adding_removing_weak_symbols_ &&
+        elf_element->GetBinding() == repr::ElfSymbolIR::Weak) {
+      continue;
+    }
     if (!ir_diff_dumper->AddElfSymbolMessageIR(elf_element, diff_kind)) {
       return false;
     }
@@ -268,15 +272,16 @@ template <typename T>
 bool HeaderAbiDiff::PopulateRemovedElements(
     const AbiElementMap<const T*> &old_elements_map,
     const AbiElementMap<const T*> &new_elements_map,
-    const AbiElementMap<const repr::ElfSymbolIR *> *elf_map,
+    const AbiElementMap<const repr::ElfSymbolIR *> *old_elf_map,
+    const AbiElementMap<const repr::ElfSymbolIR *> *new_elf_map,
     repr::IRDiffDumper *ir_diff_dumper,
     repr::IRDiffDumper::DiffKind diff_kind,
     const AbiElementMap<const repr::TypeIR *> &removed_types_map) {
   std::vector<const T *> removed_elements =
       utils::FindRemovedElements(old_elements_map, new_elements_map);
-  if (!DumpLoneElements(removed_elements, elf_map, ir_diff_dumper, diff_kind,
-                        removed_types_map)) {
-    llvm::errs() << "Dumping added / removed element to report failed\n";
+  if (!DumpLoneElements(removed_elements, old_elf_map, new_elf_map,
+                        ir_diff_dumper, diff_kind, removed_types_map)) {
+    llvm::errs() << "Dumping added or removed element to report failed\n";
     return false;
   }
   return true;
@@ -306,33 +311,52 @@ bool HeaderAbiDiff::PopulateCommonElements(
 template <typename T>
 bool HeaderAbiDiff::DumpLoneElements(
     std::vector<const T *> &elements,
-    const AbiElementMap<const repr::ElfSymbolIR *> *elf_map,
+    const AbiElementMap<const repr::ElfSymbolIR *> *old_elf_map,
+    const AbiElementMap<const repr::ElfSymbolIR *> *new_elf_map,
     repr::IRDiffDumper *ir_diff_dumper,
     repr::IRDiffDumper::DiffKind diff_kind,
     const AbiElementMap<const repr::TypeIR *> &types_map) {
-  // If the record / enum has source file information, skip it.
   std::smatch source_file_match;
   std::regex source_file_regex(" at ");
+
   for (auto &&element : elements) {
     if (IgnoreSymbol<T>(element, ignored_symbols_,
                         [](const T *e) {return e->GetLinkerSetKey();})) {
       continue;
     }
-    // The element does exist in the .dynsym table, we do not have meta-data
-    // surrounding the element.
+
+    // If an element (FunctionIR or GlobalVarIR) is missing from the new ABI
+    // dump but a corresponding ELF symbol (ElfFunctionIR or ElfObjectIR) can
+    // be found in the new ABI dump file, don't emit error on this element.
+    // This may happen when the standard reference target implements the
+    // function (or the global variable) in C/C++ and the target-under-test
+    // implements the function (or the global variable) in assembly.
     const std::string &element_linker_set_key = element->GetLinkerSetKey();
-    if ((elf_map != nullptr) &&
-        (elf_map->find(element_linker_set_key) != elf_map->end())) {
+    if (new_elf_map &&
+        new_elf_map->find(element_linker_set_key) != new_elf_map->end()) {
       continue;
     }
+
+    // If the `-ignore-weak-symbols` option is enabled, ignore the element if
+    // it was a weak symbol.
+    if (allow_adding_removing_weak_symbols_ && old_elf_map) {
+      auto elem_it = old_elf_map->find(element_linker_set_key);
+      if (elem_it != old_elf_map->end() &&
+          elem_it->second->GetBinding() == repr::ElfSymbolIR::Weak) {
+        continue;
+      }
+    }
+
+    // If the record / enum has source file information, skip it.
     if (std::regex_search(element_linker_set_key, source_file_match,
                           source_file_regex)) {
       continue;
     }
+
     auto element_copy = *element;
     ReplaceTypeIdsWithTypeNames(types_map, &element_copy);
     if (!ir_diff_dumper->AddLinkableMessageIR(&element_copy, diff_kind)) {
-      llvm::errs() << "Couldn't dump added /removed element\n";
+      llvm::errs() << "Couldn't dump added or removed element\n";
       return false;
     }
   }
diff --git a/vndk/tools/header-checker/src/diff/abi_diff.h b/vndk/tools/header-checker/src/diff/abi_diff.h
index cbda960c1..ca64716a5 100644
--- a/vndk/tools/header-checker/src/diff/abi_diff.h
+++ b/vndk/tools/header-checker/src/diff/abi_diff.h
@@ -36,6 +36,7 @@ class HeaderAbiDiff {
                 const std::string &old_dump, const std::string &new_dump,
                 const std::string &compatibility_report,
                 const std::set<std::string> &ignored_symbols,
+                bool allow_adding_removing_weak_symbols,
                 const DiffPolicyOptions &diff_policy_options,
                 bool check_all_apis, repr::TextFormatIR text_format_old,
                 repr::TextFormatIR text_format_new,
@@ -44,6 +45,7 @@ class HeaderAbiDiff {
         new_dump_(new_dump), cr_(compatibility_report),
         ignored_symbols_(ignored_symbols),
         diff_policy_options_(diff_policy_options),
+        allow_adding_removing_weak_symbols_(allow_adding_removing_weak_symbols),
         check_all_apis_(check_all_apis),
         text_format_old_(text_format_old), text_format_new_(text_format_new),
         text_format_diff_(text_format_diff) {}
@@ -90,7 +92,8 @@ class HeaderAbiDiff {
   bool PopulateRemovedElements(
       const AbiElementMap<const T *> &old_elements_map,
       const AbiElementMap<const T *> &new_elements_map,
-      const AbiElementMap<const repr::ElfSymbolIR *> *elf_map,
+      const AbiElementMap<const repr::ElfSymbolIR *> *old_elf_map,
+      const AbiElementMap<const repr::ElfSymbolIR *> *new_elf_map,
       repr::IRDiffDumper *ir_diff_dumper,
       repr::IRDiffDumper::DiffKind diff_kind,
       const AbiElementMap<const repr::TypeIR *> &types_map);
@@ -115,7 +118,8 @@ class HeaderAbiDiff {
   template <typename T>
   bool DumpLoneElements(
       std::vector<const T *> &elements,
-      const AbiElementMap<const repr::ElfSymbolIR *> *elf_map,
+      const AbiElementMap<const repr::ElfSymbolIR *> *old_elf_map,
+      const AbiElementMap<const repr::ElfSymbolIR *> *new_elf_map,
       repr::IRDiffDumper *ir_diff_dumper,
       repr::IRDiffDumper::DiffKind diff_kind,
       const AbiElementMap<const repr::TypeIR *> &old_types_map);
@@ -147,6 +151,7 @@ class HeaderAbiDiff {
   const std::string &cr_;
   const std::set<std::string> &ignored_symbols_;
   const DiffPolicyOptions &diff_policy_options_;
+  bool allow_adding_removing_weak_symbols_;
   bool check_all_apis_;
   std::set<std::string> type_cache_;
   repr::TextFormatIR text_format_old_;
diff --git a/vndk/tools/header-checker/src/diff/header_abi_diff.cpp b/vndk/tools/header-checker/src/diff/header_abi_diff.cpp
index ac13b535f..c3c2b7188 100644
--- a/vndk/tools/header-checker/src/diff/header_abi_diff.cpp
+++ b/vndk/tools/header-checker/src/diff/header_abi_diff.cpp
@@ -121,6 +121,13 @@ static llvm::cl::opt<TextFormatIR> text_format_diff(
     llvm::cl::init(TextFormatIR::ProtobufTextFormat),
     llvm::cl::cat(header_checker_category));
 
+static llvm::cl::opt<bool> allow_adding_removing_weak_symbols(
+    "allow-adding-removing-weak-symbols",
+    llvm::cl::desc("Do not treat addition or removal of weak symbols as "
+                   "incompatible changes."),
+    llvm::cl::init(false), llvm::cl::Optional,
+    llvm::cl::cat(header_checker_category));
+
 static std::set<std::string> LoadIgnoredSymbols(std::string &symbol_list_path) {
   std::ifstream symbol_ifstream(symbol_list_path);
   std::set<std::string> ignored_symbols;
@@ -154,11 +161,11 @@ int main(int argc, const char **argv) {
   if (llvm::sys::fs::exists(ignore_symbol_list)) {
     ignored_symbols = LoadIgnoredSymbols(ignore_symbol_list);
   }
-  DiffPolicyOptions diff_policy_options(
-      consider_opaque_types_different);
+  DiffPolicyOptions diff_policy_options(consider_opaque_types_different);
   HeaderAbiDiff judge(lib_name, arch, old_dump, new_dump, compatibility_report,
-                      ignored_symbols, diff_policy_options, check_all_apis,
-                      text_format_old, text_format_new, text_format_diff);
+                      ignored_symbols, allow_adding_removing_weak_symbols,
+                      diff_policy_options, check_all_apis, text_format_old,
+                      text_format_new, text_format_diff);
 
   CompatibilityStatusIR status = judge.GenerateCompatibilityReport();
 
diff --git a/vndk/tools/header-checker/src/repr/symbol/version_script_parser.cpp b/vndk/tools/header-checker/src/repr/symbol/version_script_parser.cpp
index 98ecea3f3..eecbfe554 100644
--- a/vndk/tools/header-checker/src/repr/symbol/version_script_parser.cpp
+++ b/vndk/tools/header-checker/src/repr/symbol/version_script_parser.cpp
@@ -120,11 +120,17 @@ VersionScriptParser::ParsedTags VersionScriptParser::ParseSymbolTags(
       continue;
     }
 
-    // Check future tags.
+    // Check the future tag.
     if (tag == "future") {
       result.has_future_tag_ = true;
       continue;
     }
+
+    // Check the weak binding tag.
+    if (tag == "weak") {
+      result.has_weak_tag_ = true;
+      continue;
+    }
   }
 
   return result;
@@ -183,11 +189,14 @@ bool VersionScriptParser::ParseSymbolLine(const std::string &line,
     return true;
   }
 
+  ElfSymbolIR::ElfSymbolBinding binding =
+      tags.has_weak_tag_ ? ElfSymbolIR::ElfSymbolBinding::Weak
+                         : ElfSymbolIR::ElfSymbolBinding::Global;
+
   if (tags.has_var_tag_) {
-    exported_symbols_->AddVar(symbol, ElfSymbolIR::ElfSymbolBinding::Global);
+    exported_symbols_->AddVar(symbol, binding);
   } else {
-    exported_symbols_->AddFunction(symbol,
-                                   ElfSymbolIR::ElfSymbolBinding::Global);
+    exported_symbols_->AddFunction(symbol, binding);
   }
   return true;
 }
diff --git a/vndk/tools/header-checker/src/repr/symbol/version_script_parser.h b/vndk/tools/header-checker/src/repr/symbol/version_script_parser.h
index 7a047a299..d3bb63708 100644
--- a/vndk/tools/header-checker/src/repr/symbol/version_script_parser.h
+++ b/vndk/tools/header-checker/src/repr/symbol/version_script_parser.h
@@ -44,6 +44,7 @@ class VersionScriptParser {
     unsigned has_excluded_tags_ : 1;
     unsigned has_future_tag_ : 1;
     unsigned has_var_tag_ : 1;
+    unsigned has_weak_tag_ : 1;
     utils::ApiLevel introduced_;
 
 
diff --git a/vndk/tools/header-checker/tests/integration/weak_symbols/example.c b/vndk/tools/header-checker/tests/integration/weak_symbols/example.c
new file mode 100644
index 000000000..65c35bf47
--- /dev/null
+++ b/vndk/tools/header-checker/tests/integration/weak_symbols/example.c
@@ -0,0 +1,8 @@
+#if !defined(NEW)
+
+extern void example(void) __attribute__((weak));
+
+void example(void) {
+}
+
+#endif  // !defined(NEW)
diff --git a/vndk/tools/header-checker/tests/integration/weak_symbols/libexample_new.map.txt b/vndk/tools/header-checker/tests/integration/weak_symbols/libexample_new.map.txt
new file mode 100644
index 000000000..060916683
--- /dev/null
+++ b/vndk/tools/header-checker/tests/integration/weak_symbols/libexample_new.map.txt
@@ -0,0 +1,3 @@
+LIBTEST_1.0 {
+  global:
+};
diff --git a/vndk/tools/header-checker/tests/integration/weak_symbols/libexample_old.map.txt b/vndk/tools/header-checker/tests/integration/weak_symbols/libexample_old.map.txt
new file mode 100644
index 000000000..f457f27a6
--- /dev/null
+++ b/vndk/tools/header-checker/tests/integration/weak_symbols/libexample_old.map.txt
@@ -0,0 +1,4 @@
+LIBTEST_1.0 {
+  global:
+    example;  # weak
+};
diff --git a/vndk/tools/header-checker/tests/module.py b/vndk/tools/header-checker/tests/module.py
index 3c281777d..9397ac19c 100755
--- a/vndk/tools/header-checker/tests/module.py
+++ b/vndk/tools/header-checker/tests/module.py
@@ -555,6 +555,37 @@ TEST_MODULES = [
             '--exclude-symbol-tag', 'mytag',
         ]
     ),
+
+    # Test data for test_allow_adding_removing_weak_symbols
+    LsdumpModule(
+        name='libweak_symbols_old',
+        arch='arm64',
+        srcs=[
+            'integration/weak_symbols/example.c',
+        ],
+        version_script='integration/weak_symbols/libexample_old.map.txt',
+        export_include_dirs=[],
+        dumper_flags=['-output-format', 'Json'],
+        linker_flags=[
+            '-input-format', 'Json',
+            '-output-format', 'Json',
+        ]
+    ),
+    LsdumpModule(
+        name='libweak_symbols_new',
+        arch='arm64',
+        srcs=[
+            'integration/weak_symbols/example.c',
+        ],
+        version_script='integration/weak_symbols/libexample_new.map.txt',
+        export_include_dirs=[],
+        dumper_flags=['-output-format', 'Json'],
+        linker_flags=[
+            '-input-format', 'Json',
+            '-output-format', 'Json',
+        ],
+        cflags=['-DNEW=1']
+    ),
 ]
 
 TEST_MODULES = {m.name: m for m in TEST_MODULES}
diff --git a/vndk/tools/header-checker/tests/reference_dumps/arm64/libweak_symbols_new.so.lsdump b/vndk/tools/header-checker/tests/reference_dumps/arm64/libweak_symbols_new.so.lsdump
new file mode 100644
index 000000000..30e1b67b6
--- /dev/null
+++ b/vndk/tools/header-checker/tests/reference_dumps/arm64/libweak_symbols_new.so.lsdump
@@ -0,0 +1,15 @@
+{
+ "array_types" : [],
+ "builtin_types" : [],
+ "elf_functions" : [],
+ "elf_objects" : [],
+ "enum_types" : [],
+ "function_types" : [],
+ "functions" : [],
+ "global_vars" : [],
+ "lvalue_reference_types" : [],
+ "pointer_types" : [],
+ "qualified_types" : [],
+ "record_types" : [],
+ "rvalue_reference_types" : []
+}
diff --git a/vndk/tools/header-checker/tests/reference_dumps/arm64/libweak_symbols_old.so.lsdump b/vndk/tools/header-checker/tests/reference_dumps/arm64/libweak_symbols_old.so.lsdump
new file mode 100644
index 000000000..a63ae1666
--- /dev/null
+++ b/vndk/tools/header-checker/tests/reference_dumps/arm64/libweak_symbols_old.so.lsdump
@@ -0,0 +1,37 @@
+{
+ "array_types" : [],
+ "builtin_types" :
+ [
+  {
+   "linker_set_key" : "void",
+   "name" : "void",
+   "referenced_type" : "type-1",
+   "self_type" : "type-1"
+  }
+ ],
+ "elf_functions" :
+ [
+  {
+   "binding" : "weak",
+   "name" : "example"
+  }
+ ],
+ "elf_objects" : [],
+ "enum_types" : [],
+ "function_types" : [],
+ "functions" :
+ [
+  {
+   "function_name" : "example",
+   "linker_set_key" : "example",
+   "return_type" : "type-1",
+   "source_file" : "/development/vndk/tools/header-checker/tests/integration/weak_symbols/example.c"
+  }
+ ],
+ "global_vars" : [],
+ "lvalue_reference_types" : [],
+ "pointer_types" : [],
+ "qualified_types" : [],
+ "record_types" : [],
+ "rvalue_reference_types" : []
+}
diff --git a/vndk/tools/header-checker/tests/test.py b/vndk/tools/header-checker/tests/test.py
index 1ed8852dc..57df7d378 100755
--- a/vndk/tools/header-checker/tests/test.py
+++ b/vndk/tools/header-checker/tests/test.py
@@ -309,6 +309,25 @@ class HeaderCheckerTest(unittest.TestCase):
             ["-input-format-old", "Json", "-input-format-new", "Json",
              "-consider-opaque-types-different"])
 
+    def test_allow_adding_removing_weak_symbols(self):
+        module_old = Module.get_test_modules_by_name("libweak_symbols_old")[0]
+        module_new = Module.get_test_modules_by_name("libweak_symbols_new")[0]
+        lsdump_old = self.get_or_create_ref_dump(module_old, False)
+        lsdump_new = self.get_or_create_ref_dump(module_new, False)
+
+        options = ["-input-format-old", "Json", "-input-format-new", "Json"]
+
+        # If `-allow-adding-removing-weak-symbols` is not specified, removing a
+        # weak symbol must be treated as an incompatible change.
+        self.run_and_compare_abi_diff(
+            lsdump_old, lsdump_new, "libweak_symbols", "arm64", 8, options)
+
+        # If `-allow-adding-removing-weak-symbols` is specified, removing a
+        # weak symbol must be fine and mustn't be a fatal error.
+        self.run_and_compare_abi_diff(
+            lsdump_old, lsdump_new, "libweak_symbols", "arm64", 0,
+            options + ["-allow-adding-removing-weak-symbols"])
+
     def test_linker_shared_object_file_and_version_script(self):
         base_dir = os.path.join(
             SCRIPT_DIR, 'integration', 'version_script_example')